Mordante marked 6 inline comments as done.
Mordante added inline comments.
================
Comment at: clang/test/CXX/drs/dr14xx.cpp:411-414
+ void f(const char[4]);
+ void f(const wchar_t[4]);
+ void f(const char16_t[4]);
+ void f(const char32_t[4]);
----------------
rsmith wrote:
> Mordante wrote:
> > rsmith wrote:
> > > rsmith wrote:
> > > > Mordante wrote:
> > > > > rsmith wrote:
> > > > > > These should presumably be references to arrays, rather than
> > > > > > arrays, or the parameter type is as if you wrote (for example)
> > > > > > `void f(const char *)`, which shouldn't get the special treatment
> > > > > > here.
> > > > > >
> > > > > > [over.ics.list]p4 mentions this in its footnote:
> > > > > >
> > > > > > "Otherwise, if the parameter type is a character array [Footnote:
> > > > > > Since there are no parameters of array type, this will only occur
> > > > > > as the referenced type of a reference parameter.] and the
> > > > > > initializer list has a single element that is an
> > > > > > appropriately-typed string-literal (9.4.3), the implicit conversion
> > > > > > sequence is the identity conversion."
> > > > > Ah I missed that footnote. I reread the standard and can you confirm
> > > > > some cases?
> > > > > ```
> > > > > namespace A {
> > > > > void f(const char(&)[4]);
> > > > > void g() { f({"abc"}); }
> > > > > }
> > > > > namespace B {
> > > > > void f(const char(&&)[4]);
> > > > > void g() { f({"abc"}); }
> > > > > }
> > > > > ```
> > > > > both should work and with P0388 the array without bounds will also
> > > > > work.
> > > > >
> > > > > I ask since this is my interpretation of the standard, but it seems
> > > > > there's a difference between implementations and `void f(const
> > > > > char(&&)[4]);` fails for "abc" but works for "ab".
> > > > > It seems ICC and consider "abc" an lvalue in this case and not when
> > > > > using "ab".
> > > > >
> > > > > Here's a gotbolt link with the examples https://godbolt.org/z/r1TKfx
> > > > That's a really interesting example :)
> > > >
> > > > The initializer is a list containing an lvalue of type `const char[4]`.
> > > > Per [dcl.init.list]/3.9 and /3.10, the behavior depends on whether the
> > > > referenced type is reference-related to `const char[4]` -- if so, then
> > > > the reference can only bind directly and a `&&` reference will be
> > > > invalid, because it's binding an rvalue reference to an lvalue, and if
> > > > not, then we create an array temporary and the `&&` binding is fine.
> > > >
> > > > Per [dcl.init.ref]/4, `const char[???]` is reference-related to `const
> > > > char[4]` if they are similar types, and per [conv.qual]/2, the types
> > > > are similar if `???` is omitted or `4`, and not similar otherwise.
> > > >
> > > > So:
> > > > * `const char (&&r)[4] = {"abc"}` is ill-formed (types are the same,
> > > > binds rvalue reference to lvalue)
> > > > * `const char (&&)[] = {"abc"}` is ill-formed (types are similar, binds
> > > > rvalue reference to lvalue)
> > > > * `const char (&&r)[5] = {"abc"}` is OK (types are not similar, creates
> > > > temporary)
> > > >
> > > > That seems like a very surprising outcome to me. Perhaps we should
> > > > probably ignore array bounds entirely when determining whether two
> > > > types are reference-related. I'll take this to CWG for discussion.
> > > I think that's only an answer to half of your question. The other half is
> > > that [over.ics.list]p4 does not apply (directly) to either of your
> > > testcases, because the "parameter type" is a reference type, not an array
> > > type. For:
> > >
> > > ```
> > > namespace A {
> > > void f(const char(&)[4]);
> > > void g() { f({"abc"}); }
> > > }
> > > ```
> > >
> > > ... we reach [over.ics.list]p9, which says to use the rules in
> > > [over.ics.ref]. Those rules say that if the reference binds directly to
> > > an argument expression (ignore the "expression" here; this is very old
> > > wording that predates braced initializers), then we form an identity
> > > conversion. So that's what happens in this case.
> > >
> > > For
> > >
> > > ```
> > > namespace B {
> > > void f(const char(&&)[4]);
> > > void g() { f({"abc"}); }
> > > }
> > > ```
> > >
> > > the same thing happens, but now [over.ics.ref]p3 says "an implicit
> > > conversion sequence cannot be formed if it requires binding an lvalue
> > > reference other than a reference to a non-volatile const type to an
> > > rvalue or binding an rvalue reference to an lvalue other than a function
> > > lvalue", so the candidate is not viable.
> > >
> > > If the array bound were omitted or were not 4, then the reference would
> > > not bind directly, and we would instead consider initializing a
> > > temporary. [over.ics.ref]p2 says "When a parameter of reference type is
> > > not bound directly to an argument expression, the conversion sequence is
> > > the one required to convert the argument expression to the referenced
> > > type according to 12.4.4.2.", which takes us back around to
> > > [over.ics.list] with the "parameter type" now being the array type.
> > > That's how we can reach [over.ics.list]p4 and consider string literal ->
> > > array initialization.
> > >
> > > So I think that, according to the current rules, for
> > >
> > > ```
> > > void f(const char (&&)[4]); // #1
> > > void f(const char (&&)[5]); // #2
> > > ```
> > >
> > > ... a call to `f({"abc"})` remarkably calls #2, because #1 is not viable
> > > (would bind rvalue reference to lvalue string literal), but #2 is, just
> > > like `const char (&&r)[4] = {"abc"};` is ill-formed but `const char
> > > (&&r)[5] = {"abc"};` is valid.
> > > That's a really interesting example :)
> >
> > Thanks :-)
> > Also thanks for the detailed explanation of the rules applying to these
> > cases.
> >
> > > So:
> > > * `const char (&&r)[4] = {"abc"}` is ill-formed (types are the same,
> > > binds rvalue reference to lvalue)
> > > * `const char (&&)[] = {"abc"}` is ill-formed (types are similar, binds
> > > rvalue reference to lvalue)
> >
> > Interesting, wasn't P0388 trying to solve the issues with array of unknown
> > bound?
> > Am I correct to assume it's intended that this code will become valid in
> > the future?
> >
> > > * `const char (&&r)[5] = {"abc"}` is OK (types are not similar, creates
> > > temporary)
> > >
> > > That seems like a very surprising outcome to me. Perhaps we should
> > > probably ignore array bounds entirely when determining whether two types
> > > are reference-related. I'll take this to CWG for discussion.
> >
> > I agree this seems unexpected behaviour, thanks for discussing it with CWG.
> >
> > > So I think that, according to the current rules, for
> >
> > > void f(const char (&&)[4]); // #1
> > > void f(const char (&&)[5]); // #2
> >
> > > ... a call to f({"abc"}) remarkably calls #2, because #1 is not viable
> > > (would bind rvalue reference to lvalue string literal), but #2 is, just
> > > like const char (&&r)[4] = {"abc"}; is ill-formed but const char (&&r)[5]
> > > = {"abc"}; is valid.
> >
> > MSVC agrees with your observation, GCC picks #1 and fails to create the
> > call, and ICC aborts with an ICE
> > https://godbolt.org/z/s7nTPP
> >
> >
> >
> > Am I correct to assume it's intended that this code will become valid in
> > the future?
>
> I think it's more likely that the corresponding cases with other array bounds
> will become ill-formed. String literals are lvalues, so binding an rvalue
> reference to a string literal should not be valid based on our normal
> principles; allowing the wrong-array-bound case is then pretty surprising
> emergent behavior of the form that we usually disallow (on the basis that it
> "looks sufficiently like" the intention was for the reference to bind
> directly, so it's an error if the reference can't bind directly).
>
> I think the likely outcomes are either that we ignore array bounds when
> determining reference-relatedness, or that we always create a temporary when
> binding an rvalue-reference-to-char-array to a string literal, regardless of
> whether the bound matches. Or maybe that we set the array bound of the string
> literal to that of the reference-to-array, allowing the `const &` cases but
> not the `&&` cases with an incorrect bound. But I wouldn't want to guess
> which.
Thanks for your insight.
================
Comment at: clang/test/SemaCXX/overload-array-size.cpp:6-13
+void f(const char(&&)[4]); // #1
+void f(const char(&&)[5]); // #2
+void g() {
+ f({"abc"});
+ // CHECK: ExprWithCleanups {{.*}} <line:[[@LINE-1]]:{{.*}}> 'void'
+ // CHECK-NEXT: CallExpr
+ // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(const char (&&)[5])'
----------------
rsmith wrote:
> It's more common to test such things via diagnostics rather than by AST
> dumps. (AST dump tests tend to be more fragile in practice.) You could mark
> the `[5]` case as `=delete` and check that you get the "call to a deleted
> function" diagnostic, or you could change the `[5]` case to return `int &`
> and check that you can initialize an `int &` from a call to it, or something
> like that.
I'm also not too fond of the AST checks, but forgot I could use `= delete` to
detect whether the wanted overload is tested. Since I adjusted the other tests
as suggested this test has no additional value and I removed it.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87561/new/
https://reviews.llvm.org/D87561
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
