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