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

[Tyker via Phabricator via cfe-commits]

Tyker added inline comments.


================
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:
> > > > > 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).
> > the standard is clear when calling an explicit constructor should fail. but it doesn't appear to say for deduction guides
> 
> The standard says that you take the set of deduction guides and notionally form a set of constructors from them (see [over.match.class.deduct]). So the constructor rules apply to deduction guides too.
> 
> > as this was the previous behavior i did not change it with explicit(bool).
> 
> I don't think that's correct. We filter out explicit deduction guides for non-list copy-initialization on line ~9239 (prior to this change). Today we get this result:
> 
> ```
> template<typename T> struct X { X(int); };
> 
> explicit X(int) -> X<int>; // #1
> 
> X a = 0; // error: no viable deduction guide, #1 is not a candidate
> X b = {0}; // error: selected explicit deduction guide #1
> X c{0}; // ok
> X d(0); // ok
> ```
> 
> ... which is correct. If we change the deduction guide to have a dependent `explicit(bool)` specifier:
> 
> ```
> template<bool E = true>
> explicit(E) X(int) -> X<int>;
> ```
> 
> ... we should get the same result, but I think we won't get that result with this patch: I think we'll incorrectly select an explicit deduction guide for the declaration of `a`, because we never filter out explicit deduction guides.
> 
> `DeduceTemplateSpecializationFromInitializer` should compute an `AllowExplicit` flag (`= !Kind.isCopyInit() || ListInit`), pass it into `AddTemplateOverloadCandidate`, and that should filter out explicit deduction guide specializations if it's `true`.
there was an issue. now fixed.



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

https://reviews.llvm.org/D60934