]> granicus.if.org Git - clang/commitdiff
Rework our handling of temporary objects within the conditions of
authorDouglas Gregor <dgregor@apple.com>
Thu, 6 May 2010 17:25:47 +0000 (17:25 +0000)
committerDouglas Gregor <dgregor@apple.com>
Thu, 6 May 2010 17:25:47 +0000 (17:25 +0000)
if/switch/while/do/for statements. Previously, we would end up either:

  (1) Forgetting to destroy temporaries created in the condition (!),
  (2) Destroying the temporaries created in the condition *before*
  converting the condition to a boolean value (or, in the case of a
  switch statement, to an integral or enumeral value), or
  (3) In a for statement, destroying the condition's temporaries at
  the end of the increment expression (!).

We now destroy temporaries in conditions at the right times. This
required some tweaking of the Parse/Sema interaction, since the parser
was building full expressions too early in many places.

Fixes PR7067.

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

include/clang/Parse/Action.h
include/clang/Parse/Parser.h
lib/Frontend/PrintParserCallbacks.cpp
lib/Parse/ParseExprCXX.cpp
lib/Parse/ParseStmt.cpp
lib/Sema/Sema.h
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
lib/Sema/SemaStmt.cpp
lib/Sema/TreeTransform.h
test/CodeGenCXX/condition.cpp

index 98c9538d5ef98749414d91533629b5b2e64da91b..afa8bb625477e8dc0b62d74f0ec8d9a6d0043f3d 100644 (file)
@@ -105,12 +105,19 @@ public:
 
   class FullExprArg {
   public:
+    FullExprArg(ActionBase &actions) : Expr(actions) { }
+                
     // FIXME: The const_cast here is ugly. RValue references would make this
     // much nicer (or we could duplicate a bunch of the move semantics
     // emulation code from Ownership.h).
     FullExprArg(const FullExprArg& Other)
       : Expr(move(const_cast<FullExprArg&>(Other).Expr)) {}
 
+    FullExprArg &operator=(const FullExprArg& Other) {
+      Expr = move(const_cast<FullExprArg&>(Other).Expr);
+      return *this;
+    }
+
     OwningExprResult release() {
       return move(Expr);
     }
@@ -826,21 +833,19 @@ public:
 
   /// \brief Parsed the start of a "switch" statement.
   ///
+  /// \param SwitchLoc The location of the "switch" keyword.
+  ///
   /// \param Cond if the "switch" condition was parsed as an expression, 
   /// the expression itself.
   ///
   /// \param CondVar if the "switch" condition was parsed as a condition 
   /// variable, the condition variable itself.
-  virtual OwningStmtResult ActOnStartOfSwitchStmt(FullExprArg Cond,
+  virtual OwningStmtResult ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
+                                                  ExprArg Cond,
                                                   DeclPtrTy CondVar) {
     return StmtEmpty();
   }
 
-  /// ActOnSwitchBodyError - This is called if there is an error parsing the
-  /// body of the switch stmt instead of ActOnFinishSwitchStmt.
-  virtual void ActOnSwitchBodyError(SourceLocation SwitchLoc, StmtArg Switch,
-                                    StmtArg Body) {}
-  
   virtual OwningStmtResult ActOnFinishSwitchStmt(SourceLocation SwitchLoc,
                                                  StmtArg Switch, StmtArg Body) {
     return StmtEmpty();
@@ -1609,6 +1614,22 @@ public:
     return DeclResult();
   }
 
+  /// \brief Parsed an expression that will be handled as the condition in
+  /// an if/while/for statement. 
+  ///
+  /// This routine handles the conversion of the expression to 'bool'.
+  ///
+  /// \param S The scope in which the expression occurs.
+  ///
+  /// \param Loc The location of the construct that requires the conversion to
+  /// a boolean value.
+  ///
+  /// \param SubExpr The expression that is being converted to bool.
+  virtual OwningExprResult ActOnBooleanCondition(Scope *S, SourceLocation Loc,
+                                                 ExprArg SubExpr) {
+    return move(SubExpr);
+  }
+  
   /// ActOnCXXNew - Parsed a C++ 'new' expression. UseGlobal is true if the
   /// new was qualified (::new). In a full new like
   /// @code new (p1, p2) type(c1, c2) @endcode
index 0180419e0429ff7e9f6851c9f356ebfa8ce08565..530570913246510a6aab313ab25e32d71b1d04ff 100644 (file)
@@ -1004,7 +1004,8 @@ private:
 
   //===--------------------------------------------------------------------===//
   // C++ if/switch/while condition expression.
-  bool ParseCXXCondition(OwningExprResult &ExprResult, DeclPtrTy &DeclResult);
+  bool ParseCXXCondition(OwningExprResult &ExprResult, DeclPtrTy &DeclResult,
+                         SourceLocation Loc, bool ConvertToBoolean);
 
   //===--------------------------------------------------------------------===//
   // C++ types
@@ -1060,7 +1061,9 @@ private:
                                           bool isStmtExpr = false);
   OwningStmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
   bool ParseParenExprOrCondition(OwningExprResult &ExprResult,
-                                 DeclPtrTy &DeclResult);
+                                 DeclPtrTy &DeclResult,
+                                 SourceLocation Loc,
+                                 bool ConvertToBoolean);
   OwningStmtResult ParseIfStatement(AttributeList *Attr);
   OwningStmtResult ParseSwitchStatement(AttributeList *Attr);
   OwningStmtResult ParseWhileStatement(AttributeList *Attr);
index 4df5c5263c7eedd99294652346ce1796b691a0ba..258d352452ee9a8487f21b152bb80ffce4608d96 100644 (file)
@@ -315,7 +315,8 @@ namespace {
       return StmtEmpty();
     }
 
-    virtual OwningStmtResult ActOnStartOfSwitchStmt(FullExprArg Cond, 
+    virtual OwningStmtResult ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
+                                                    ExprArg Cond, 
                                                     DeclPtrTy CondVar) {
       Out << __FUNCTION__ << "\n";
       return StmtEmpty();
index 9eb95062394ac0b819d3ba212109f0947366744d..b7ccea53d5903f46fe648572398bf89c87ea49d9 100644 (file)
@@ -689,17 +689,33 @@ Parser::ParseCXXTypeConstructExpression(const DeclSpec &DS) {
 /// \param DeclResult if the condition was parsed as a declaration, the
 /// parsed declaration.
 ///
+/// \param Loc The location of the start of the statement that requires this
+/// condition, e.g., the "for" in a for loop.
+///
+/// \param ConvertToBoolean Whether the condition expression should be
+/// converted to a boolean value.
+///
 /// \returns true if there was a parsing, false otherwise.
 bool Parser::ParseCXXCondition(OwningExprResult &ExprResult,
-                               DeclPtrTy &DeclResult) {
+                               DeclPtrTy &DeclResult,
+                               SourceLocation Loc,
+                               bool ConvertToBoolean) {
   if (Tok.is(tok::code_completion)) {
     Actions.CodeCompleteOrdinaryName(CurScope, Action::CCC_Condition);
     ConsumeToken();
   }
 
   if (!isCXXConditionDeclaration()) {
+    // Parse the expression.
     ExprResult = ParseExpression(); // expression
     DeclResult = DeclPtrTy();
+    if (ExprResult.isInvalid())
+      return true;
+
+    // If required, convert to a boolean value.
+    if (ConvertToBoolean)
+      ExprResult
+        = Actions.ActOnBooleanCondition(CurScope, Loc, move(ExprResult));
     return ExprResult.isInvalid();
   }
 
@@ -747,6 +763,9 @@ bool Parser::ParseCXXCondition(OwningExprResult &ExprResult,
     Diag(Tok, diag::err_expected_equal_after_declarator);
   }
   
+  // FIXME: Build a reference to this declaration? Convert it to bool?
+  // (This is currently handled by Sema).
+  
   return false;
 }
 
index 9b2227002f1bbf86114b5b1cda1e0921a7e00c34..71bda78f6cbf0426b8c48b3812dc9fcbdb0290fe 100644 (file)
@@ -536,15 +536,23 @@ Parser::OwningStmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
 /// successfully parsed.  Note that a successful parse can still have semantic
 /// errors in the condition.
 bool Parser::ParseParenExprOrCondition(OwningExprResult &ExprResult,
-                                       DeclPtrTy &DeclResult) {
+                                       DeclPtrTy &DeclResult,
+                                       SourceLocation Loc,
+                                       bool ConvertToBoolean) {
   bool ParseError = false;
   
   SourceLocation LParenLoc = ConsumeParen();
   if (getLang().CPlusPlus) 
-    ParseError = ParseCXXCondition(ExprResult, DeclResult);
+    ParseError = ParseCXXCondition(ExprResult, DeclResult, Loc, 
+                                   ConvertToBoolean);
   else {
     ExprResult = ParseExpression();
     DeclResult = DeclPtrTy();
+    
+    // If required, convert to a boolean value.
+    if (!ExprResult.isInvalid() && ConvertToBoolean)
+      ExprResult
+        = Actions.ActOnBooleanCondition(CurScope, Loc, move(ExprResult));
   }
 
   // If the parser was confused by the condition and we don't have a ')', try to
@@ -603,7 +611,7 @@ Parser::OwningStmtResult Parser::ParseIfStatement(AttributeList *Attr) {
   // Parse the condition.
   OwningExprResult CondExp(Actions);
   DeclPtrTy CondVar;
-  if (ParseParenExprOrCondition(CondExp, CondVar))
+  if (ParseParenExprOrCondition(CondExp, CondVar, IfLoc, true))
     return StmtError();
 
   FullExprArg FullCondExp(Actions.MakeFullExpr(CondExp));
@@ -735,13 +743,24 @@ Parser::OwningStmtResult Parser::ParseSwitchStatement(AttributeList *Attr) {
   // Parse the condition.
   OwningExprResult Cond(Actions);
   DeclPtrTy CondVar;
-  if (ParseParenExprOrCondition(Cond, CondVar))
+  if (ParseParenExprOrCondition(Cond, CondVar, SwitchLoc, false))
     return StmtError();
 
-  FullExprArg FullCond(Actions.MakeFullExpr(Cond));
+  OwningStmtResult Switch
+    = Actions.ActOnStartOfSwitchStmt(SwitchLoc, move(Cond), CondVar);
+
+  if (Switch.isInvalid()) {
+    // Skip the switch body. 
+    // FIXME: This is not optimal recovery, but parsing the body is more
+    // dangerous due to the presence of case and default statements, which
+    // will have no place to connect back with the switch.
+    if (Tok.is(tok::l_brace))
+      MatchRHSPunctuation(tok::r_brace, ConsumeBrace());
+    else
+      SkipUntil(tok::semi);
+    return move(Switch);
+  }
   
-  OwningStmtResult Switch = Actions.ActOnStartOfSwitchStmt(FullCond, CondVar);
-
   // C99 6.8.4p3 - In C99, the body of the switch statement is a scope, even if
   // there is no compound stmt.  C90 does not have this clause.  We only do this
   // if the body isn't a compound statement to avoid push/pop in common cases.
@@ -763,11 +782,6 @@ Parser::OwningStmtResult Parser::ParseSwitchStatement(AttributeList *Attr) {
   InnerScope.Exit();
   SwitchScope.Exit();
 
-  if (Cond.isInvalid() && !CondVar.get()) {
-    Actions.ActOnSwitchBodyError(SwitchLoc, move(Switch), move(Body));
-    return StmtError();
-  }
-
   if (Body.isInvalid())
     // FIXME: Remove the case statement list from the Switch statement.
     Body = Actions.ActOnNullStmt(Tok.getLocation());
@@ -818,7 +832,7 @@ Parser::OwningStmtResult Parser::ParseWhileStatement(AttributeList *Attr) {
   // Parse the condition.
   OwningExprResult Cond(Actions);
   DeclPtrTy CondVar;
-  if (ParseParenExprOrCondition(Cond, CondVar))
+  if (ParseParenExprOrCondition(Cond, CondVar, WhileLoc, true))
     return StmtError();
 
   FullExprArg FullCond(Actions.MakeFullExpr(Cond));
@@ -975,7 +989,9 @@ Parser::OwningStmtResult Parser::ParseForStatement(AttributeList *Attr) {
 
   bool ForEach = false;
   OwningStmtResult FirstPart(Actions);
-  OwningExprResult SecondPart(Actions), ThirdPart(Actions);
+  FullExprArg SecondPart(Actions);
+  OwningExprResult Collection(Actions);
+  FullExprArg ThirdPart(Actions);
   DeclPtrTy SecondVar;
   
   if (Tok.is(tok::code_completion)) {
@@ -1009,7 +1025,7 @@ Parser::OwningStmtResult Parser::ParseForStatement(AttributeList *Attr) {
       Actions.ActOnForEachDeclStmt(DG);
       // ObjC: for (id x in expr)
       ConsumeToken(); // consume 'in'
-      SecondPart = ParseExpression();
+      Collection = ParseExpression();
     } else {
       Diag(Tok, diag::err_expected_semi_for);
       SkipUntil(tok::semi);
@@ -1025,35 +1041,43 @@ Parser::OwningStmtResult Parser::ParseForStatement(AttributeList *Attr) {
       ConsumeToken();
     } else if ((ForEach = isTokIdentifier_in())) {
       ConsumeToken(); // consume 'in'
-      SecondPart = ParseExpression();
+      Collection = ParseExpression();
     } else {
       if (!Value.isInvalid()) Diag(Tok, diag::err_expected_semi_for);
       SkipUntil(tok::semi);
     }
   }
   if (!ForEach) {
-    assert(!SecondPart.get() && "Shouldn't have a second expression yet.");
+    assert(!SecondPart->get() && "Shouldn't have a second expression yet.");
     // Parse the second part of the for specifier.
     if (Tok.is(tok::semi)) {  // for (...;;
       // no second part.
     } else {
+      OwningExprResult Second(Actions);
       if (getLang().CPlusPlus)
-        ParseCXXCondition(SecondPart, SecondVar);
-      else
-        SecondPart = ParseExpression();
+        ParseCXXCondition(Second, SecondVar, ForLoc, true);
+      else {
+        Second = ParseExpression();
+        if (!Second.isInvalid())
+          Second = Actions.ActOnBooleanCondition(CurScope, ForLoc, 
+                                                 move(Second));
+      }
+      SecondPart = Actions.MakeFullExpr(Second);
     }
 
     if (Tok.is(tok::semi)) {
       ConsumeToken();
     } else {
-      if (!SecondPart.isInvalid() || SecondVar.get()) 
+      if (!SecondPart->isInvalid() || SecondVar.get()) 
         Diag(Tok, diag::err_expected_semi_for);
       SkipUntil(tok::semi);
     }
 
     // Parse the third part of the for specifier.
-    if (Tok.isNot(tok::r_paren))    // for (...;...;)
-      ThirdPart = ParseExpression();
+    if (Tok.isNot(tok::r_paren)) {   // for (...;...;)
+      OwningExprResult Third = ParseExpression();
+      ThirdPart = Actions.MakeFullExpr(Third);
+    }
   }
   // Match the ')'.
   SourceLocation RParenLoc = MatchRHSPunctuation(tok::r_paren, LParenLoc);
@@ -1085,15 +1109,14 @@ Parser::OwningStmtResult Parser::ParseForStatement(AttributeList *Attr) {
     return StmtError();
 
   if (!ForEach)
-    return Actions.ActOnForStmt(ForLoc, LParenLoc, move(FirstPart),
-                                Actions.MakeFullExpr(SecondPart), SecondVar,
-                                Actions.MakeFullExpr(ThirdPart), RParenLoc, 
-                                move(Body));
-
-  return Actions.ActOnObjCForCollectionStmt(ForLoc, LParenLoc,
-                                            move(FirstPart),
-                                            move(SecondPart),
-                                            RParenLoc, move(Body));
+    return Actions.ActOnForStmt(ForLoc, LParenLoc, move(FirstPart), SecondPart,
+                                SecondVar, ThirdPart, RParenLoc, move(Body));
+
+  // FIXME: It isn't clear how to communicate the late destruction of 
+  // C++ temporaries used to create the collection.
+  return Actions.ActOnObjCForCollectionStmt(ForLoc, LParenLoc, move(FirstPart), 
+                                            move(Collection), RParenLoc, 
+                                            move(Body));
 }
 
 /// ParseGotoStatement
index 8656f4883acdbe9c1bd08340860d72b87fac5db4..1bc2a72179f2caca85ec442e2abfe8cc02a0d7a0 100644 (file)
@@ -1661,10 +1661,9 @@ public:
                                        FullExprArg CondVal, DeclPtrTy CondVar,
                                        StmtArg ThenVal,
                                        SourceLocation ElseLoc, StmtArg ElseVal);
-  virtual OwningStmtResult ActOnStartOfSwitchStmt(FullExprArg Cond,
+  virtual OwningStmtResult ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
+                                                  ExprArg Cond,
                                                   DeclPtrTy CondVar);
-  virtual void ActOnSwitchBodyError(SourceLocation SwitchLoc, StmtArg Switch,
-                                    StmtArg Body);
   virtual OwningStmtResult ActOnFinishSwitchStmt(SourceLocation SwitchLoc,
                                                  StmtArg Switch, StmtArg Body);
   virtual OwningStmtResult ActOnWhileStmt(SourceLocation WhileLoc,
@@ -2321,7 +2320,9 @@ public:
 
   virtual DeclResult ActOnCXXConditionDeclaration(Scope *S,
                                                   Declarator &D);
-  OwningExprResult CheckConditionVariable(VarDecl *ConditionVar);
+  OwningExprResult CheckConditionVariable(VarDecl *ConditionVar,
+                                          SourceLocation StmtLoc,
+                                          bool ConvertToBoolean);
 
   /// ActOnUnaryTypeTrait - Parsed one of the unary type trait support
   /// pseudo-functions.
@@ -4312,6 +4313,9 @@ public:
   /// \return true iff there were any errors
   bool CheckBooleanCondition(Expr *&CondExpr, SourceLocation Loc);
 
+  virtual OwningExprResult ActOnBooleanCondition(Scope *S, SourceLocation Loc,
+                                                 ExprArg SubExpr);
+  
   /// DiagnoseAssignmentAsCondition - Given that an expression is
   /// being used as a boolean condition, warn if it's an assignment.
   void DiagnoseAssignmentAsCondition(Expr *E);
index 8bfb4ca1004262c7322c7716e1563b82e3968d3a..40c1ee9e6892c3e5b39914ecdd5a6f1b48d9746c 100644 (file)
@@ -7694,3 +7694,17 @@ bool Sema::CheckBooleanCondition(Expr *&E, SourceLocation Loc) {
 
   return false;
 }
+
+Sema::OwningExprResult Sema::ActOnBooleanCondition(Scope *S, SourceLocation Loc,
+                                                   ExprArg SubExpr) {
+  if (SubExpr.isInvalid())
+    return ExprError();
+  
+  Expr *Sub = SubExpr.takeAs<Expr>();
+  if (CheckBooleanCondition(Sub, Loc)) {
+    Sub->Destroy(Context);
+    return ExprError();
+  }
+  
+  return Owned(Sub);
+}
index 217d2307aa06a58163c4619582ef189c506ef8ff..35b38edbcaee4e21a787087dfe9f2bc7569ed7fe 100644 (file)
@@ -1437,7 +1437,9 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
 
 /// \brief Check the use of the given variable as a C++ condition in an if,
 /// while, do-while, or switch statement.
-Action::OwningExprResult Sema::CheckConditionVariable(VarDecl *ConditionVar) {
+Action::OwningExprResult Sema::CheckConditionVariable(VarDecl *ConditionVar,
+                                                      SourceLocation StmtLoc,
+                                                      bool ConvertToBoolean) {
   QualType T = ConditionVar->getType();
   
   // C++ [stmt.select]p2:
@@ -1451,9 +1453,15 @@ Action::OwningExprResult Sema::CheckConditionVariable(VarDecl *ConditionVar) {
                           diag::err_invalid_use_of_array_type)
                      << ConditionVar->getSourceRange());
 
-  return Owned(DeclRefExpr::Create(Context, 0, SourceRange(), ConditionVar,
-                                   ConditionVar->getLocation(), 
-                                ConditionVar->getType().getNonReferenceType()));
+  Expr *Condition = DeclRefExpr::Create(Context, 0, SourceRange(), ConditionVar,
+                                        ConditionVar->getLocation(), 
+                                 ConditionVar->getType().getNonReferenceType());
+  if (ConvertToBoolean && CheckBooleanCondition(Condition, StmtLoc)) {
+    Condition->Destroy(Context);
+    return ExprError();
+  }
+  
+  return Owned(Condition);
 }
 
 /// CheckCXXBooleanCondition - Returns true if a conversion to bool is invalid.
@@ -2941,6 +2949,9 @@ CXXMemberCallExpr *Sema::BuildCXXMemberCallExpr(Expr *Exp,
 }
 
 Sema::OwningExprResult Sema::ActOnFinishFullExpr(ExprArg Arg) {
+  if (Arg.isInvalid())
+    return ExprError();
+
   Expr *FullExpr = Arg.takeAs<Expr>();
   if (FullExpr)
     FullExpr = MaybeCreateCXXExprWithTemporaries(FullExpr);
index 9d6132d050c636968f818f751d67e678f39a2358..3f75af685ed2dbb554452153768b799f9f7657b8 100644 (file)
@@ -280,7 +280,7 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, DeclPtrTy CondVar,
   VarDecl *ConditionVar = 0;
   if (CondVar.get()) {
     ConditionVar = CondVar.getAs<VarDecl>();
-    CondResult = CheckConditionVariable(ConditionVar);
+    CondResult = CheckConditionVariable(ConditionVar, IfLoc, true);
     if (CondResult.isInvalid())
       return StmtError();
   }
@@ -288,11 +288,6 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, DeclPtrTy CondVar,
   if (!ConditionExpr)
     return StmtError();
   
-  if (CheckBooleanCondition(ConditionExpr, IfLoc)) {
-    CondResult = ConditionExpr;
-    return StmtError();
-  }
-
   Stmt *thenStmt = ThenVal.takeAs<Stmt>();
   DiagnoseUnusedExprResult(thenStmt);
 
@@ -313,23 +308,6 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, DeclPtrTy CondVar,
                                     thenStmt, ElseLoc, elseStmt));
 }
 
-Action::OwningStmtResult
-Sema::ActOnStartOfSwitchStmt(FullExprArg cond, DeclPtrTy CondVar) {
-  OwningExprResult CondResult(cond.release());
-  
-  VarDecl *ConditionVar = 0;
-  if (CondVar.get()) {
-    ConditionVar = CondVar.getAs<VarDecl>();
-    CondResult = CheckConditionVariable(ConditionVar);
-    if (CondResult.isInvalid())
-      return StmtError();
-  }
-  SwitchStmt *SS = new (Context) SwitchStmt(ConditionVar, 
-                                            CondResult.takeAs<Expr>());
-  getSwitchStack().push_back(SS);
-  return Owned(SS);
-}
-
 /// ConvertIntegerToTypeWarnOnOverflow - Convert the specified APInt to have
 /// the specified width and sign.  If an overflow occurs, detect it and emit
 /// the specified diagnostic.
@@ -540,14 +518,34 @@ static bool CheckCXXSwitchCondition(Sema &S, SourceLocation SwitchLoc,
   return false;
 }
 
-/// ActOnSwitchBodyError - This is called if there is an error parsing the
-/// body of the switch stmt instead of ActOnFinishSwitchStmt.
-void Sema::ActOnSwitchBodyError(SourceLocation SwitchLoc, StmtArg Switch,
-                                StmtArg Body) {
-  // Keep the switch stack balanced.
-  assert(getSwitchStack().back() == (SwitchStmt*)Switch.get() &&
-         "switch stack missing push/pop!");
-  getSwitchStack().pop_back();
+Action::OwningStmtResult
+Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, ExprArg Cond, 
+                             DeclPtrTy CondVar) {
+  VarDecl *ConditionVar = 0;
+  if (CondVar.get()) {
+    ConditionVar = CondVar.getAs<VarDecl>();
+    Cond = CheckConditionVariable(ConditionVar, SourceLocation(), false);
+    if (Cond.isInvalid())
+      return StmtError();
+  }
+  
+  Expr *CondExpr = Cond.takeAs<Expr>();
+  if (!CondExpr)
+    return StmtError();
+  
+  if (getLangOptions().CPlusPlus && 
+      CheckCXXSwitchCondition(*this, SwitchLoc, CondExpr))
+    return StmtError();  
+
+  if (!CondVar.get()) {
+    CondExpr = MaybeCreateCXXExprWithTemporaries(CondExpr);
+    if (!CondExpr)
+      return StmtError();
+  }
+    
+  SwitchStmt *SS = new (Context) SwitchStmt(ConditionVar, CondExpr);
+  getSwitchStack().push_back(SS);
+  return Owned(SS);
 }
 
 Action::OwningStmtResult
@@ -570,10 +568,6 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, StmtArg Switch,
   QualType CondTypeBeforePromotion =
       GetTypeBeforeIntegralPromotion(CondExpr);
 
-  if (getLangOptions().CPlusPlus &&
-      CheckCXXSwitchCondition(*this, SwitchLoc, CondExpr))
-    return StmtError();
-
   // C99 6.8.4.2p5 - Integer promotions are performed on the controlling expr.
   UsualUnaryConversions(CondExpr);
   QualType CondType = CondExpr->getType();
@@ -870,7 +864,7 @@ Sema::ActOnWhileStmt(SourceLocation WhileLoc, FullExprArg Cond,
   VarDecl *ConditionVar = 0;
   if (CondVar.get()) {
     ConditionVar = CondVar.getAs<VarDecl>();
-    CondResult = CheckConditionVariable(ConditionVar);
+    CondResult = CheckConditionVariable(ConditionVar, WhileLoc, true);
     if (CondResult.isInvalid())
       return StmtError();
   }
@@ -878,11 +872,6 @@ Sema::ActOnWhileStmt(SourceLocation WhileLoc, FullExprArg Cond,
   if (!ConditionExpr)
     return StmtError();
   
-  if (CheckBooleanCondition(ConditionExpr, WhileLoc)) {
-    CondResult = ConditionExpr;
-    return StmtError();
-  }
-
   Stmt *bodyStmt = Body.takeAs<Stmt>();
   DiagnoseUnusedExprResult(bodyStmt);
 
@@ -903,6 +892,10 @@ Sema::ActOnDoStmt(SourceLocation DoLoc, StmtArg Body,
     return StmtError();
   }
 
+  condExpr = MaybeCreateCXXExprWithTemporaries(condExpr);
+  if (!condExpr)
+    return StmtError();
+  
   Stmt *bodyStmt = Body.takeAs<Stmt>();
   DiagnoseUnusedExprResult(bodyStmt);
 
@@ -939,17 +932,11 @@ Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
   VarDecl *ConditionVar = 0;
   if (secondVar.get()) {
     ConditionVar = secondVar.getAs<VarDecl>();
-    SecondResult = CheckConditionVariable(ConditionVar);
+    SecondResult = CheckConditionVariable(ConditionVar, ForLoc, true);
     if (SecondResult.isInvalid())
       return StmtError();
   }
   
-  Expr *Second = SecondResult.takeAs<Expr>();
-  if (Second && CheckBooleanCondition(Second, ForLoc)) {
-    SecondResult = Second;
-    return StmtError();
-  }
-
   Expr *Third  = third.release().takeAs<Expr>();
   Stmt *Body  = static_cast<Stmt*>(body.get());
   
@@ -959,7 +946,8 @@ Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
 
   first.release();
   body.release();
-  return Owned(new (Context) ForStmt(First, Second, ConditionVar, Third, Body, 
+  return Owned(new (Context) ForStmt(First, SecondResult.takeAs<Expr>(), 
+                                     ConditionVar, Third, Body, 
                                      ForLoc, LParenLoc, RParenLoc));
 }
 
index cc9842a57ea69dfc0f512bbdbce2bf89b986d7b6..43ffbfb737b210af0e3c5c6d4717c5d73d29dc44 100644 (file)
@@ -758,10 +758,18 @@ 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::FullExprArg Cond,
+  OwningStmtResult RebuildIfStmt(SourceLocation IfLoc, Sema::ExprArg Cond,
                                  VarDecl *CondVar, StmtArg Then, 
                                  SourceLocation ElseLoc, StmtArg Else) {
-    return getSema().ActOnIfStmt(IfLoc, Cond, DeclPtrTy::make(CondVar), 
+    if (Cond.get()) {
+      // Convert the condition to a boolean value.
+      Cond = getSema().ActOnBooleanCondition(0, IfLoc, move(Cond));
+      if (Cond.isInvalid())
+        return getSema().StmtError();
+    }
+    
+    Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond));
+    return getSema().ActOnIfStmt(IfLoc, FullCond, DeclPtrTy::make(CondVar), 
                                  move(Then), ElseLoc, move(Else));
   }
 
@@ -769,9 +777,11 @@ public:
   ///
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
-  OwningStmtResult RebuildSwitchStmtStart(Sema::FullExprArg Cond, 
+  OwningStmtResult RebuildSwitchStmtStart(SourceLocation SwitchLoc,
+                                          Sema::ExprArg Cond, 
                                           VarDecl *CondVar) {
-    return getSema().ActOnStartOfSwitchStmt(Cond, DeclPtrTy::make(CondVar));
+    return getSema().ActOnStartOfSwitchStmt(SwitchLoc, move(Cond), 
+                                            DeclPtrTy::make(CondVar));
   }
 
   /// \brief Attach the body to the switch statement.
@@ -789,11 +799,19 @@ 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::FullExprArg Cond,
+                                    Sema::ExprArg Cond,
                                     VarDecl *CondVar,
                                     StmtArg Body) {
-    return getSema().ActOnWhileStmt(WhileLoc, Cond, DeclPtrTy::make(CondVar),
-                                    move(Body));
+    if (Cond.get()) {
+      // Convert the condition to a boolean value.
+      Cond = getSema().ActOnBooleanCondition(0, WhileLoc, move(Cond));
+      if (Cond.isInvalid())
+        return getSema().StmtError();
+    }
+    
+    Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond));
+    return getSema().ActOnWhileStmt(WhileLoc, FullCond, 
+                                    DeclPtrTy::make(CondVar), move(Body));
   }
 
   /// \brief Build a new do-while statement.
@@ -815,10 +833,18 @@ public:
   /// Subclasses may override this routine to provide different behavior.
   OwningStmtResult RebuildForStmt(SourceLocation ForLoc,
                                   SourceLocation LParenLoc,
-                                  StmtArg Init, Sema::FullExprArg Cond, 
+                                  StmtArg Init, Sema::ExprArg Cond, 
                                   VarDecl *CondVar, Sema::FullExprArg Inc,
                                   SourceLocation RParenLoc, StmtArg Body) {
-    return getSema().ActOnForStmt(ForLoc, LParenLoc, move(Init), Cond, 
+    if (Cond.get()) {
+      // Convert the condition to a boolean value.
+      Cond = getSema().ActOnBooleanCondition(0, ForLoc, move(Cond));
+      if (Cond.isInvalid())
+        return getSema().StmtError();
+    }
+    
+    Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond));
+    return getSema().ActOnForStmt(ForLoc, LParenLoc, move(Init), FullCond, 
                                   DeclPtrTy::make(CondVar),
                                   Inc, RParenLoc, move(Body));
   }
@@ -3491,8 +3517,6 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
       return SemaRef.StmtError();
   }
   
-  Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond));
-
   // Transform the "then" branch.
   OwningStmtResult Then = getDerived().TransformStmt(S->getThen());
   if (Then.isInvalid())
@@ -3504,13 +3528,13 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
     return SemaRef.StmtError();
 
   if (!getDerived().AlwaysRebuild() &&
-      FullCond->get() == S->getCond() &&
+      Cond.get() == S->getCond() &&
       ConditionVar == S->getConditionVariable() &&
       Then.get() == S->getThen() &&
       Else.get() == S->getElse())
     return SemaRef.Owned(S->Retain());
 
-  return getDerived().RebuildIfStmt(S->getIfLoc(), FullCond, ConditionVar,
+  return getDerived().RebuildIfStmt(S->getIfLoc(), move(Cond), ConditionVar,
                                     move(Then),
                                     S->getElseLoc(), move(Else));
 }
@@ -3536,11 +3560,10 @@ TreeTransform<Derived>::TransformSwitchStmt(SwitchStmt *S) {
       return SemaRef.StmtError();
   }
 
-  Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond));
-  
   // Rebuild the switch statement.
-  OwningStmtResult Switch = getDerived().RebuildSwitchStmtStart(FullCond,
-                                                                ConditionVar);
+  OwningStmtResult Switch
+    = getDerived().RebuildSwitchStmtStart(S->getSwitchLoc(), move(Cond),
+                                          ConditionVar);
   if (Switch.isInvalid())
     return SemaRef.StmtError();
 
@@ -3575,21 +3598,19 @@ TreeTransform<Derived>::TransformWhileStmt(WhileStmt *S) {
       return SemaRef.StmtError();
   }
 
-  Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond));
-
   // Transform the body
   OwningStmtResult Body = getDerived().TransformStmt(S->getBody());
   if (Body.isInvalid())
     return SemaRef.StmtError();
 
   if (!getDerived().AlwaysRebuild() &&
-      FullCond->get() == S->getCond() &&
+      Cond.get() == S->getCond() &&
       ConditionVar == S->getConditionVariable() &&
       Body.get() == S->getBody())
     return SemaRef.Owned(S->Retain());
 
-  return getDerived().RebuildWhileStmt(S->getWhileLoc(), FullCond, ConditionVar,
-                                       move(Body));
+  return getDerived().RebuildWhileStmt(S->getWhileLoc(), move(Cond),
+                                       ConditionVar, move(Body));
 }
 
 template<typename Derived>
@@ -3659,8 +3680,7 @@ TreeTransform<Derived>::TransformForStmt(ForStmt *S) {
     return SemaRef.Owned(S->Retain());
 
   return getDerived().RebuildForStmt(S->getForLoc(), S->getLParenLoc(),
-                                     move(Init), getSema().MakeFullExpr(Cond),
-                                     ConditionVar,
+                                     move(Init), move(Cond), ConditionVar,
                                      getSema().MakeFullExpr(Inc),
                                      S->getRParenLoc(), move(Body));
 }
index e435408aa8409cc4cdb34413b82095c00f0b5cb0..0ebb22a6f93bbef74440ff7c9d09c7657adee2df 100644 (file)
@@ -23,6 +23,8 @@ struct Y {
   ~Y();
 };
 
+X getX();
+
 void if_destruct(int z) {
   // Verify that the condition variable is destroyed at the end of the
   // "if" statement.
@@ -44,6 +46,14 @@ void if_destruct(int z) {
   // CHECK: call  void @_ZN1YD1Ev
   // CHECK: br
   // CHECK: call  void @_ZN1XD1Ev
+
+  // CHECK: call void @_Z4getXv
+  // CHECK: call zeroext i1 @_ZN1XcvbEv
+  // CHECK: call void @_ZN1XD1Ev
+  // CHECK: br
+  if (getX()) { }
+
+  // CHECK: ret
 }
 
 struct ConvertibleToInt {
@@ -52,6 +62,8 @@ struct ConvertibleToInt {
   operator int();
 };
 
+ConvertibleToInt getConvToInt();
+
 void switch_destruct(int z) {
   // CHECK: call void @_ZN16ConvertibleToIntC1Ev
   switch (ConvertibleToInt conv = ConvertibleToInt()) {
@@ -68,6 +80,17 @@ void switch_destruct(int z) {
   // CHECK: call void @_ZN16ConvertibleToIntD1Ev
   // CHECK: store i32 20
   z = 20;
+
+  // CHECK: call void @_Z12getConvToIntv
+  // CHECK: call i32 @_ZN16ConvertibleToIntcviEv
+  // CHECK: call void @_ZN16ConvertibleToIntD1Ev
+  switch(getConvToInt()) {
+  case 0:
+    break;
+  }
+  // CHECK: store i32 27
+  z = 27;
+  // CHECK: ret
 }
 
 int foo();
@@ -88,6 +111,17 @@ void while_destruct(int z) {
   // CHECK: {{while.end|:7}}
   // CHECK: store i32 22
   z = 22;
+
+  // CHECK: call void @_Z4getXv
+  // CHECK: call zeroext i1 @_ZN1XcvbEv
+  // CHECK: call void @_ZN1XD1Ev
+  // CHECK: br
+  while(getX()) { }
+
+  // CHECK: store i32 25
+  z = 25;
+
+  // CHECK: ret
 }
 
 void for_destruct(int z) {
@@ -107,4 +141,33 @@ void for_destruct(int z) {
   // CHECK: call void @_ZN1YD1Ev
   // CHECK: store i32 24
   z = 24;
+
+  // CHECK: call void @_Z4getXv
+  // CHECK: call zeroext i1 @_ZN1XcvbEv
+  // CHECK: call void @_ZN1XD1Ev
+  // CHECK: br
+  // CHECK: call void @_Z4getXv
+  // CHECK: load
+  // CHECK: add
+  // CHECK: call void @_ZN1XD1Ev
+  int i = 0;
+  for(; getX(); getX(), ++i) { }
+  z = 26;
+  // CHECK: store i32 26
+  // CHECK: ret
+}
+
+void do_destruct(int z) {
+  // CHECK: define void @_Z11do_destruct
+  do {
+    // CHECK: store i32 77
+    z = 77;
+    // CHECK: call void @_Z4getXv
+    // CHECK: call zeroext i1 @_ZN1XcvbEv
+    // CHECK: call void @_ZN1XD1Ev
+    // CHECK: br
+  } while (getX());
+  // CHECK: store i32 99
+  z = 99;
+  // CHECK: ret
 }