When instantiating statements that involve conditions (if, while, do,

for, and switch), be careful to construct the full expressions as soon
as we perform template instantation, so we don't either forget to call
temporary destructors or destroy temporaries at the wrong time. This
is the template-instantiation analogue to r103187, during which I
hadn't realized that the issue would affect the handling of these
constructs differently inside and outside of templates.

Fixes a regression in Boost.Function.

llvm-svn: 103357

3 files changed, 138 insertions, 52 deletions

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 9825086809a..ec6e96b706d 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -758,21 +758,10 @@ public:
   ///
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
-  OwningStmtResult RebuildIfStmt(SourceLocation IfLoc, Sema::ExprArg Cond,
+  OwningStmtResult RebuildIfStmt(SourceLocation IfLoc, Sema::FullExprArg Cond,
                                  VarDecl *CondVar, StmtArg Then, 
                                  SourceLocation ElseLoc, StmtArg Else) {
-    if (Cond.get()) {
-      // Convert the condition to a boolean value.
-      OwningExprResult CondE = getSema().ActOnBooleanCondition(0, IfLoc, 
-                                                               move(Cond));
-      if (CondE.isInvalid())
-        return getSema().StmtError();
-      
-      Cond = move(CondE);
-    }
-    
-    Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond));
-    return getSema().ActOnIfStmt(IfLoc, FullCond, DeclPtrTy::make(CondVar), 
+    return getSema().ActOnIfStmt(IfLoc, Cond, DeclPtrTy::make(CondVar), 
                                  move(Then), ElseLoc, move(Else));
   }
 
@@ -802,20 +791,10 @@ public:
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
   OwningStmtResult RebuildWhileStmt(SourceLocation WhileLoc,
-                                    Sema::ExprArg Cond,
+                                    Sema::FullExprArg Cond,
                                     VarDecl *CondVar,
                                     StmtArg Body) {
-    if (Cond.get()) {
-      // Convert the condition to a boolean value.
-      OwningExprResult CondE = getSema().ActOnBooleanCondition(0, WhileLoc, 
-                                                               move(Cond));
-      if (CondE.isInvalid())
-        return getSema().StmtError();
-      Cond = move(CondE);
-    }
-    
-    Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond));
-    return getSema().ActOnWhileStmt(WhileLoc, FullCond, 
+    return getSema().ActOnWhileStmt(WhileLoc, Cond, 
                                     DeclPtrTy::make(CondVar), move(Body));
   }
 
@@ -838,21 +817,10 @@ public:
   /// Subclasses may override this routine to provide different behavior.
   OwningStmtResult RebuildForStmt(SourceLocation ForLoc,
                                   SourceLocation LParenLoc,
-                                  StmtArg Init, Sema::ExprArg Cond, 
+                                  StmtArg Init, Sema::FullExprArg Cond, 
                                   VarDecl *CondVar, Sema::FullExprArg Inc,
                                   SourceLocation RParenLoc, StmtArg Body) {
-    if (Cond.get()) {
-      // Convert the condition to a boolean value.
-      OwningExprResult CondE = getSema().ActOnBooleanCondition(0, ForLoc, 
-                                                               move(Cond));
-      if (CondE.isInvalid())
-        return getSema().StmtError();
-      
-      Cond = move(CondE);
-    }
-    
-    Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond));
-    return getSema().ActOnForStmt(ForLoc, LParenLoc, move(Init), FullCond, 
+    return getSema().ActOnForStmt(ForLoc, LParenLoc, move(Init), Cond, 
                                   DeclPtrTy::make(CondVar),
                                   Inc, RParenLoc, move(Body));
   }
@@ -3523,8 +3491,20 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
   
     if (Cond.isInvalid())
       return SemaRef.StmtError();
+    
+    // Convert the condition to a boolean value.
+    OwningExprResult CondE = getSema().ActOnBooleanCondition(0, S->getIfLoc(), 
+                                                             move(Cond));
+    if (CondE.isInvalid())
+      return getSema().StmtError();
+    
+    Cond = move(CondE);
   }
   
+  Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond));
+  if (!S->getConditionVariable() && S->getCond() && !FullCond->get())
+    return SemaRef.StmtError();
+  
   // Transform the "then" branch.
   OwningStmtResult Then = getDerived().TransformStmt(S->getThen());
   if (Then.isInvalid())
@@ -3536,13 +3516,13 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
     return SemaRef.StmtError();
 
   if (!getDerived().AlwaysRebuild() &&
-      Cond.get() == S->getCond() &&
+      FullCond->get() == S->getCond() &&
       ConditionVar == S->getConditionVariable() &&
       Then.get() == S->getThen() &&
       Else.get() == S->getElse())
     return SemaRef.Owned(S->Retain());
 
-  return getDerived().RebuildIfStmt(S->getIfLoc(), move(Cond), ConditionVar,
+  return getDerived().RebuildIfStmt(S->getIfLoc(), FullCond, ConditionVar,
                                     move(Then),
                                     S->getElseLoc(), move(Else));
 }
@@ -3604,36 +3584,48 @@ TreeTransform<Derived>::TransformWhileStmt(WhileStmt *S) {
     
     if (Cond.isInvalid())
       return SemaRef.StmtError();
+    
+    // Convert the condition to a boolean value.
+    OwningExprResult CondE = getSema().ActOnBooleanCondition(0, 
+                                                             S->getWhileLoc(), 
+                                                             move(Cond));
+    if (CondE.isInvalid())
+      return getSema().StmtError();
+    Cond = move(CondE);
   }
 
+  Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond));
+  if (!S->getConditionVariable() && S->getCond() && !FullCond->get())
+    return SemaRef.StmtError();
+
   // Transform the body
   OwningStmtResult Body = getDerived().TransformStmt(S->getBody());
   if (Body.isInvalid())
     return SemaRef.StmtError();
 
   if (!getDerived().AlwaysRebuild() &&
-      Cond.get() == S->getCond() &&
+      FullCond->get() == S->getCond() &&
       ConditionVar == S->getConditionVariable() &&
       Body.get() == S->getBody())
     return SemaRef.Owned(S->Retain());
 
-  return getDerived().RebuildWhileStmt(S->getWhileLoc(), move(Cond),
+  return getDerived().RebuildWhileStmt(S->getWhileLoc(), FullCond,
                                        ConditionVar, move(Body));
 }
 
 template<typename Derived>
 Sema::OwningStmtResult
 TreeTransform<Derived>::TransformDoStmt(DoStmt *S) {
-  // Transform the condition
-  OwningExprResult Cond = getDerived().TransformExpr(S->getCond());
-  if (Cond.isInvalid())
-    return SemaRef.StmtError();
-
   // Transform the body
   OwningStmtResult Body = getDerived().TransformStmt(S->getBody());
   if (Body.isInvalid())
     return SemaRef.StmtError();
 
+  // Transform the condition
+  OwningExprResult Cond = getDerived().TransformExpr(S->getCond());
+  if (Cond.isInvalid())
+    return SemaRef.StmtError();
+  
   if (!getDerived().AlwaysRebuild() &&
       Cond.get() == S->getCond() &&
       Body.get() == S->getBody())
@@ -3668,13 +3660,29 @@ TreeTransform<Derived>::TransformForStmt(ForStmt *S) {
     
     if (Cond.isInvalid())
       return SemaRef.StmtError();
+    
+    // Convert the condition to a boolean value.
+    OwningExprResult CondE = getSema().ActOnBooleanCondition(0, S->getForLoc(), 
+                                                             move(Cond));
+    if (CondE.isInvalid())
+      return getSema().StmtError();
+    
+    Cond = move(CondE);
   }
 
+  Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond));  
+  if (!S->getConditionVariable() && S->getCond() && !FullCond->get())
+    return SemaRef.StmtError();
+
   // Transform the increment
   OwningExprResult Inc = getDerived().TransformExpr(S->getInc());
   if (Inc.isInvalid())
     return SemaRef.StmtError();
 
+  Sema::FullExprArg FullInc(getSema().MakeFullExpr(Inc));
+  if (S->getInc() && !FullInc->get())
+    return SemaRef.StmtError();
+
   // Transform the body
   OwningStmtResult Body = getDerived().TransformStmt(S->getBody());
   if (Body.isInvalid())
@@ -3682,15 +3690,14 @@ TreeTransform<Derived>::TransformForStmt(ForStmt *S) {
 
   if (!getDerived().AlwaysRebuild() &&
       Init.get() == S->getInit() &&
-      Cond.get() == S->getCond() &&
+      FullCond->get() == S->getCond() &&
       Inc.get() == S->getInc() &&
       Body.get() == S->getBody())
     return SemaRef.Owned(S->Retain());
 
   return getDerived().RebuildForStmt(S->getForLoc(), S->getLParenLoc(),
-                                     move(Init), move(Cond), ConditionVar,
-                                     getSema().MakeFullExpr(Inc),
-                                     S->getRParenLoc(), move(Body));
+                                     move(Init), FullCond, ConditionVar,
+                                     FullInc, S->getRParenLoc(), move(Body));
 }
 
 template<typename Derived>
diff --git a/clang/test/CodeGenCXX/condition.cpp b/clang/test/CodeGenCXX/condition.cpp
index e4266cf5028..f5b43d2ef49 100644
--- a/clang/test/CodeGenCXX/condition.cpp
+++ b/clang/test/CodeGenCXX/condition.cpp
@@ -14,6 +14,7 @@ void h() {
 
 struct X {
   X();
+  X(const X&);
   ~X();
   operator bool();
 };
@@ -171,3 +172,81 @@ void do_destruct(int z) {
   z = 99;
   // CHECK: ret
 }
+
+int f(X); 
+
+template<typename T>
+int instantiated(T x) { 
+  int result;
+
+  // CHECK: call void @_ZN1XC1ERKS_
+  // CHECK: call i32 @_Z1f1X
+  // CHECK: call void @_ZN1XD1Ev
+  // CHECK: br
+  // CHECK: store i32 2
+  // CHECK: br
+  // CHECK: store i32 3
+  if (f(x)) { result = 2; } else { result = 3; }
+
+  // CHECK: call void @_ZN1XC1ERKS_
+  // CHECK: call i32 @_Z1f1X
+  // CHECK: call void @_ZN1XD1Ev
+  // CHECK: br
+  // CHECK: store i32 4
+  // CHECK: br
+  while (f(x)) { result = 4; }
+
+  // CHECK: call void @_ZN1XC1ERKS_
+  // CHECK: call i32 @_Z1f1X
+  // CHECK: call void @_ZN1XD1Ev
+  // CHECK: br
+  // CHECK: store i32 6
+  // CHECK: br
+  // CHECK: call void @_ZN1XC1ERKS_
+  // CHECK: call i32 @_Z1f1X
+  // CHECK: store i32 5
+  // CHECK: call void @_ZN1XD1Ev
+  // CHECK: br
+  for (; f(x); f(x), result = 5) {
+    result = 6;
+  }
+
+  // CHECK: call void @_ZN1XC1ERKS_
+  // CHECK: call i32 @_Z1f1X
+  // CHECK: call void @_ZN1XD1Ev
+  // CHECK: switch i32
+  // CHECK: store i32 7
+  // CHECK: store i32 8
+  switch (f(x)) {
+  case 0: 
+    result = 7;
+    break;
+
+  case 1:
+    result = 8;
+  }
+
+  // CHECK: store i32 9
+  // CHECK: br
+  // CHECK: call void @_ZN1XC1ERKS_
+  // CHECK: call i32 @_Z1f1X
+  // CHECK: call void @_ZN1XD1Ev
+  // CHECK: br
+  do {
+    result = 9;
+  } while (f(x));
+
+  // CHECK: store i32 10
+  // CHECK: call void @_ZN1XC1ERKS_
+  // CHECK: call zeroext i1 @_ZN1XcvbEv
+  // CHECK: call void @_ZN1XD1Ev
+  // CHECK: br
+  do {
+    result = 10;
+  } while (X(x));
+
+  // CHECK: ret i32
+  return result;
+}
+
+template int instantiated(X);
diff --git a/clang/test/SemaTemplate/instantiate-expr-3.cpp b/clang/test/SemaTemplate/instantiate-expr-3.cpp
index 41a96a32e0b..d506b19a7a9 100644
--- a/clang/test/SemaTemplate/instantiate-expr-3.cpp
+++ b/clang/test/SemaTemplate/instantiate-expr-3.cpp
@@ -63,7 +63,7 @@ template struct Conditional0<int, int, int>;
 template<typename T>
 struct StatementExpr0 {
   void f(T t) {
-    (void)({ if (t) t = t + 17; t + 12;}); // expected-error{{invalid}}
+    (void)({ if (t) t = t + 17; t + 12;}); // expected-error{{contextually convertible}}
   }
 };