From: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Wed, 23 Sep 2015 21:41:42 +0000 (+0000)
Subject: PR14858: Initial support for proper sizeof... handling within alias templates.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1878f91a0aaf74e1424ca10061ee2785b1c619e9;p=clang

PR14858: Initial support for proper sizeof... handling within alias templates.
This doesn't quite get alias template equivalence right yet, but handles the
egregious cases where we would silently give the wrong answers.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@248431 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/clang/AST/ExprCXX.h b/include/clang/AST/ExprCXX.h
index 2acb3e4cac..cf1f86ebfa 100644
--- a/include/clang/AST/ExprCXX.h
+++ b/include/clang/AST/ExprCXX.h
@@ -3558,43 +3558,51 @@ class SizeOfPackExpr : public Expr {
 
   /// \brief The length of the parameter pack, if known.
   ///
-  /// When this expression is value-dependent, the length of the parameter pack
-  /// is unknown. When this expression is not value-dependent, the length is
-  /// known.
+  /// When this expression is not value-dependent, this is the length of
+  /// the pack. When the expression was parsed rather than instantiated
+  /// (and thus is value-dependent), this is zero.
+  ///
+  /// After partial substitution into a sizeof...(X) expression (for instance,
+  /// within an alias template or during function template argument deduction),
+  /// we store a trailing array of partially-substituted TemplateArguments,
+  /// and this is the length of that array.
   unsigned Length;
 
-  /// \brief The parameter pack itself.
+  /// \brief The parameter pack.
   NamedDecl *Pack;
 
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
 
-public:
-  /// \brief Create a value-dependent expression that computes the length of
-  /// the given parameter pack.
-  SizeOfPackExpr(QualType SizeType, SourceLocation OperatorLoc, NamedDecl *Pack,
-                 SourceLocation PackLoc, SourceLocation RParenLoc)
-    : Expr(SizeOfPackExprClass, SizeType, VK_RValue, OK_Ordinary,
-           /*TypeDependent=*/false, /*ValueDependent=*/true,
-           /*InstantiationDependent=*/true,
-           /*ContainsUnexpandedParameterPack=*/false),
-      OperatorLoc(OperatorLoc), PackLoc(PackLoc), RParenLoc(RParenLoc),
-      Length(0), Pack(Pack) { }
-
   /// \brief Create an expression that computes the length of
-  /// the given parameter pack, which is already known.
+  /// the given parameter pack.
   SizeOfPackExpr(QualType SizeType, SourceLocation OperatorLoc, NamedDecl *Pack,
                  SourceLocation PackLoc, SourceLocation RParenLoc,
-                 unsigned Length)
-  : Expr(SizeOfPackExprClass, SizeType, VK_RValue, OK_Ordinary,
-         /*TypeDependent=*/false, /*ValueDependent=*/false,
-         /*InstantiationDependent=*/false,
-         /*ContainsUnexpandedParameterPack=*/false),
-    OperatorLoc(OperatorLoc), PackLoc(PackLoc), RParenLoc(RParenLoc),
-    Length(Length), Pack(Pack) { }
+                 Optional<unsigned> Length, ArrayRef<TemplateArgument> PartialArgs)
+      : Expr(SizeOfPackExprClass, SizeType, VK_RValue, OK_Ordinary,
+             /*TypeDependent=*/false, /*ValueDependent=*/!Length,
+             /*InstantiationDependent=*/!Length,
+             /*ContainsUnexpandedParameterPack=*/false),
+        OperatorLoc(OperatorLoc), PackLoc(PackLoc), RParenLoc(RParenLoc),
+        Length(Length ? *Length : PartialArgs.size()), Pack(Pack) {
+    assert((!Length || PartialArgs.empty()) &&
+           "have partial args for non-dependent sizeof... expression");
+    TemplateArgument *Args = reinterpret_cast<TemplateArgument *>(this + 1);
+    std::uninitialized_copy(PartialArgs.begin(), PartialArgs.end(), Args);
+  }
 
   /// \brief Create an empty expression.
-  SizeOfPackExpr(EmptyShell Empty) : Expr(SizeOfPackExprClass, Empty) { }
+  SizeOfPackExpr(EmptyShell Empty, unsigned NumPartialArgs)
+      : Expr(SizeOfPackExprClass, Empty), Length(NumPartialArgs), Pack() {}
+
+public:
+  static SizeOfPackExpr *Create(ASTContext &Context, SourceLocation OperatorLoc,
+                                NamedDecl *Pack, SourceLocation PackLoc,
+                                SourceLocation RParenLoc,
+                                Optional<unsigned> Length = None,
+                                ArrayRef<TemplateArgument> PartialArgs = None);
+  static SizeOfPackExpr *CreateDeserialized(ASTContext &Context,
+                                            unsigned NumPartialArgs);
 
   /// \brief Determine the location of the 'sizeof' keyword.
   SourceLocation getOperatorLoc() const { return OperatorLoc; }
@@ -3618,6 +3626,23 @@ public:
     return Length;
   }
 
+  /// \brief Determine whether this represents a partially-substituted sizeof...
+  /// expression, such as is produced for:
+  ///
+  ///   template<typename ...Ts> using X = int[sizeof...(Ts)];
+  ///   template<typename ...Us> void f(X<Us..., 1, 2, 3, Us...>);
+  bool isPartiallySubstituted() const {
+    return isValueDependent() && Length;
+  }
+
+  /// \brief Get
+  ArrayRef<TemplateArgument> getPartialArguments() const {
+    assert(isPartiallySubstituted());
+    const TemplateArgument *Args =
+        reinterpret_cast<const TemplateArgument *>(this + 1);
+    return llvm::makeArrayRef(Args, Args + Length);
+  }
+
   SourceLocation getLocStart() const LLVM_READONLY { return OperatorLoc; }
   SourceLocation getLocEnd() const LLVM_READONLY { return RParenLoc; }
 
diff --git a/include/clang/AST/TemplateBase.h b/include/clang/AST/TemplateBase.h
index b1b79014fe..73b667ce1b 100644
--- a/include/clang/AST/TemplateBase.h
+++ b/include/clang/AST/TemplateBase.h
@@ -541,6 +541,10 @@ public:
     return Arguments.data();
   }
 
+  llvm::ArrayRef<TemplateArgumentLoc> arguments() const {
+    return Arguments;
+  }
+
   const TemplateArgumentLoc &operator[](unsigned I) const {
     return Arguments[I];
   }
diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp
index c95922b141..0c9c14ba30 100644
--- a/lib/AST/ASTDumper.cpp
+++ b/lib/AST/ASTDumper.cpp
@@ -2017,6 +2017,9 @@ void ASTDumper::VisitSizeOfPackExpr(const SizeOfPackExpr *Node) {
   VisitExpr(Node);
   dumpPointer(Node->getPack());
   dumpName(Node->getPack());
+  if (Node->isPartiallySubstituted())
+    for (const auto &A : Node->getPartialArguments())
+      dumpTemplateArgument(A);
 }
 
 
diff --git a/lib/AST/ExprCXX.cpp b/lib/AST/ExprCXX.cpp
index 56f720a978..0dd3671511 100644
--- a/lib/AST/ExprCXX.cpp
+++ b/lib/AST/ExprCXX.cpp
@@ -1432,6 +1432,25 @@ CXXRecordDecl *UnresolvedMemberExpr::getNamingClass() const {
   return Record;
 }
 
+SizeOfPackExpr *
+SizeOfPackExpr::Create(ASTContext &Context, SourceLocation OperatorLoc,
+                       NamedDecl *Pack, SourceLocation PackLoc,
+                       SourceLocation RParenLoc,
+                       Optional<unsigned> Length,
+                       ArrayRef<TemplateArgument> PartialArgs) {
+  void *Storage = Context.Allocate(
+      sizeof(SizeOfPackExpr) + sizeof(TemplateArgument) * PartialArgs.size());
+  return new (Storage) SizeOfPackExpr(Context.getSizeType(), OperatorLoc, Pack,
+                                      PackLoc, RParenLoc, Length, PartialArgs);
+}
+
+SizeOfPackExpr *SizeOfPackExpr::CreateDeserialized(ASTContext &Context,
+                                                   unsigned NumPartialArgs) {
+  void *Storage = Context.Allocate(
+      sizeof(SizeOfPackExpr) + sizeof(TemplateArgument) * NumPartialArgs);
+  return new (Storage) SizeOfPackExpr(EmptyShell(), NumPartialArgs);
+}
+
 SubstNonTypeTemplateParmPackExpr::
 SubstNonTypeTemplateParmPackExpr(QualType T, 
                                  NonTypeTemplateParmDecl *Param,
diff --git a/lib/AST/ItaniumMangle.cpp b/lib/AST/ItaniumMangle.cpp
index 9fafef13f6..64f9ccd996 100644
--- a/lib/AST/ItaniumMangle.cpp
+++ b/lib/AST/ItaniumMangle.cpp
@@ -3458,8 +3458,17 @@ recurse:
     break;
       
   case Expr::SizeOfPackExprClass: {
+    auto *SPE = cast<SizeOfPackExpr>(E);
+    if (SPE->isPartiallySubstituted()) {
+      Out << "sP";
+      for (const auto &A : SPE->getPartialArguments())
+        mangleTemplateArg(A);
+      Out << "E";
+      break;
+    }
+
     Out << "sZ";
-    const NamedDecl *Pack = cast<SizeOfPackExpr>(E)->getPack();
+    const NamedDecl *Pack = SPE->getPack();
     if (const TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(Pack))
       mangleTemplateParameter(TTP->getIndex());
     else if (const NonTypeTemplateParmDecl *NTTP
diff --git a/lib/AST/StmtProfile.cpp b/lib/AST/StmtProfile.cpp
index 231f450c9b..b379745b4a 100644
--- a/lib/AST/StmtProfile.cpp
+++ b/lib/AST/StmtProfile.cpp
@@ -1313,6 +1313,14 @@ void StmtProfiler::VisitPackExpansionExpr(const PackExpansionExpr *S) {
 void StmtProfiler::VisitSizeOfPackExpr(const SizeOfPackExpr *S) {
   VisitExpr(S);
   VisitDecl(S->getPack());
+  if (S->isPartiallySubstituted()) {
+    auto Args = S->getPartialArguments();
+    ID.AddInteger(Args.size());
+    for (const auto &TA : Args)
+      VisitTemplateArgument(TA);
+  } else {
+    ID.AddInteger(0);
+  }
 }
 
 void StmtProfiler::VisitSubstNonTypeTemplateParmPackExpr(
diff --git a/lib/Sema/SemaTemplateVariadic.cpp b/lib/Sema/SemaTemplateVariadic.cpp
index fd3ba3549b..a7fef8a29d 100644
--- a/lib/Sema/SemaTemplateVariadic.cpp
+++ b/lib/Sema/SemaTemplateVariadic.cpp
@@ -867,8 +867,8 @@ ExprResult Sema::ActOnSizeofParameterPackExpr(Scope *S,
 
   MarkAnyDeclReferenced(OpLoc, ParameterPack, true);
 
-  return new (Context) SizeOfPackExpr(Context.getSizeType(), OpLoc, 
-                                      ParameterPack, NameLoc, RParenLoc);
+  return SizeOfPackExpr::Create(Context, OpLoc, ParameterPack, NameLoc,
+                                RParenLoc);
 }
 
 TemplateArgumentLoc
diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h
index 1629138c5b..820fe65c6c 100644
--- a/lib/Sema/TreeTransform.h
+++ b/lib/Sema/TreeTransform.h
@@ -504,7 +504,8 @@ public:
   ///
   /// Returns true if there was an error.
   bool TransformTemplateArgument(const TemplateArgumentLoc &Input,
-                                 TemplateArgumentLoc &Output);
+                                 TemplateArgumentLoc &Output,
+                                 bool Uneval = false);
 
   /// \brief Transform the given set of template arguments.
   ///
@@ -526,8 +527,10 @@ public:
   /// Returns true if an error occurred.
   bool TransformTemplateArguments(const TemplateArgumentLoc *Inputs,
                                   unsigned NumInputs,
-                                  TemplateArgumentListInfo &Outputs) {
-    return TransformTemplateArguments(Inputs, Inputs + NumInputs, Outputs);
+                                  TemplateArgumentListInfo &Outputs,
+                                  bool Uneval = false) {
+    return TransformTemplateArguments(Inputs, Inputs + NumInputs, Outputs,
+                                      Uneval);
   }
 
   /// \brief Transform the given set of template arguments.
@@ -547,7 +550,8 @@ public:
   template<typename InputIterator>
   bool TransformTemplateArguments(InputIterator First,
                                   InputIterator Last,
-                                  TemplateArgumentListInfo &Outputs);
+                                  TemplateArgumentListInfo &Outputs,
+                                  bool Uneval = false);
 
   /// \brief Fakes up a TemplateArgumentLoc for a given TemplateArgument.
   void InventTemplateArgumentLoc(const TemplateArgument &Arg,
@@ -2615,18 +2619,14 @@ public:
   }
 
   /// \brief Build a new expression to compute the length of a parameter pack.
-  ExprResult RebuildSizeOfPackExpr(SourceLocation OperatorLoc, NamedDecl *Pack,
+  ExprResult RebuildSizeOfPackExpr(SourceLocation OperatorLoc,
+                                   NamedDecl *Pack,
                                    SourceLocation PackLoc,
                                    SourceLocation RParenLoc,
-                                   Optional<unsigned> Length) {
-    if (Length)
-      return new (SemaRef.Context) SizeOfPackExpr(SemaRef.Context.getSizeType(),
-                                                  OperatorLoc, Pack, PackLoc,
-                                                  RParenLoc, *Length);
-
-    return new (SemaRef.Context) SizeOfPackExpr(SemaRef.Context.getSizeType(),
-                                                OperatorLoc, Pack, PackLoc,
-                                                RParenLoc);
+                                   Optional<unsigned> Length,
+                                   ArrayRef<TemplateArgument> PartialArgs) {
+    return SizeOfPackExpr::Create(SemaRef.Context, OperatorLoc, Pack, PackLoc,
+                                  RParenLoc, Length, PartialArgs);
   }
 
   /// \brief Build a new Objective-C boxed expression.
@@ -3511,7 +3511,7 @@ void TreeTransform<Derived>::InventTemplateArgumentLoc(
 template<typename Derived>
 bool TreeTransform<Derived>::TransformTemplateArgument(
                                          const TemplateArgumentLoc &Input,
-                                         TemplateArgumentLoc &Output) {
+                                         TemplateArgumentLoc &Output, bool Uneval) {
   const TemplateArgument &Arg = Input.getArgument();
   switch (Arg.getKind()) {
   case TemplateArgument::Null:
@@ -3559,8 +3559,8 @@ bool TreeTransform<Derived>::TransformTemplateArgument(
 
   case TemplateArgument::Expression: {
     // Template argument expressions are constant expressions.
-    EnterExpressionEvaluationContext Unevaluated(getSema(),
-                                                 Sema::ConstantEvaluated);
+    EnterExpressionEvaluationContext Unevaluated(
+        getSema(), Uneval ? Sema::Unevaluated : Sema::ConstantEvaluated);
 
     Expr *InputExpr = Input.getSourceExpression();
     if (!InputExpr) InputExpr = Input.getArgument().getAsExpr();
@@ -3638,9 +3638,9 @@ public:
 
 template<typename Derived>
 template<typename InputIterator>
-bool TreeTransform<Derived>::TransformTemplateArguments(InputIterator First,
-                                                        InputIterator Last,
-                                            TemplateArgumentListInfo &Outputs) {
+bool TreeTransform<Derived>::TransformTemplateArguments(
+    InputIterator First, InputIterator Last, TemplateArgumentListInfo &Outputs,
+    bool Uneval) {
   for (; First != Last; ++First) {
     TemplateArgumentLoc Out;
     TemplateArgumentLoc In = *First;
@@ -3658,7 +3658,7 @@ bool TreeTransform<Derived>::TransformTemplateArguments(InputIterator First,
                                                  In.getArgument().pack_begin()),
                                      PackLocIterator(*this,
                                                    In.getArgument().pack_end()),
-                                     Outputs))
+                                     Outputs, Uneval))
         return true;
 
       continue;
@@ -3696,7 +3696,7 @@ bool TreeTransform<Derived>::TransformTemplateArguments(InputIterator First,
         // expansion.
         TemplateArgumentLoc OutPattern;
         Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), -1);
-        if (getDerived().TransformTemplateArgument(Pattern, OutPattern))
+        if (getDerived().TransformTemplateArgument(Pattern, OutPattern, Uneval))
           return true;
 
         Out = getDerived().RebuildPackExpansion(OutPattern, Ellipsis,
@@ -3713,7 +3713,7 @@ bool TreeTransform<Derived>::TransformTemplateArguments(InputIterator First,
       for (unsigned I = 0; I != *NumExpansions; ++I) {
         Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), I);
 
-        if (getDerived().TransformTemplateArgument(Pattern, Out))
+        if (getDerived().TransformTemplateArgument(Pattern, Out, Uneval))
           return true;
 
         if (Out.getArgument().containsUnexpandedParameterPack()) {
@@ -3731,7 +3731,7 @@ bool TreeTransform<Derived>::TransformTemplateArguments(InputIterator First,
       if (RetainExpansion) {
         ForgetPartiallySubstitutedPackRAII Forget(getDerived());
 
-        if (getDerived().TransformTemplateArgument(Pattern, Out))
+        if (getDerived().TransformTemplateArgument(Pattern, Out, Uneval))
           return true;
 
         Out = getDerived().RebuildPackExpansion(Out, Ellipsis,
@@ -3746,7 +3746,7 @@ bool TreeTransform<Derived>::TransformTemplateArguments(InputIterator First,
     }
 
     // The simple case:
-    if (getDerived().TransformTemplateArgument(In, Out))
+    if (getDerived().TransformTemplateArgument(In, Out, Uneval))
       return true;
 
     Outputs.addArgument(Out);
@@ -10005,36 +10005,86 @@ TreeTransform<Derived>::TransformSizeOfPackExpr(SizeOfPackExpr *E) {
   if (!E->isValueDependent())
     return E;
 
-  // Note: None of the implementations of TryExpandParameterPacks can ever
-  // produce a diagnostic when given only a single unexpanded parameter pack,
-  // so
-  UnexpandedParameterPack Unexpanded(E->getPack(), E->getPackLoc());
-  bool ShouldExpand = false;
-  bool RetainExpansion = false;
-  Optional<unsigned> NumExpansions;
-  if (getDerived().TryExpandParameterPacks(E->getOperatorLoc(), E->getPackLoc(),
-                                           Unexpanded,
-                                           ShouldExpand, RetainExpansion,
-                                           NumExpansions))
-    return ExprError();
+  EnterExpressionEvaluationContext Unevaluated(getSema(), Sema::Unevaluated);
 
-  if (RetainExpansion)
-    return E;
+  ArrayRef<TemplateArgument> PackArgs;
+  TemplateArgument ArgStorage;
+
+  // Find the argument list to transform.
+  if (E->isPartiallySubstituted()) {
+    PackArgs = E->getPartialArguments();
+  } else if (E->isValueDependent()) {
+    UnexpandedParameterPack Unexpanded(E->getPack(), E->getPackLoc());
+    bool ShouldExpand = false;
+    bool RetainExpansion = false;
+    Optional<unsigned> NumExpansions;
+    if (getDerived().TryExpandParameterPacks(E->getOperatorLoc(), E->getPackLoc(),
+                                             Unexpanded,
+                                             ShouldExpand, RetainExpansion,
+                                             NumExpansions))
+      return ExprError();
+
+    // If we need to expand the pack, build a template argument from it and
+    // expand that.
+    if (ShouldExpand) {
+      auto *Pack = E->getPack();
+      if (auto *TTPD = dyn_cast<TemplateTypeParmDecl>(Pack)) {
+        ArgStorage = getSema().Context.getPackExpansionType(
+            getSema().Context.getTypeDeclType(TTPD), None);
+      } else if (auto *TTPD = dyn_cast<TemplateTemplateParmDecl>(Pack)) {
+        ArgStorage = TemplateArgument(TemplateName(TTPD), None);
+      } else {
+        auto *VD = cast<ValueDecl>(Pack);
+        ExprResult DRE = getSema().BuildDeclRefExpr(VD, VD->getType(),
+                                                    VK_RValue, E->getPackLoc());
+        if (DRE.isInvalid())
+          return ExprError();
+        ArgStorage = new (getSema().Context) PackExpansionExpr(
+            getSema().Context.DependentTy, DRE.get(), E->getPackLoc(), None);
+      }
+      PackArgs = ArgStorage;
+    }
+  }
 
-  NamedDecl *Pack = E->getPack();
-  if (!ShouldExpand) {
-    Pack = cast_or_null<NamedDecl>(getDerived().TransformDecl(E->getPackLoc(),
-                                                              Pack));
+  // If we're not expanding the pack, just transform the decl.
+  if (!PackArgs.size()) {
+    auto *Pack = cast_or_null<NamedDecl>(
+        getDerived().TransformDecl(E->getPackLoc(), E->getPack()));
     if (!Pack)
       return ExprError();
+    return getDerived().RebuildSizeOfPackExpr(E->getOperatorLoc(), Pack,
+                                              E->getPackLoc(),
+                                              E->getRParenLoc(), None, None);
+  }
+
+  TemplateArgumentListInfo TransformedPackArgs(E->getPackLoc(),
+                                               E->getPackLoc());
+  {
+    TemporaryBase Rebase(*this, E->getPackLoc(), getBaseEntity());
+    typedef TemplateArgumentLocInventIterator<
+        Derived, const TemplateArgument*> PackLocIterator;
+    if (TransformTemplateArguments(PackLocIterator(*this, PackArgs.begin()),
+                                   PackLocIterator(*this, PackArgs.end()),
+                                   TransformedPackArgs, /*Uneval*/true))
+      return ExprError();
+  }
+
+  SmallVector<TemplateArgument, 8> Args;
+  bool PartialSubstitution = false;
+  for (auto &Loc : TransformedPackArgs.arguments()) {
+    Args.push_back(Loc.getArgument());
+    if (Loc.getArgument().isPackExpansion())
+      PartialSubstitution = true;
   }
 
+  if (PartialSubstitution)
+    return getDerived().RebuildSizeOfPackExpr(E->getOperatorLoc(), E->getPack(),
+                                              E->getPackLoc(),
+                                              E->getRParenLoc(), None, Args);
 
-  // We now know the length of the parameter pack, so build a new expression
-  // that stores that length.
-  return getDerived().RebuildSizeOfPackExpr(E->getOperatorLoc(), Pack,
+  return getDerived().RebuildSizeOfPackExpr(E->getOperatorLoc(), E->getPack(),
                                             E->getPackLoc(), E->getRParenLoc(),
-                                            NumExpansions);
+                                            Args.size(), None);
 }
 
 template<typename Derived>
diff --git a/lib/Serialization/ASTReaderStmt.cpp b/lib/Serialization/ASTReaderStmt.cpp
index 1bdd65cdf8..98955f63af 100644
--- a/lib/Serialization/ASTReaderStmt.cpp
+++ b/lib/Serialization/ASTReaderStmt.cpp
@@ -1554,11 +1554,20 @@ void ASTStmtReader::VisitPackExpansionExpr(PackExpansionExpr *E) {
 
 void ASTStmtReader::VisitSizeOfPackExpr(SizeOfPackExpr *E) {
   VisitExpr(E);
+  unsigned NumPartialArgs = Record[Idx++];
   E->OperatorLoc = ReadSourceLocation(Record, Idx);
   E->PackLoc = ReadSourceLocation(Record, Idx);
   E->RParenLoc = ReadSourceLocation(Record, Idx);
-  E->Length = Record[Idx++];
-  E->Pack = ReadDeclAs<NamedDecl>(Record, Idx);
+  E->Pack = Reader.ReadDeclAs<NamedDecl>(F, Record, Idx);
+  if (E->isPartiallySubstituted()) {
+    assert(E->Length == NumPartialArgs);
+    for (auto *I = reinterpret_cast<TemplateArgument *>(E + 1),
+              *E = I + NumPartialArgs;
+         I != E; ++I)
+      new (I) TemplateArgument(Reader.ReadTemplateArgument(F, Record, Idx));
+  } else if (!E->isValueDependent()) {
+    E->Length = Record[Idx++];
+  }
 }
 
 void ASTStmtReader::VisitSubstNonTypeTemplateParmExpr(
@@ -3114,7 +3123,9 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile &F) {
       break;
         
     case EXPR_SIZEOF_PACK:
-      S = new (Context) SizeOfPackExpr(Empty);
+      S = SizeOfPackExpr::CreateDeserialized(
+              Context,
+              /*NumPartialArgs=*/Record[ASTStmtReader::NumExprFields]);
       break;
         
     case EXPR_SUBST_NON_TYPE_TEMPLATE_PARM:
diff --git a/lib/Serialization/ASTWriterStmt.cpp b/lib/Serialization/ASTWriterStmt.cpp
index 41e9f96992..92d21389cb 100644
--- a/lib/Serialization/ASTWriterStmt.cpp
+++ b/lib/Serialization/ASTWriterStmt.cpp
@@ -1558,11 +1558,18 @@ void ASTStmtWriter::VisitPackExpansionExpr(PackExpansionExpr *E) {
 
 void ASTStmtWriter::VisitSizeOfPackExpr(SizeOfPackExpr *E) {
   VisitExpr(E);
+  Record.push_back(E->isPartiallySubstituted() ? E->getPartialArguments().size()
+                                               : 0);
   Writer.AddSourceLocation(E->OperatorLoc, Record);
   Writer.AddSourceLocation(E->PackLoc, Record);
   Writer.AddSourceLocation(E->RParenLoc, Record);
-  Record.push_back(E->Length);
   Writer.AddDeclRef(E->Pack, Record);
+  if (E->isPartiallySubstituted()) {
+    for (const auto &TA : E->getPartialArguments())
+      Writer.AddTemplateArgument(TA, Record);
+  } else if (!E->isValueDependent()) {
+    Record.push_back(E->getPackLength());
+  }
   Code = serialization::EXPR_SIZEOF_PACK;
 }
 
diff --git a/test/CodeGenCXX/mangle-variadic-templates.cpp b/test/CodeGenCXX/mangle-variadic-templates.cpp
index 264cc113cd..d2c1b77265 100644
--- a/test/CodeGenCXX/mangle-variadic-templates.cpp
+++ b/test/CodeGenCXX/mangle-variadic-templates.cpp
@@ -3,11 +3,12 @@
 template<unsigned I, typename ...Types>
 struct X { };
 
-template<typename T> struct identity { };
+template<typename T> struct identity { using type = T; };
 template<typename T> struct add_reference;
 template<typename ...Types> struct tuple { };
 template<int ...Values> struct int_tuple { };
 template<template<typename> class ...Templates> struct template_tuple { };
+template<typename ...T> using ArrayOfN = int[sizeof...(T)];
 
 // CHECK-LABEL: define weak_odr void @_Z2f0IJEEv1XIXsZT_EJDpRT_EE
 template<typename ...Types>
@@ -65,3 +66,12 @@ template<template<typename> class ...Templates>
 template_tuple<Templates...> f7() {}
 // CHECK-LABEL: define weak_odr void @_Z2f7IJ8identity13add_referenceEE14template_tupleIJDpT_EEv
 template template_tuple<identity, add_reference> f7();
+
+template<typename T, typename ...U> void f8(ArrayOfN<int, U..., T, typename U::type...>&) {}
+// CHECK-LABEL: define weak_odr void @_Z2f8IiJ8identityIiES0_IfEEEvRAsPiDpT0_T_DpNS3_4typeEE_i
+template void f8<int, identity<int>, identity<float>>(int (&)[6]);
+
+template<typename ...T> void f10(ArrayOfN<T...> &) {}
+// FIXME: This is wrong; should be @_Z3f10IJifEEvRAsZT__i
+// CHECK-LABEL: define weak_odr void @_Z3f10IJifEEvRAsPDpT_E_i
+template void f10<int, float>(int (&)[2]);
diff --git a/test/SemaTemplate/alias-templates.cpp b/test/SemaTemplate/alias-templates.cpp
index e7be184db3..1849ff6402 100644
--- a/test/SemaTemplate/alias-templates.cpp
+++ b/test/SemaTemplate/alias-templates.cpp
@@ -201,3 +201,23 @@ namespace PR16904 {
   template <typename T, typename U, typename V>
   using derived2 = ::PR16904::base<T, U>::template derived<V>; // expected-error {{expected a type}} expected-error {{expected ';'}}
 }
+
+namespace PR14858 {
+  template<typename ...T> using X = int[sizeof...(T)];
+
+  template<typename ...U> struct Y {
+    using Z = X<U...>;
+  };
+  using A = Y<int, int, int, int>::Z;
+  using A = int[4];
+
+  // FIXME: These should be treated as being redeclarations.
+  template<typename ...T> void f(X<T...> &) {}
+  template<typename ...T> void f(int(&)[sizeof...(T)]) {}
+
+  template<typename ...T> void g(X<typename T::type...> &) {}
+  template<typename ...T> void g(int(&)[sizeof...(T)]) {} // ok, different
+
+  template<typename ...T, typename ...U> void h(X<T...> &) {}
+  template<typename ...T, typename ...U> void h(X<U...> &) {} // ok, different
+}