This is an archive of the discontinued LLVM Phabricator instance.

[clang] p0388 array list initialization overloads
ClosedPublic

Authored by urnathan on Jun 8 2021, 9:33 AM.

Details

Summary

This is the second part of p0388, dealing with overloads of list
initialization to incomplete array types. It extends the handling
added in D103088 to permit incomplete arrays. We have to record that
the conversion involved an incomplete array, and so (readd) a bit flag
into the standard conversion sequence object. Comparing such
conversion sequences requires knowing (a) the number of array elements
initialized and (b) whether the initialization is of an incomplete array.

This also updates the web page to indicate p0388 is implemented (there
is no feature macro).

Diff Detail

Unit TestsFailed

Event Timeline

urnathan requested review of this revision.Jun 8 2021, 9:33 AM
urnathan created this revision.

Mostly nits from me, but one question I have is: do we want to support this as an extension in older Clang modes (with the usual diagnostics for compatibility checks)?

clang/include/clang/Sema/Overload.h
686
clang/lib/Sema/SemaOverload.cpp
3844–3845
3847–3853
5021–5023
5024
5105–5106
5110
urnathan marked 7 inline comments as done.Jun 21 2021, 6:41 AM
urnathan updated this revision to Diff 353373.Jun 21 2021, 7:53 AM

Updated diff, thanks for the comments. IYDM, can we address accepting this as an extension in <C++20 as a followup?

urnathan updated this revision to Diff 353382.Jun 21 2021, 8:15 AM
aaron.ballman accepted this revision.Jun 21 2021, 11:56 AM

Updated diff, thanks for the comments. IYDM, can we address accepting this as an extension in <C++20 as a followup?

I don't mind addressing that later. Thanks, this LGTM aside from one auto nit I had missed before.

clang/lib/Sema/SemaOverload.cpp
5081

Oops, I missed this auto earlier.

clang/test/SemaCXX/cxx20-p0388-unbound-ary.cpp
114–125

Btw, thank you for the excellent commenting in this test case -- this is the sort of thing that makes the test easier for everyone to understand.

This revision is now accepted and ready to land.Jun 21 2021, 11:56 AM
urnathan marked 2 inline comments as done.Jun 22 2021, 10:22 AM
urnathan added inline comments.
clang/test/SemaCXX/cxx20-p0388-unbound-ary.cpp
114–125

I can't really claim credit, these come from the paper :)

urnathan updated this revision to Diff 353697.Jun 22 2021, 10:23 AM
urnathan marked an inline comment as done.

This might need a bit of reworking, depending on how the p0388 pre-patch works out.

rsmith added inline comments.Jun 23 2021, 2:25 PM
clang/lib/Sema/SemaOverload.cpp
3843

(And similarly later.)

5081

Can we give this a more descriptive name? It's not obvious to me what CTT means, and I can't work it out from the type or the initializer. Same for OIA. I think this is CompletedToType and InferredArrayBound or something like that?

rsmith added inline comments.Jun 23 2021, 2:46 PM
clang/test/SemaCXX/cxx20-p0388-unbound-ary.cpp
141

Perhaps it'd be useful to include a test that demonstrates that brace elision does not work with the rules from P0388. Example:

struct A { int x, y; };
void f(int (&&)[]); // #1
void f(A (&&)[]); // #2
void g() {
  f({1, 2, 3, 4, 5, 6});
}

Here, the "prefer to initialize fewer elements" rule would pick #2 if brace elision were supported by the overload resolution rule for list initialization sequences for arrays, but we pick #1 instead because #2 appears to be non-viable, even though a call to #2 would actually work:

struct A { int x, y; };
void f(A (&&)[]);
void g() {
    f({1, 2, 3, 4, 5, 6}); // error
    (&f)({1, 2, 3, 4, 5, 6}); // OK! No overload resolution to mess things up here.
}
urnathan updated this revision to Diff 372911.Sep 16 2021, 5:55 AM
urnathan marked an inline comment as done.

Added additional testcases

urnathan marked an inline comment as done.Sep 16 2021, 5:56 AM
This revision was landed with ongoing or failed builds.Oct 12 2021, 7:35 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 12 2021, 7:35 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript