[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Tyker via Phabricator via cfe-commits]

Tyker added inline comments.


================
Comment at: clang/include/clang/AST/DeclCXX.h:2579
+    assert(
+        !ES.getExpr() ||
+        CXXConstructorDeclBits.HasTrailingExplicitSpecifier &&
----------------
Rakete1111 wrote:
> Your or needs parens or the disambiguation is wrong.
i don't understand. but there is no need for disambiguation, `a && true == a` so this will work regardless of operator priority.


================
Comment at: clang/include/clang/Serialization/ASTReader.h:2435
+    uint64_t Kind = readInt();
+    bool hasExpr = Kind & 0x1;
+    Kind = Kind >> 1;
----------------
Rakete1111 wrote:
> same here.
what is the issue


================
Comment at: clang/lib/Sema/SemaInit.cpp:9361
         // Only consider converting constructors.
-        if (GD->isExplicit())
+        if (!GD->isMaybeNotExplicit())
           continue;
----------------
rsmith wrote:
> Tyker wrote:
> > rsmith wrote:
> > > Tyker wrote:
> > > > rsmith wrote:
> > > > > We need to substitute into the deduction guide first to detect whether it forms a "converting constructor", and that will need to be done inside `AddTemplateOverloadCandidate`.
> > > > similarly as the previous if. this check removes deduction guide that are already resolve to be explicit when we are in a context that doesn't allow explicit.
> > > > every time the explicitness was checked before my change i replaced it by a check that removes already resolved explicit specifiers.
> > > Unlike in the previous case, we do //not// yet pass an `AllowExplicit` flag into `AddTemplateOverloadCandidate` here, so this will incorrectly allow dependent //explicit-specifier//s that evaluate to `true` in copy-initialization contexts.
> > the default value for `AllowExplicit` is false. so the conversion will be removed in `AddTemplateOverloadCandidate`.
> That doesn't sound right: that'd mean we never use explicit deduction guides (we never pass `AllowExplicit = true` to `AddTemplateOverloadCandidate`). Do we have any test coverage that demonstrates that conditionally-explicit deduction guides are handled properly?
my mistake. the default value for AllowExplicit is false. but AddTemplateOverloadCandidate will only remove conversions and constructors. dependent explicit specifier that are resolved to true on deduction guides are removed at line 9480. they are not removed from the overload set. CTAD just fails if a explicit deduction guide is selected in a CopyInitList. i agree this is weird but the behavior is the same as before the patch.
the result on clang is:
```
template<typename T>
struct A {
  explicit A(T);
};
A a = 0; // error with explicit constructor meaning CTAD succeed.
A a = { 0 }; // error with explicit deduction guide
```
all compiler don't agree on this https://godbolt.org/z/ElHlkE. icc and clang have this behavior, gcc and msvc fail at CTAD time on both initialization. as of what the standard say from what i read, it isn't clear, the standard is clear when calling an explicit constructor should fail. but it doesn't appear to say for deduction guides.
as this was the previous behavior i did not change it with explicit(bool).


================
Comment at: clang/test/CXX/temp/temp.deduct.guide/p1.cpp:74
 virtual A(int(&)[28]) -> A<int>; // expected-error {{'virtual' can only appear on non-static member functions}}
-const A(int(&)[28]) -> A<int>; // expected-error {{deduction guide cannot be declared 'const'}}
+const A(int(&)[31]) -> A<int>; // expected-error {{deduction guide cannot be declared 'const'}}
 
----------------
Rakete1111 wrote:
> Is there a reason why you changed this?
yes. One of change that was made is making deduction guide redeclaration be an error. Without this change, this line was a redeclartion so the test didn't serve its purpose.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60934/new/

https://reviews.llvm.org/D60934