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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
