From: Douglas Gregor Date: Thu, 6 May 2010 17:25:47 +0000 (+0000) Subject: Rework our handling of temporary objects within the conditions of X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=586596fd7f7a336a2847b300c80614dcf39ab6d5;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@103187 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Parse/Action.h b/include/clang/Parse/Action.h index 98c9538d5e..afa8bb6254 100644 --- a/include/clang/Parse/Action.h +++ b/include/clang/Parse/Action.h @@ -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(Other).Expr)) {} + FullExprArg &operator=(const FullExprArg& Other) { + Expr = move(const_cast(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 diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index 0180419e04..5305709132 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -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); diff --git a/lib/Frontend/PrintParserCallbacks.cpp b/lib/Frontend/PrintParserCallbacks.cpp index 4df5c5263c..258d352452 100644 --- a/lib/Frontend/PrintParserCallbacks.cpp +++ b/lib/Frontend/PrintParserCallbacks.cpp @@ -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(); diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp index 9eb9506239..b7ccea53d5 100644 --- a/lib/Parse/ParseExprCXX.cpp +++ b/lib/Parse/ParseExprCXX.cpp @@ -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; } diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index 9b2227002f..71bda78f6c 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/lib/Parse/ParseStmt.cpp @@ -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 diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index 8656f4883a..1bc2a72179 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -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); diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 8bfb4ca100..40c1ee9e68 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -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(); + if (CheckBooleanCondition(Sub, Loc)) { + Sub->Destroy(Context); + return ExprError(); + } + + return Owned(Sub); +} diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index 217d2307aa..35b38edbca 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/lib/Sema/SemaExprCXX.cpp @@ -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(); if (FullExpr) FullExpr = MaybeCreateCXXExprWithTemporaries(FullExpr); diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index 9d6132d050..3f75af685e 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -280,7 +280,7 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, DeclPtrTy CondVar, VarDecl *ConditionVar = 0; if (CondVar.get()) { ConditionVar = CondVar.getAs(); - 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(); 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(); - CondResult = CheckConditionVariable(ConditionVar); - if (CondResult.isInvalid()) - return StmtError(); - } - SwitchStmt *SS = new (Context) SwitchStmt(ConditionVar, - CondResult.takeAs()); - 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(); + Cond = CheckConditionVariable(ConditionVar, SourceLocation(), false); + if (Cond.isInvalid()) + return StmtError(); + } + + Expr *CondExpr = Cond.takeAs(); + 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(); - 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(); 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(); DiagnoseUnusedExprResult(bodyStmt); @@ -939,17 +932,11 @@ Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc, VarDecl *ConditionVar = 0; if (secondVar.get()) { ConditionVar = secondVar.getAs(); - SecondResult = CheckConditionVariable(ConditionVar); + SecondResult = CheckConditionVariable(ConditionVar, ForLoc, true); if (SecondResult.isInvalid()) return StmtError(); } - Expr *Second = SecondResult.takeAs(); - if (Second && CheckBooleanCondition(Second, ForLoc)) { - SecondResult = Second; - return StmtError(); - } - Expr *Third = third.release().takeAs(); Stmt *Body = static_cast(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(), + ConditionVar, Third, Body, ForLoc, LParenLoc, RParenLoc)); } diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h index cc9842a57e..43ffbfb737 100644 --- a/lib/Sema/TreeTransform.h +++ b/lib/Sema/TreeTransform.h @@ -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::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::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::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::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 @@ -3659,8 +3680,7 @@ TreeTransform::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)); } diff --git a/test/CodeGenCXX/condition.cpp b/test/CodeGenCXX/condition.cpp index e435408aa8..0ebb22a6f9 100644 --- a/test/CodeGenCXX/condition.cpp +++ b/test/CodeGenCXX/condition.cpp @@ -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 }