[clang] 740a164 - PR46377: Fix dependence calculation for function types and typedef
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 31 17:20:42 PDT 2020
Thanks, was just an overzealous assertion. Should be fixed in
llvmorg-12-init-1765-g234f51a65a4.
On Fri, 31 Jul 2020 at 16:23, Nico Weber <thakis at chromium.org> wrote:
> 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/50e65d06/attachment-0001.html>
More information about the cfe-commits
mailing list