[cfe-commits] r103187 - in /cfe/trunk: 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

Douglas Gregor dgregor at apple.com
Thu May 6 10:25:47 PDT 2010


Author: dgregor
Date: Thu May  6 12:25:47 2010
New Revision: 103187

URL: http://llvm.org/viewvc/llvm-project?rev=103187&view=rev
Log:
Rework our handling of temporary objects within the conditions of
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.




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

Modified: cfe/trunk/include/clang/Parse/Action.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Action.h?rev=103187&r1=103186&r2=103187&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Action.h (original)
+++ cfe/trunk/include/clang/Parse/Action.h Thu May  6 12:25:47 2010
@@ -105,12 +105,19 @@
 
   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 @@
 
   /// \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 @@
     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

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=103187&r1=103186&r2=103187&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Thu May  6 12:25:47 2010
@@ -1004,7 +1004,8 @@
 
   //===--------------------------------------------------------------------===//
   // 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 @@
                                           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);

Modified: cfe/trunk/lib/Frontend/PrintParserCallbacks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PrintParserCallbacks.cpp?rev=103187&r1=103186&r2=103187&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/PrintParserCallbacks.cpp (original)
+++ cfe/trunk/lib/Frontend/PrintParserCallbacks.cpp Thu May  6 12:25:47 2010
@@ -315,7 +315,8 @@
       return StmtEmpty();
     }
 
-    virtual OwningStmtResult ActOnStartOfSwitchStmt(FullExprArg Cond, 
+    virtual OwningStmtResult ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
+                                                    ExprArg Cond, 
                                                     DeclPtrTy CondVar) {
       Out << __FUNCTION__ << "\n";
       return StmtEmpty();

Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=103187&r1=103186&r2=103187&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Thu May  6 12:25:47 2010
@@ -689,17 +689,33 @@
 /// \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 @@
     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;
 }
 

Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=103187&r1=103186&r2=103187&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Thu May  6 12:25:47 2010
@@ -536,15 +536,23 @@
 /// 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 @@
   // 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 @@
   // 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(FullCond, CondVar);
+  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);
+  }
+  
   // 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 @@
   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 @@
   // 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 @@
 
   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 @@
       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 @@
       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 @@
     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

Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=103187&r1=103186&r2=103187&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Thu May  6 12:25:47 2010
@@ -1661,10 +1661,9 @@
                                        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 @@
 
   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 @@
   /// \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);

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=103187&r1=103186&r2=103187&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu May  6 12:25:47 2010
@@ -7694,3 +7694,17 @@
 
   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);
+}

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=103187&r1=103186&r2=103187&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Thu May  6 12:25:47 2010
@@ -1437,7 +1437,9 @@
 
 /// \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 @@
                           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 @@
 }
 
 Sema::OwningExprResult Sema::ActOnFinishFullExpr(ExprArg Arg) {
+  if (Arg.isInvalid())
+    return ExprError();
+
   Expr *FullExpr = Arg.takeAs<Expr>();
   if (FullExpr)
     FullExpr = MaybeCreateCXXExprWithTemporaries(FullExpr);

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=103187&r1=103186&r2=103187&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu May  6 12:25:47 2010
@@ -280,7 +280,7 @@
   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 @@
   if (!ConditionExpr)
     return StmtError();
   
-  if (CheckBooleanCondition(ConditionExpr, IfLoc)) {
-    CondResult = ConditionExpr;
-    return StmtError();
-  }
-
   Stmt *thenStmt = ThenVal.takeAs<Stmt>();
   DiagnoseUnusedExprResult(thenStmt);
 
@@ -313,23 +308,6 @@
                                     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 @@
   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 @@
   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 @@
   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 @@
   if (!ConditionExpr)
     return StmtError();
   
-  if (CheckBooleanCondition(ConditionExpr, WhileLoc)) {
-    CondResult = ConditionExpr;
-    return StmtError();
-  }
-
   Stmt *bodyStmt = Body.takeAs<Stmt>();
   DiagnoseUnusedExprResult(bodyStmt);
 
@@ -903,6 +892,10 @@
     return StmtError();
   }
 
+  condExpr = MaybeCreateCXXExprWithTemporaries(condExpr);
+  if (!condExpr)
+    return StmtError();
+  
   Stmt *bodyStmt = Body.takeAs<Stmt>();
   DiagnoseUnusedExprResult(bodyStmt);
 
@@ -939,17 +932,11 @@
   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 @@
 
   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));
 }
 

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=103187&r1=103186&r2=103187&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Thu May  6 12:25:47 2010
@@ -758,10 +758,18 @@
   ///
   /// 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 @@
   ///
   /// 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 @@
   /// 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 @@
   /// 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 @@
       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 @@
     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 @@
       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 @@
       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 @@
     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));
 }

Modified: cfe/trunk/test/CodeGenCXX/condition.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/condition.cpp?rev=103187&r1=103186&r2=103187&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/condition.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/condition.cpp Thu May  6 12:25:47 2010
@@ -23,6 +23,8 @@
   ~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 @@
   // 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 @@
   operator int();
 };
 
+ConvertibleToInt getConvToInt();
+
 void switch_destruct(int z) {
   // CHECK: call void @_ZN16ConvertibleToIntC1Ev
   switch (ConvertibleToInt conv = ConvertibleToInt()) {
@@ -68,6 +80,17 @@
   // 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 @@
   // 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 @@
   // 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
 }





More information about the cfe-commits mailing list