Improve template instantiation for object constructions in several ways:

  - During instantiation, drop default arguments from constructor and
    call expressions; they'll be recomputed anyway, and we don't want
    to instantiate them twice.
  - Rewrote the instantiation of variable initializers to cope with
    non-dependent forms properly.

Together, these fix a handful of problems I introduced with the switch
to always rebuild expressions from the source code "as written."



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

5 files changed, 178 insertions, 100 deletions

diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h
index 610cbd8c30..469598ff37 100644
--- a/include/clang/AST/Expr.h
+++ b/include/clang/AST/Expr.h
@@ -309,6 +309,15 @@ public:
   /// ParenExpr or CastExprs, returning their operand.
   Expr *IgnoreParenNoopCasts(ASTContext &Ctx);
 
+  /// \brief Determine whether this expression is a default function argument.
+  ///
+  /// Default arguments are implicitly generated in the abstract syntax tree
+  /// by semantic analysis for function calls, object constructions, etc. in 
+  /// C++. Default arguments are represented by \c CXXDefaultArgExpr nodes;
+  /// this routine also looks through any implicit casts to determine whether
+  /// the expression is a default argument.
+  bool isDefaultArgument() const;
+  
   const Expr* IgnoreParens() const {
     return const_cast<Expr*>(this)->IgnoreParens();
   }
@@ -1609,6 +1618,14 @@ public:
   const Expr *getSubExpr() const { return cast<Expr>(Op); }
   void setSubExpr(Expr *E) { Op = E; }
 
+  /// \brief Retrieve the cast subexpression as it was written in the source
+  /// code, looking through any implicit casts or other intermediate nodes
+  /// introduced by semantic analysis.
+  Expr *getSubExprAsWritten();
+  const Expr *getSubExprAsWritten() const {
+    return const_cast<CastExpr *>(this)->getSubExprAsWritten();
+  }
+    
   static bool classof(const Stmt *T) {
     StmtClass SC = T->getStmtClass();
     if (SC >= CXXNamedCastExprClass && SC <= CXXFunctionalCastExprClass)
diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index 6d9a268993..139e04b2ed 100644
--- a/lib/AST/Expr.cpp
+++ b/lib/AST/Expr.cpp
@@ -568,6 +568,30 @@ const char *CastExpr::getCastKindName() const {
   return 0;
 }
 
+Expr *CastExpr::getSubExprAsWritten() {
+  Expr *SubExpr = 0;
+  CastExpr *E = this;
+  do {
+    SubExpr = E->getSubExpr();
+    
+    // Skip any temporary bindings; they're implicit.
+    if (CXXBindTemporaryExpr *Binder = dyn_cast<CXXBindTemporaryExpr>(SubExpr))
+      SubExpr = Binder->getSubExpr();
+    
+    // Conversions by constructor and conversion functions have a
+    // subexpression describing the call; strip it off.
+    if (E->getCastKind() == CastExpr::CK_ConstructorConversion)
+      SubExpr = cast<CXXConstructExpr>(SubExpr)->getArg(0);
+    else if (E->getCastKind() == CastExpr::CK_UserDefinedConversion)
+      SubExpr = cast<CXXMemberCallExpr>(SubExpr)->getImplicitObjectArgument();
+    
+    // If the subexpression we're left with is an implicit cast, look
+    // through that, too.
+  } while ((E = dyn_cast<ImplicitCastExpr>(SubExpr)));  
+  
+  return SubExpr;
+}
+
 /// getOpcodeStr - Turn an Opcode enum value into the punctuation char it
 /// corresponds to, e.g. "<<=".
 const char *BinaryOperator::getOpcodeStr(Opcode Op) {
@@ -1347,6 +1371,13 @@ Expr *Expr::IgnoreParenNoopCasts(ASTContext &Ctx) {
   }
 }
 
+bool Expr::isDefaultArgument() const {
+  const Expr *E = this;
+  while (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E))
+    E = ICE->getSubExprAsWritten();
+  
+  return isa<CXXDefaultArgExpr>(E);
+}
 
 /// hasAnyTypeDependentArguments - Determines if any of the expressions
 /// in Exprs is type-dependent.
diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 4512bcde16..9c4c38a134 100644
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -150,6 +150,35 @@ Decl *TemplateDeclInstantiator::VisitTypedefDecl(TypedefDecl *D) {
   return Typedef;
 }
 
+/// \brief Instantiate the arguments provided as part of initialization.
+///
+/// \returns true if an error occurred, false otherwise.
+static bool InstantiateInitializationArguments(Sema &SemaRef,
+                                               Expr **Args, unsigned NumArgs,
+                           const MultiLevelTemplateArgumentList &TemplateArgs,
+                         llvm::SmallVectorImpl<SourceLocation> &FakeCommaLocs,
+                           ASTOwningVector<&ActionBase::DeleteExpr> &InitArgs) {
+  for (unsigned I = 0; I != NumArgs; ++I) {
+    // When we hit the first defaulted argument, break out of the loop:
+    // we don't pass those default arguments on.
+    if (Args[I]->isDefaultArgument())
+      break;
+  
+    Sema::OwningExprResult Arg = SemaRef.SubstExpr(Args[I], TemplateArgs);
+    if (Arg.isInvalid())
+      return true;
+  
+    Expr *ArgExpr = (Expr *)Arg.get();
+    InitArgs.push_back(Arg.release());
+    
+    // FIXME: We're faking all of the comma locations. Do we need them?
+    FakeCommaLocs.push_back(
+                          SemaRef.PP.getLocForEndOfToken(ArgExpr->getLocEnd()));
+  }
+  
+  return false;
+}
+
 Decl *TemplateDeclInstantiator::VisitVarDecl(VarDecl *D) {
   // Do substitution on the type of the declaration
   TypeSourceInfo *DI = SemaRef.SubstType(D->getTypeSourceInfo(),
@@ -201,78 +230,76 @@ Decl *TemplateDeclInstantiator::VisitVarDecl(VarDecl *D) {
     else
       SemaRef.PushExpressionEvaluationContext(Sema::PotentiallyEvaluated);
 
-    OwningExprResult Init
-      = SemaRef.SubstExpr(D->getInit(), TemplateArgs);
-    if (Init.isInvalid())
-      Var->setInvalidDecl();
-    else if (ParenListExpr *PLE = dyn_cast<ParenListExpr>((Expr *)Init.get())) {
-      // FIXME: We're faking all of the comma locations, which is suboptimal.
-      // Do we even need these comma locations?
-      llvm::SmallVector<SourceLocation, 4> FakeCommaLocs;
-      if (PLE->getNumExprs() > 0) {
-        FakeCommaLocs.reserve(PLE->getNumExprs() - 1);
-        for (unsigned I = 0, N = PLE->getNumExprs() - 1; I != N; ++I) {
-          Expr *E = PLE->getExpr(I)->Retain();
-          FakeCommaLocs.push_back(
-                                SemaRef.PP.getLocForEndOfToken(E->getLocEnd()));
-        }
-        PLE->getExpr(PLE->getNumExprs() - 1)->Retain();
+    // Extract the initializer, skipping through any temporary-binding 
+    // expressions and look at the subexpression as it was written.
+    Expr *DInit = D->getInit();
+    while (CXXBindTemporaryExpr *Binder = dyn_cast<CXXBindTemporaryExpr>(DInit))
+      DInit = Binder->getSubExpr();
+    if (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(DInit))
+      DInit = ICE->getSubExprAsWritten();
+        
+    if (ParenListExpr *PLE = dyn_cast<ParenListExpr>(DInit)) {
+      // The initializer is a parenthesized list of expressions that is
+      // type-dependent. Instantiate each of the expressions; we'll be 
+      // performing direct initialization with them.
+      llvm::SmallVector<SourceLocation, 4> CommaLocs;
+      ASTOwningVector<&ActionBase::DeleteExpr> InitArgs(SemaRef);
+      if (!InstantiateInitializationArguments(SemaRef, 
+                                              PLE->getExprs(), 
+                                              PLE->getNumExprs(),
+                                              TemplateArgs,
+                                              CommaLocs, InitArgs)) {
+        // Add the direct initializer to the declaration.
+        SemaRef.AddCXXDirectInitializerToDecl(Sema::DeclPtrTy::make(Var),
+                                              PLE->getLParenLoc(),
+                                              move_arg(InitArgs),
+                                              CommaLocs.data(),
+                                              PLE->getRParenLoc());
       }
-
-      // Add the direct initializer to the declaration.
-      SemaRef.AddCXXDirectInitializerToDecl(Sema::DeclPtrTy::make(Var),
-                                            PLE->getLParenLoc(),
-                                            Sema::MultiExprArg(SemaRef,
-                                                       (void**)PLE->getExprs(),
-                                                           PLE->getNumExprs()),
-                                            FakeCommaLocs.data(),
-                                            PLE->getRParenLoc());
-
-      // When Init is destroyed, it will destroy the instantiated ParenListExpr;
-      // we've explicitly retained all of its subexpressions already.
-    } else if (CXXConstructExpr *Construct
-                 = dyn_cast<CXXConstructExpr>((Expr *)Init.get())) {
-      // We build CXXConstructExpr nodes to capture the implicit
-      // construction of objects. Rip apart the CXXConstructExpr to
-      // pass its pieces down to the appropriate initialization
-      // function.
-      if (D->hasCXXDirectInitializer()) {
-        // FIXME: Poor source location information
-        SourceLocation FakeLParenLoc = 
-          SemaRef.PP.getLocForEndOfToken(D->getLocation());
-        SourceLocation FakeRParenLoc = FakeLParenLoc;
-        llvm::SmallVector<SourceLocation, 4> FakeCommaLocs;
-        if (Construct->getNumArgs() > 0) {
-          FakeRParenLoc 
-            = SemaRef.PP.getLocForEndOfToken(
-                  Construct->getArg(Construct->getNumArgs() - 1)->getLocEnd());
-
-          FakeCommaLocs.reserve(Construct->getNumArgs() - 1);
-          for (unsigned I = 0, N = Construct->getNumArgs() - 1; I != N; ++I) {
-            Expr *E = Construct->getArg(I)->Retain();
-            FakeCommaLocs.push_back(
-                                SemaRef.PP.getLocForEndOfToken(E->getLocEnd()));
-          }
-          Construct->getArg(Construct->getNumArgs() - 1)->Retain();
+    } else if (CXXConstructExpr *Construct =dyn_cast<CXXConstructExpr>(DInit)) {
+      // The initializer resolved to a constructor. Instantiate the constructor
+      // arguments.
+      llvm::SmallVector<SourceLocation, 4> CommaLocs;
+      ASTOwningVector<&ActionBase::DeleteExpr> InitArgs(SemaRef);
+      
+      if (!InstantiateInitializationArguments(SemaRef, 
+                                              Construct->getArgs(),
+                                              Construct->getNumArgs(),
+                                              TemplateArgs,
+                                              CommaLocs, InitArgs)) {
+        if (D->hasCXXDirectInitializer()) {
+          SourceLocation FakeLParenLoc = 
+            SemaRef.PP.getLocForEndOfToken(D->getLocation());
+          SourceLocation FakeRParenLoc = CommaLocs.empty()? FakeLParenLoc
+                                                          : CommaLocs.back();
+          SemaRef.AddCXXDirectInitializerToDecl(Sema::DeclPtrTy::make(Var),
+                                                FakeLParenLoc,
+                                                move_arg(InitArgs),
+                                                CommaLocs.data(),
+                                                FakeRParenLoc);          
+        } else if (InitArgs.size() == 1) {
+          Expr *Init = (Expr*)(InitArgs.take()[0]);
+          SemaRef.AddInitializerToDecl(Sema::DeclPtrTy::make(Var), 
+                                       SemaRef.Owned(Init),
+                                       false);
+        } else {
+          assert(InitArgs.size() == 0);
+          SemaRef.ActOnUninitializedDecl(Sema::DeclPtrTy::make(Var), false);          
         }
+      } 
+    } else {
+      OwningExprResult Init
+        = SemaRef.SubstExpr(D->getInit(), TemplateArgs);
 
-        SemaRef.AddCXXDirectInitializerToDecl(Sema::DeclPtrTy::make(Var),
-                                              FakeLParenLoc,
-                               Sema::MultiExprArg(SemaRef,
-                                                  (void **)Construct->getArgs(),
-                                                  Construct->getNumArgs()),
-                                              FakeCommaLocs.data(),
-                                              FakeRParenLoc);
-                                              
-      } else if (Construct->getNumArgs() >= 1) {
-        SemaRef.AddInitializerToDecl(Sema::DeclPtrTy::make(Var), 
-                                  SemaRef.Owned(Construct->getArg(0)->Retain()),
-                                     false);
-      } else
-        SemaRef.ActOnUninitializedDecl(Sema::DeclPtrTy::make(Var), false);
-    } else 
-      SemaRef.AddInitializerToDecl(Sema::DeclPtrTy::make(Var), move(Init),
-                                   D->hasCXXDirectInitializer());
+      // FIXME: Not happy about invalidating decls just because of a bad 
+      // initializer, unless it affects the type.
+      if (Init.isInvalid())
+        Var->setInvalidDecl();
+      else
+        SemaRef.AddInitializerToDecl(Sema::DeclPtrTy::make(Var), move(Init),
+                                     D->hasCXXDirectInitializer());
+    }
+    
     SemaRef.PopExpressionEvaluationContext();
   } else if (!Var->isStaticDataMember() || Var->isOutOfLine())
     SemaRef.ActOnUninitializedDecl(Sema::DeclPtrTy::make(Var), false);
diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h
index b1d7f06489..fd19987344 100644
--- a/lib/Sema/TreeTransform.h
+++ b/lib/Sema/TreeTransform.h
@@ -172,6 +172,16 @@ public:
     return T.isNull();
   }
 
+  /// \brief Determine whether the given call argument should be dropped, e.g.,
+  /// because it is a default argument.
+  ///
+  /// Subclasses can provide an alternative implementation of this routine to
+  /// determine which kinds of call arguments get dropped. By default,
+  /// CXXDefaultArgument nodes are dropped (prior to transformation).
+  bool DropCallArgument(Expr *E) {
+    return E->isDefaultArgument();
+  }
+  
   /// \brief Transforms the given type into another type.
   ///
   /// By default, this routine transforms a type by creating a
@@ -3770,39 +3780,12 @@ TreeTransform<Derived>::TransformConditionalOperator(ConditionalOperator *E) {
                                                  move(RHS));
 }
 
-/// \brief Given a cast expression, extract the subexpression of the
-/// cast, looking through intermediate AST nodes that were generated
-/// as part of type checking.
-static Expr *getCastSubExprAsWritten(CastExpr *E) {
-  Expr *SubExpr = 0;
-  do {
-    SubExpr = E->getSubExpr();
-
-    // Temporaries will be re-bound when rebuilding the original cast
-    // expression.
-    if (CXXBindTemporaryExpr *Binder = dyn_cast<CXXBindTemporaryExpr>(SubExpr))
-      SubExpr = Binder->getSubExpr();
-
-    // Conversions by constructor and conversion functions have a
-    // subexpression describing the call; strip it off.
-    if (E->getCastKind() == CastExpr::CK_ConstructorConversion)
-      SubExpr = cast<CXXConstructExpr>(SubExpr)->getArg(0);
-    else if (E->getCastKind() == CastExpr::CK_UserDefinedConversion)
-      SubExpr = cast<CXXMemberCallExpr>(SubExpr)->getImplicitObjectArgument();
-
-    // If the subexpression we're left with is an implicit cast, look
-    // through that, too.
-  } while ((E = dyn_cast<ImplicitCastExpr>(SubExpr)));
-
-  return SubExpr;
-}
-
 template<typename Derived>
 Sema::OwningExprResult
 TreeTransform<Derived>::TransformImplicitCastExpr(ImplicitCastExpr *E) {
   // Implicit casts are eliminated during transformation, since they
   // will be recomputed by semantic analysis after transformation.
-  return getDerived().TransformExpr(getCastSubExprAsWritten(E));
+  return getDerived().TransformExpr(E->getSubExprAsWritten());
 }
 
 template<typename Derived>
@@ -3828,7 +3811,7 @@ TreeTransform<Derived>::TransformCStyleCastExpr(CStyleCastExpr *E) {
   }
 
   OwningExprResult SubExpr
-    = getDerived().TransformExpr(getCastSubExprAsWritten(E));
+    = getDerived().TransformExpr(E->getSubExprAsWritten());
   if (SubExpr.isInvalid())
     return SemaRef.ExprError();
 
@@ -4161,6 +4144,9 @@ TreeTransform<Derived>::TransformCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
     ASTOwningVector<&ActionBase::DeleteExpr> Args(SemaRef);
     llvm::SmallVector<SourceLocation, 4> FakeCommaLocs;
     for (unsigned I = 1, N = E->getNumArgs(); I != N; ++I) {
+      if (getDerived().DropCallArgument(E->getArg(I)))
+        break;
+      
       OwningExprResult Arg = getDerived().TransformExpr(E->getArg(I));
       if (Arg.isInvalid())
         return SemaRef.ExprError();
@@ -4247,7 +4233,7 @@ TreeTransform<Derived>::TransformCXXNamedCastExpr(CXXNamedCastExpr *E) {
   }
 
   OwningExprResult SubExpr
-    = getDerived().TransformExpr(getCastSubExprAsWritten(E));
+    = getDerived().TransformExpr(E->getSubExprAsWritten());
   if (SubExpr.isInvalid())
     return SemaRef.ExprError();
 
@@ -4312,7 +4298,7 @@ TreeTransform<Derived>::TransformCXXFunctionalCastExpr(
   }
 
   OwningExprResult SubExpr
-    = getDerived().TransformExpr(getCastSubExprAsWritten(E));
+    = getDerived().TransformExpr(E->getSubExprAsWritten());
   if (SubExpr.isInvalid())
     return SemaRef.ExprError();
 
@@ -4713,6 +4699,11 @@ TreeTransform<Derived>::TransformCXXConstructExpr(CXXConstructExpr *E) {
   for (CXXConstructExpr::arg_iterator Arg = E->arg_begin(),
        ArgEnd = E->arg_end();
        Arg != ArgEnd; ++Arg) {
+    if (getDerived().DropCallArgument(*Arg)) {
+      ArgumentChanged = true;
+      break;
+    }
+
     OwningExprResult TransArg = getDerived().TransformExpr(*Arg);
     if (TransArg.isInvalid())
       return SemaRef.ExprError();
diff --git a/test/SemaTemplate/constructor-template.cpp b/test/SemaTemplate/constructor-template.cpp
index 203977e9ed..0b6916fa48 100644
--- a/test/SemaTemplate/constructor-template.cpp
+++ b/test/SemaTemplate/constructor-template.cpp
@@ -82,3 +82,15 @@ X4 test_X4(bool Cond, X4 x4) {
   X4 b(x4); // okay, copy constructor
   return X4(); // expected-error{{no viable conversion}}
 }
+
+// Instantiation of a non-dependent use of a constructor
+struct DefaultCtorHasDefaultArg {
+  explicit DefaultCtorHasDefaultArg(int i = 17);
+};
+
+template<typename T>
+void default_ctor_inst() {
+  DefaultCtorHasDefaultArg def;
+}
+
+template void default_ctor_inst<int>();