<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>