Tyker added inline comments.
================
Comment at: clang/include/clang/AST/DeclCXX.h:2033
+
+ void setExplicitSpecifier(ExplicitSpecInfo ESI);
+
----------------
Tyker wrote:
> Tyker wrote:
> > rsmith wrote:
> > > Generally we don't want to have setters in the AST; the AST is intended
> > > to be immutable after creation. Is this necessary?
> > this is used in 2 cases:
> > - for deserialization.
> > - for in `Sema::ActOnFunctionDeclarator` to make every declaration of the
> > same function have the same explicit specifier as the canonical one. there
> > is probably a better way to do this but i didn't find how.
> >
> > the second use case will need to be changed because the constructor's
> > explicit specifier will be tail-allocated so the explicit specifier from
> > the canonical declaration will need to be recovered before construction to
> > allocate storage.
> >
> > how can we find the canonical declaration of a function before it is
> > constructed ?
> i found a solution around that issue.
> now setExplicitSpecifier is only used for deserialization.
as it is only used by deserialization and ASTDeclReader is friend to all class
with a setExplicitSpecifier.
I made setExplicitSpecifier private.
================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6174
CXXConversionDecl)) {
- return Node.isExplicit();
+ return Node.getExplicitSpecifier().isSpecified();
}
----------------
rsmith wrote:
> This will match `explicit(false)` constructors. I think
> `getExplicitSpecifier().isExplicit()` would be better, but please also add a
> FIXME here: it's not clear whether this should match a dependent
> `explicit(....)`.
shouldn't this be able to match with deduction guides too ?
================
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:
> > > 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`.
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
