[clang] 740a164 - PR46377: Fix dependence calculation for function types and typedef
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 31 16:22:50 PDT 2020
Heads-up: This causes Chromium's build to fail with
clang-cl:
/usr/local/google/home/thakis/src/chrome/src/third_party/llvm/clang/lib/AST/ASTContext.cpp:4823:
clang::QualType clang::ASTContext::getPackExpansionType(clang::QualType,
llvm::Optional<unsigned int>, bool): Assertion `(!ExpectPackInType ||
Pattern->containsUnexpandedParameterPack()) && "Pack expansions must expand
one or more parameter packs"' failed.
under certain scenarios with precompiled headers. At the moment I only have
a repro when we use a Chromium-specific compiler plugin (
https://bugs.chromium.org/p/chromium/issues/detail?id=1110981#c19), but
it's likely I'll have one without that soonish.
On Tue, Jul 28, 2020 at 4:23 PM Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
>
> Author: Richard Smith
> Date: 2020-07-28T13:23:13-07:00
> New Revision: 740a164dec483225cbd02ab6c82199e2747ffacb
>
> URL:
> https://github.com/llvm/llvm-project/commit/740a164dec483225cbd02ab6c82199e2747ffacb
> DIFF:
> https://github.com/llvm/llvm-project/commit/740a164dec483225cbd02ab6c82199e2747ffacb.diff
>
> LOG: PR46377: Fix dependence calculation for function types and typedef
> types.
>
> We previously did not treat a function type as dependent if it had a
> parameter pack with a non-dependent type -- such a function type depends
> on the arity of the pack so is dependent even though none of the
> parameter types is dependent. In order to properly handle this, we now
> treat pack expansion types as always being dependent types (depending on
> at least the pack arity), and always canonically being pack expansion
> types, even in the unusual case when the pattern is not a dependent
> type. This does mean that we can have canonical types that are pack
> expansions that contain no unexpanded packs, which is unfortunate but
> not inaccurate.
>
> We also previously did not treat a typedef type as
> instantiation-dependent if its canonical type was not
> instantiation-dependent. That's wrong because instantiation-dependence
> is a property of the type sugar, not of the type; an
> instantiation-dependent type can have a non-instantiation-dependent
> canonical type.
>
> Added:
> clang/test/SemaTemplate/alias-template-nondependent.cpp
>
> Modified:
> clang/include/clang/AST/ASTContext.h
> clang/include/clang/AST/Type.h
> clang/include/clang/Basic/TypeNodes.td
> clang/lib/AST/ASTContext.cpp
> clang/lib/AST/Type.cpp
> clang/lib/CodeGen/CGDebugInfo.cpp
> clang/lib/CodeGen/CodeGenFunction.cpp
> clang/lib/Sema/SemaExpr.cpp
> clang/lib/Sema/SemaLambda.cpp
> clang/lib/Sema/SemaTemplateVariadic.cpp
> clang/lib/Sema/SemaType.cpp
>
> Removed:
>
>
>
>
> ################################################################################
> diff --git a/clang/include/clang/AST/ASTContext.h
> b/clang/include/clang/AST/ASTContext.h
> index 59e2679ddded..6c00fe86f282 100644
> --- a/clang/include/clang/AST/ASTContext.h
> +++ b/clang/include/clang/AST/ASTContext.h
> @@ -1459,8 +1459,16 @@ class ASTContext : public
> RefCountedBase<ASTContext> {
> void getInjectedTemplateArgs(const TemplateParameterList *Params,
> SmallVectorImpl<TemplateArgument> &Args);
>
> + /// Form a pack expansion type with the given pattern.
> + /// \param NumExpansions The number of expansions for the pack, if
> known.
> + /// \param ExpectPackInType If \c false, we should not expect \p
> Pattern to
> + /// contain an unexpanded pack. This only makes sense if the pack
> + /// expansion is used in a context where the arity is inferred
> from
> + /// elsewhere, such as if the pattern contains a placeholder
> type or
> + /// if this is the canonical type of another pack expansion type.
> QualType getPackExpansionType(QualType Pattern,
> - Optional<unsigned> NumExpansions);
> + Optional<unsigned> NumExpansions,
> + bool ExpectPackInType = true);
>
> QualType getObjCInterfaceType(const ObjCInterfaceDecl *Decl,
> ObjCInterfaceDecl *PrevDecl = nullptr)
> const;
>
> diff --git a/clang/include/clang/AST/Type.h
> b/clang/include/clang/AST/Type.h
> index 9a745ef20fac..7fe652492b0e 100644
> --- a/clang/include/clang/AST/Type.h
> +++ b/clang/include/clang/AST/Type.h
> @@ -4383,11 +4383,7 @@ class TypedefType : public Type {
> protected:
> friend class ASTContext; // ASTContext creates these.
>
> - TypedefType(TypeClass tc, const TypedefNameDecl *D, QualType can)
> - : Type(tc, can, can->getDependence() &
> ~TypeDependence::UnexpandedPack),
> - Decl(const_cast<TypedefNameDecl *>(D)) {
> - assert(!isa<TypedefType>(can) && "Invalid canonical type");
> - }
> + TypedefType(TypeClass tc, const TypedefNameDecl *D, QualType can);
>
> public:
> TypedefNameDecl *getDecl() const { return Decl; }
> @@ -5624,7 +5620,8 @@ class PackExpansionType : public Type, public
> llvm::FoldingSetNode {
> PackExpansionType(QualType Pattern, QualType Canon,
> Optional<unsigned> NumExpansions)
> : Type(PackExpansion, Canon,
> - (Pattern->getDependence() | TypeDependence::Instantiation) &
> + (Pattern->getDependence() | TypeDependence::Dependent |
> + TypeDependence::Instantiation) &
> ~TypeDependence::UnexpandedPack),
> Pattern(Pattern) {
> PackExpansionTypeBits.NumExpansions =
> @@ -5645,8 +5642,8 @@ class PackExpansionType : public Type, public
> llvm::FoldingSetNode {
> return None;
> }
>
> - bool isSugared() const { return !Pattern->isDependentType(); }
> - QualType desugar() const { return isSugared() ? Pattern :
> QualType(this, 0); }
> + bool isSugared() const { return false; }
> + QualType desugar() const { return QualType(this, 0); }
>
> void Profile(llvm::FoldingSetNodeID &ID) {
> Profile(ID, getPattern(), getNumExpansions());
>
> diff --git a/clang/include/clang/Basic/TypeNodes.td
> b/clang/include/clang/Basic/TypeNodes.td
> index a4e3002b9075..011394c3ef45 100644
> --- a/clang/include/clang/Basic/TypeNodes.td
> +++ b/clang/include/clang/Basic/TypeNodes.td
> @@ -100,7 +100,7 @@ def DeducedTemplateSpecializationType :
> TypeNode<DeducedType>;
> def InjectedClassNameType : TypeNode<Type>, AlwaysDependent, LeafType;
> def DependentNameType : TypeNode<Type>, AlwaysDependent;
> def DependentTemplateSpecializationType : TypeNode<Type>, AlwaysDependent;
> -def PackExpansionType : TypeNode<Type>, NeverCanonicalUnlessDependent;
> +def PackExpansionType : TypeNode<Type>, AlwaysDependent;
> def ObjCTypeParamType : TypeNode<Type>, NeverCanonical;
> def ObjCObjectType : TypeNode<Type>;
> def ObjCInterfaceType : TypeNode<ObjCObjectType>, LeafType;
>
> diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
> index e7518a538fe6..25bf71519d1c 100644
> --- a/clang/lib/AST/ASTContext.cpp
> +++ b/clang/lib/AST/ASTContext.cpp
> @@ -4817,37 +4817,27 @@ ASTContext::getInjectedTemplateArgs(const
> TemplateParameterList *Params,
> }
>
> QualType ASTContext::getPackExpansionType(QualType Pattern,
> - Optional<unsigned>
> NumExpansions) {
> + Optional<unsigned>
> NumExpansions,
> + bool ExpectPackInType) {
> + assert((!ExpectPackInType ||
> Pattern->containsUnexpandedParameterPack()) &&
> + "Pack expansions must expand one or more parameter packs");
> +
> llvm::FoldingSetNodeID ID;
> PackExpansionType::Profile(ID, Pattern, NumExpansions);
>
> - // A deduced type can deduce to a pack, eg
> - // auto ...x = some_pack;
> - // That declaration isn't (yet) valid, but is created as part of
> building an
> - // init-capture pack:
> - // [...x = some_pack] {}
> - assert((Pattern->containsUnexpandedParameterPack() ||
> - Pattern->getContainedDeducedType()) &&
> - "Pack expansions must expand one or more parameter packs");
> void *InsertPos = nullptr;
> - PackExpansionType *T
> - = PackExpansionTypes.FindNodeOrInsertPos(ID, InsertPos);
> + PackExpansionType *T = PackExpansionTypes.FindNodeOrInsertPos(ID,
> InsertPos);
> if (T)
> return QualType(T, 0);
>
> QualType Canon;
> if (!Pattern.isCanonical()) {
> - Canon = getCanonicalType(Pattern);
> - // The canonical type might not contain an unexpanded parameter pack,
> if it
> - // contains an alias template specialization which ignores one of its
> - // parameters.
> - if (Canon->containsUnexpandedParameterPack()) {
> - Canon = getPackExpansionType(Canon, NumExpansions);
> -
> - // Find the insert position again, in case we inserted an element
> into
> - // PackExpansionTypes and invalidated our insert position.
> - PackExpansionTypes.FindNodeOrInsertPos(ID, InsertPos);
> - }
> + Canon = getPackExpansionType(getCanonicalType(Pattern), NumExpansions,
> + /*ExpectPackInType=*/false);
> +
> + // Find the insert position again, in case we inserted an element into
> + // PackExpansionTypes and invalidated our insert position.
> + PackExpansionTypes.FindNodeOrInsertPos(ID, InsertPos);
> }
>
> T = new (*this, TypeAlignment)
>
> diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
> index 0122d2e7de52..d40ba4c648c4 100644
> --- a/clang/lib/AST/Type.cpp
> +++ b/clang/lib/AST/Type.cpp
> @@ -1187,9 +1187,6 @@ struct SimpleTransformVisitor : public
> TypeVisitor<Derived, QualType> {
> T->getTypeConstraintArguments());
> }
>
> - // FIXME: Non-trivial to implement, but important for C++
> - SUGARED_TYPE_CLASS(PackExpansion)
> -
> QualType VisitObjCObjectType(const ObjCObjectType *T) {
> QualType baseType = recurse(T->getBaseType());
> if (baseType.isNull())
> @@ -3348,6 +3345,12 @@ void
> FunctionProtoType::Profile(llvm::FoldingSetNodeID &ID,
> getExtProtoInfo(), Ctx, isCanonicalUnqualified());
> }
>
> +TypedefType::TypedefType(TypeClass tc, const TypedefNameDecl *D, QualType
> can)
> + : Type(tc, can, D->getUnderlyingType()->getDependence()),
> + Decl(const_cast<TypedefNameDecl *>(D)) {
> + assert(!isa<TypedefType>(can) && "Invalid canonical type");
> +}
> +
> QualType TypedefType::desugar() const {
> return getDecl()->getUnderlyingType();
> }
>
> diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp
> b/clang/lib/CodeGen/CGDebugInfo.cpp
> index 6965c4a1209c..780e0c692c05 100644
> --- a/clang/lib/CodeGen/CGDebugInfo.cpp
> +++ b/clang/lib/CodeGen/CGDebugInfo.cpp
> @@ -3252,7 +3252,6 @@ llvm::DIType *CGDebugInfo::CreateTypeNode(QualType
> Ty, llvm::DIFile *Unit) {
> case Type::TypeOf:
> case Type::Decltype:
> case Type::UnaryTransform:
> - case Type::PackExpansion:
> break;
> }
>
>
> diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp
> b/clang/lib/CodeGen/CodeGenFunction.cpp
> index 8ce488f35dd3..8f79cc77f0e6 100644
> --- a/clang/lib/CodeGen/CodeGenFunction.cpp
> +++ b/clang/lib/CodeGen/CodeGenFunction.cpp
> @@ -2075,7 +2075,6 @@ void
> CodeGenFunction::EmitVariablyModifiedType(QualType type) {
> case Type::UnaryTransform:
> case Type::Attributed:
> case Type::SubstTemplateTypeParm:
> - case Type::PackExpansion:
> case Type::MacroQualified:
> // Keep walking after single level desugaring.
> type = type.getSingleStepDesugaredType(getContext());
>
> diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
> index 21d3bbf419a9..bb0b1fa49851 100644
> --- a/clang/lib/Sema/SemaExpr.cpp
> +++ b/clang/lib/Sema/SemaExpr.cpp
> @@ -4345,7 +4345,6 @@ static void captureVariablyModifiedType(ASTContext
> &Context, QualType T,
> case Type::UnaryTransform:
> case Type::Attributed:
> case Type::SubstTemplateTypeParm:
> - case Type::PackExpansion:
> case Type::MacroQualified:
> // Keep walking after single level desugaring.
> T = T.getSingleStepDesugaredType(Context);
>
> diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
> index 657ed13f207a..dc74f6e2f7dc 100644
> --- a/clang/lib/Sema/SemaLambda.cpp
> +++ b/clang/lib/Sema/SemaLambda.cpp
> @@ -803,7 +803,8 @@ QualType Sema::buildLambdaInitCaptureInitialization(
> Diag(EllipsisLoc, getLangOpts().CPlusPlus20
> ? diag::warn_cxx17_compat_init_capture_pack
> : diag::ext_init_capture_pack);
> - DeductType = Context.getPackExpansionType(DeductType,
> NumExpansions);
> + DeductType = Context.getPackExpansionType(DeductType, NumExpansions,
> +
> /*ExpectPackInType=*/false);
>
> TLB.push<PackExpansionTypeLoc>(DeductType).setEllipsisLoc(EllipsisLoc);
> } else {
> // Just ignore the ellipsis for now and form a non-pack variable.
> We'll
>
> diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp
> b/clang/lib/Sema/SemaTemplateVariadic.cpp
> index 7b77d1cb482a..259cc5165776 100644
> --- a/clang/lib/Sema/SemaTemplateVariadic.cpp
> +++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
> @@ -614,7 +614,8 @@ QualType Sema::CheckPackExpansion(QualType Pattern,
> SourceRange PatternRange,
> return QualType();
> }
>
> - return Context.getPackExpansionType(Pattern, NumExpansions);
> + return Context.getPackExpansionType(Pattern, NumExpansions,
> + /*ExpectPackInType=*/false);
> }
>
> ExprResult Sema::ActOnPackExpansion(Expr *Pattern, SourceLocation
> EllipsisLoc) {
>
> diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
> index 3eabe7ca6ffe..4c7eece68bca 100644
> --- a/clang/lib/Sema/SemaType.cpp
> +++ b/clang/lib/Sema/SemaType.cpp
> @@ -5516,7 +5516,7 @@ static TypeSourceInfo
> *GetFullTypeForDeclarator(TypeProcessingState &state,
> << T << D.getSourceRange();
> D.setEllipsisLoc(SourceLocation());
> } else {
> - T = Context.getPackExpansionType(T, None);
> + T = Context.getPackExpansionType(T, None,
> /*ExpectPackInType=*/false);
> }
> break;
> case DeclaratorContext::TemplateParamContext:
>
> diff --git a/clang/test/SemaTemplate/alias-template-nondependent.cpp
> b/clang/test/SemaTemplate/alias-template-nondependent.cpp
> new file mode 100644
> index 000000000000..e8ea16483a09
> --- /dev/null
> +++ b/clang/test/SemaTemplate/alias-template-nondependent.cpp
> @@ -0,0 +1,24 @@
> +// RUN: %clang_cc1 -std=c++20 -verify %s
> +
> +namespace PR46377 {
> + template<typename> using IntPtr = int*;
> + template<typename ...T> auto non_dependent_typedef() {
> + typedef int(*P)(IntPtr<T>...);
> + return P();
> + }
> + template<typename ...T> auto non_dependent_alias() {
> + using P = int(*)(IntPtr<T>...);
> + return P();
> + }
> + template<typename ...T> auto non_dependent_via_sizeof() {
> + using P = int(*)(int(...pack)[sizeof(sizeof(T))]); // expected-error
> {{invalid application of 'sizeof'}}
> + return P();
> + }
> +
> + using a = int (*)(int*, int*);
> + using a = decltype(non_dependent_typedef<void, void>());
> + using a = decltype(non_dependent_alias<void, void>());
> + using a = decltype(non_dependent_via_sizeof<float, float>());
> +
> + using b = decltype(non_dependent_via_sizeof<float, void>()); //
> expected-note {{instantiation of}}
> +}
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200731/4e95eb32/attachment-0001.html>
More information about the cfe-commits
mailing list