[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:23:38 PDT 2020
Sorry, the repro link should've pointed to
https://bugs.chromium.org/p/chromium/issues/detail?id=1110981#c22 which has
a nicer stack.
On Fri, Jul 31, 2020 at 7:22 PM Nico Weber <thakis at chromium.org> wrote:
> 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/37283206/attachment-0001.html>
More information about the cfe-commits
mailing list