r273548 - Rearrange condition handling so that semantic checks on a condition variable

Nico Weber via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 23 08:15:43 PDT 2016


This breaks Wfor-loop-analysis. Code (e.g. WebKit) has a pattern where a
class has an accessor that takes an index and returns an element if the
index is in bounds or nullptr else, and calling code passes indices
starting at 0 until a nullptr element is returned. That used to be fine,
but now clang warns:

../../third_party/WebKit/Source/core/html/HTMLInputElement.cpp(1843,49):
 error: variable 'option' used in loop condition not modified in loop body
[-Werror,-Wfor-loop-analysis]
        for (unsigned i = 0; HTMLOptionElement* option = options->item(i);
++i) {
                                                ^~~~~~

(in many other places too.)

Can you take a look?

On Thu, Jun 23, 2016 at 4:41 AM, Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: rsmith
> Date: Thu Jun 23 03:41:20 2016
> New Revision: 273548
>
> URL: http://llvm.org/viewvc/llvm-project?rev=273548&view=rev
> Log:
> Rearrange condition handling so that semantic checks on a condition
> variable
> are performed before the other substatements of the construct are parsed,
> rather than deferring them until the end. This allows better error recovery
> from semantic errors in the condition, improves diagnostic order, and is a
> prerequisite for C++17 constexpr if.
>
> Modified:
>     cfe/trunk/include/clang/Parse/Parser.h
>     cfe/trunk/include/clang/Sema/Sema.h
>     cfe/trunk/lib/Parse/ParseDeclCXX.cpp
>     cfe/trunk/lib/Parse/ParseExprCXX.cpp
>     cfe/trunk/lib/Parse/ParseStmt.cpp
>     cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>     cfe/trunk/lib/Sema/SemaExpr.cpp
>     cfe/trunk/lib/Sema/SemaExprCXX.cpp
>     cfe/trunk/lib/Sema/SemaOpenMP.cpp
>     cfe/trunk/lib/Sema/SemaStmt.cpp
>     cfe/trunk/lib/Sema/TreeTransform.h
>     cfe/trunk/test/FixIt/fixit-vexing-parse.cpp
>     cfe/trunk/test/Parser/cxx0x-condition.cpp
>     cfe/trunk/test/SemaCXX/crashes.cpp
>     cfe/trunk/test/SemaCXX/for-range-examples.cpp
>     cfe/trunk/test/SemaObjCXX/foreach.mm
>
> Modified: cfe/trunk/include/clang/Parse/Parser.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Parse/Parser.h (original)
> +++ cfe/trunk/include/clang/Parse/Parser.h Thu Jun 23 03:41:20 2016
> @@ -1588,8 +1588,8 @@ private:
>
>
>  //===--------------------------------------------------------------------===//
>    // C++ if/switch/while condition expression.
> -  bool ParseCXXCondition(ExprResult &ExprResult, Decl *&DeclResult,
> -                         SourceLocation Loc, bool ConvertToBoolean);
> +  Sema::ConditionResult ParseCXXCondition(SourceLocation Loc,
> +                                          Sema::ConditionKind CK);
>
>
>  //===--------------------------------------------------------------------===//
>    // C++ Coroutines
> @@ -1680,10 +1680,9 @@ private:
>                                      unsigned ScopeFlags);
>    void ParseCompoundStatementLeadingPragmas();
>    StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
> -  bool ParseParenExprOrCondition(ExprResult &ExprResult,
> -                                 Decl *&DeclResult,
> +  bool ParseParenExprOrCondition(Sema::ConditionResult &CondResult,
>                                   SourceLocation Loc,
> -                                 bool ConvertToBoolean);
> +                                 Sema::ConditionKind CK);
>    StmtResult ParseIfStatement(SourceLocation *TrailingElseLoc);
>    StmtResult ParseSwitchStatement(SourceLocation *TrailingElseLoc);
>    StmtResult ParseWhileStatement(SourceLocation *TrailingElseLoc);
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Thu Jun 23 03:41:20 2016
> @@ -3298,6 +3298,7 @@ public:
>  public:
>    class FullExprArg {
>    public:
> +    FullExprArg() : E(nullptr) { }
>      FullExprArg(Sema &actions) : E(nullptr) { }
>
>      ExprResult release() {
> @@ -3391,27 +3392,23 @@ public:
>                                   ArrayRef<const Attr*> Attrs,
>                                   Stmt *SubStmt);
>
> -  StmtResult ActOnIfStmt(SourceLocation IfLoc,
> -                         FullExprArg CondVal, Decl *CondVar,
> -                         Stmt *ThenVal,
> -                         SourceLocation ElseLoc, Stmt *ElseVal);
> +  class ConditionResult;
> +  StmtResult ActOnIfStmt(SourceLocation IfLoc, ConditionResult Cond,
> +                         Stmt *ThenVal, SourceLocation ElseLoc, Stmt
> *ElseVal);
>    StmtResult ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
> -                                            Expr *Cond,
> -                                            Decl *CondVar);
> +                                    ConditionResult Cond);
>    StmtResult ActOnFinishSwitchStmt(SourceLocation SwitchLoc,
>                                             Stmt *Switch, Stmt *Body);
> -  StmtResult ActOnWhileStmt(SourceLocation WhileLoc,
> -                            FullExprArg Cond,
> -                            Decl *CondVar, Stmt *Body);
> +  StmtResult ActOnWhileStmt(SourceLocation WhileLoc, ConditionResult Cond,
> +                            Stmt *Body);
>    StmtResult ActOnDoStmt(SourceLocation DoLoc, Stmt *Body,
> -                                 SourceLocation WhileLoc,
> -                                 SourceLocation CondLParen, Expr *Cond,
> -                                 SourceLocation CondRParen);
> +                         SourceLocation WhileLoc, SourceLocation
> CondLParen,
> +                         Expr *Cond, SourceLocation CondRParen);
>
>    StmtResult ActOnForStmt(SourceLocation ForLoc,
>                            SourceLocation LParenLoc,
> -                          Stmt *First, FullExprArg Second,
> -                          Decl *SecondVar,
> +                          Stmt *First,
> +                          ConditionResult Second,
>                            FullExprArg Third,
>                            SourceLocation RParenLoc,
>                            Stmt *Body);
> @@ -4801,11 +4798,6 @@ public:
>                              bool WarnOnNonAbstractTypes,
>                              SourceLocation DtorLoc);
>
> -  DeclResult ActOnCXXConditionDeclaration(Scope *S, Declarator &D);
> -  ExprResult CheckConditionVariable(VarDecl *ConditionVar,
> -                                    SourceLocation StmtLoc,
> -                                    bool ConvertToBoolean);
> -
>    ExprResult ActOnNoexceptExpr(SourceLocation KeyLoc, SourceLocation
> LParen,
>                                 Expr *Operand, SourceLocation RParen);
>    ExprResult BuildCXXNoexceptExpr(SourceLocation KeyLoc, Expr *Operand,
> @@ -8923,6 +8915,46 @@ public:
>    /// type, and if so, emit a note describing what happened.
>    void EmitRelatedResultTypeNoteForReturn(QualType destType);
>
> +  class ConditionResult {
> +    Decl *ConditionVar;
> +    FullExprArg Condition;
> +    bool Invalid;
> +
> +    friend class Sema;
> +    ConditionResult(Decl *ConditionVar, FullExprArg Condition)
> +        : ConditionVar(ConditionVar), Condition(Condition),
> Invalid(false) {}
> +    explicit ConditionResult(bool Invalid)
> +        : ConditionVar(nullptr), Condition(nullptr), Invalid(Invalid) {}
> +
> +  public:
> +    ConditionResult() : ConditionResult(false) {}
> +    bool isInvalid() const { return Invalid; }
> +    std::pair<VarDecl *, Expr *> get() const {
> +      return std::make_pair(cast_or_null<VarDecl>(ConditionVar),
> +                            Condition.get());
> +    }
> +  };
> +  static ConditionResult ConditionError() { return ConditionResult(true);
> }
> +
> +  enum class ConditionKind {
> +    Boolean, ///< A boolean condition, from 'if', 'while', 'for', or 'do'.
> +    Switch   ///< An integral condition for a 'switch' statement.
> +  };
> +
> +  ConditionResult ActOnCondition(Scope *S, SourceLocation Loc,
> +                                 Expr *SubExpr, ConditionKind CK);
> +
> +  ConditionResult ActOnConditionVariable(Decl *ConditionVar,
> +                                         SourceLocation StmtLoc,
> +                                         ConditionKind CK);
> +
> +  DeclResult ActOnCXXConditionDeclaration(Scope *S, Declarator &D);
> +
> +  ExprResult CheckConditionVariable(VarDecl *ConditionVar,
> +                                    SourceLocation StmtLoc,
> +                                    ConditionKind CK);
> +  ExprResult CheckSwitchCondition(SourceLocation SwitchLoc, Expr *Cond);
> +
>    /// CheckBooleanCondition - Diagnose problems involving the use of
>    /// the given expression as a boolean condition (e.g. in an if
>    /// statement).  Also performs the standard function and array
> @@ -8931,10 +8963,7 @@ public:
>    /// \param Loc - A location associated with the condition, e.g. the
>    /// 'if' keyword.
>    /// \return true iff there were any errors
> -  ExprResult CheckBooleanCondition(Expr *E, SourceLocation Loc);
> -
> -  ExprResult ActOnBooleanCondition(Scope *S, SourceLocation Loc,
> -                                   Expr *SubExpr);
> +  ExprResult CheckBooleanCondition(SourceLocation Loc, Expr *E);
>
>    /// DiagnoseAssignmentAsCondition - Given that an expression is
>    /// being used as a boolean condition, warn if it's an assignment.
>
> Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Thu Jun 23 03:41:20 2016
> @@ -3420,10 +3420,11 @@ Parser::tryParseExceptionSpecification(b
>      NoexceptExpr = ParseConstantExpression();
>      T.consumeClose();
>      // The argument must be contextually convertible to bool. We use
> -    // ActOnBooleanCondition for this purpose.
> +    // CheckBooleanCondition for this purpose.
> +    // FIXME: Add a proper Sema entry point for this.
>      if (!NoexceptExpr.isInvalid()) {
> -      NoexceptExpr = Actions.ActOnBooleanCondition(getCurScope(),
> KeywordLoc,
> -                                                   NoexceptExpr.get());
> +      NoexceptExpr =
> +          Actions.CheckBooleanCondition(KeywordLoc, NoexceptExpr.get());
>        NoexceptRange = SourceRange(KeywordLoc, T.getCloseLocation());
>      } else {
>        NoexceptType = EST_None;
>
> Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Thu Jun 23 03:41:20 2016
> @@ -1726,27 +1726,19 @@ Parser::ParseCXXTypeConstructExpression(
>  /// [GNU]   type-specifier-seq declarator simple-asm-expr[opt]
> attributes[opt]
>  ///             '=' assignment-expression
>  ///
> -/// \param ExprOut if the condition was parsed as an expression, the
> parsed
> -/// expression.
> -///
> -/// \param DeclOut 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(ExprResult &ExprOut,
> -                               Decl *&DeclOut,
> -                               SourceLocation Loc,
> -                               bool ConvertToBoolean) {
> +/// \returns The parsed condition.
> +Sema::ConditionResult Parser::ParseCXXCondition(SourceLocation Loc,
> +                                                Sema::ConditionKind CK) {
>    if (Tok.is(tok::code_completion)) {
>      Actions.CodeCompleteOrdinaryName(getCurScope(), Sema::PCC_Condition);
>      cutOffParsing();
> -    return true;
> +    return Sema::ConditionError();
>    }
>
>    ParsedAttributesWithRange attrs(AttrFactory);
> @@ -1756,16 +1748,11 @@ bool Parser::ParseCXXCondition(ExprResul
>      ProhibitAttributes(attrs);
>
>      // Parse the expression.
> -    ExprOut = ParseExpression(); // expression
> -    DeclOut = nullptr;
> -    if (ExprOut.isInvalid())
> -      return true;
> -
> -    // If required, convert to a boolean value.
> -    if (ConvertToBoolean)
> -      ExprOut
> -        = Actions.ActOnBooleanCondition(getCurScope(), Loc,
> ExprOut.get());
> -    return ExprOut.isInvalid();
> +    ExprResult Expr = ParseExpression(); // expression
> +    if (Expr.isInvalid())
> +      return Sema::ConditionError();
> +
> +    return Actions.ActOnCondition(getCurScope(), Loc, Expr.get(), CK);
>    }
>
>    // type-specifier-seq
> @@ -1783,7 +1770,7 @@ bool Parser::ParseCXXCondition(ExprResul
>      ExprResult AsmLabel(ParseSimpleAsm(&Loc));
>      if (AsmLabel.isInvalid()) {
>        SkipUntil(tok::semi, StopAtSemi);
> -      return true;
> +      return Sema::ConditionError();
>      }
>      DeclaratorInfo.setAsmLabel(AsmLabel.get());
>      DeclaratorInfo.SetRangeEnd(Loc);
> @@ -1795,8 +1782,9 @@ bool Parser::ParseCXXCondition(ExprResul
>    // Type-check the declaration itself.
>    DeclResult Dcl = Actions.ActOnCXXConditionDeclaration(getCurScope(),
>                                                          DeclaratorInfo);
> -  DeclOut = Dcl.get();
> -  ExprOut = ExprError();
> +  if (Dcl.isInvalid())
> +    return Sema::ConditionError();
> +  Decl *DeclOut = Dcl.get();
>
>    // '=' assignment-expression
>    // If a '==' or '+=' is found, suggest a fixit to '='.
> @@ -1816,12 +1804,11 @@ bool Parser::ParseCXXCondition(ExprResul
>      SourceLocation LParen = ConsumeParen(), RParen = LParen;
>      if (SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch))
>        RParen = ConsumeParen();
> -    Diag(DeclOut ? DeclOut->getLocation() : LParen,
> +    Diag(DeclOut->getLocation(),
>           diag::err_expected_init_in_condition_lparen)
>        << SourceRange(LParen, RParen);
>    } else {
> -    Diag(DeclOut ? DeclOut->getLocation() : Tok.getLocation(),
> -         diag::err_expected_init_in_condition);
> +    Diag(DeclOut->getLocation(), diag::err_expected_init_in_condition);
>    }
>
>    if (!InitExpr.isInvalid())
> @@ -1834,8 +1821,7 @@ bool Parser::ParseCXXCondition(ExprResul
>    // (This is currently handled by Sema).
>
>    Actions.FinalizeDeclaration(DeclOut);
> -
> -  return false;
> +  return Actions.ActOnConditionVariable(DeclOut, Loc, CK);
>  }
>
>  /// ParseCXXSimpleTypeSpecifier - [C++ 7.1.5.2] Simple type specifiers.
>
> Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseStmt.cpp Thu Jun 23 03:41:20 2016
> @@ -1052,29 +1052,28 @@ StmtResult Parser::ParseCompoundStatemen
>  /// should try to recover harder.  It returns false if the condition is
>  /// successfully parsed.  Note that a successful parse can still have
> semantic
>  /// errors in the condition.
> -bool Parser::ParseParenExprOrCondition(ExprResult &ExprResult,
> -                                       Decl *&DeclResult,
> +bool Parser::ParseParenExprOrCondition(Sema::ConditionResult &Cond,
>                                         SourceLocation Loc,
> -                                       bool ConvertToBoolean) {
> +                                       Sema::ConditionKind CK) {
>    BalancedDelimiterTracker T(*this, tok::l_paren);
>    T.consumeOpen();
>
>    if (getLangOpts().CPlusPlus)
> -    ParseCXXCondition(ExprResult, DeclResult, Loc, ConvertToBoolean);
> +    Cond = ParseCXXCondition(Loc, CK);
>    else {
> -    ExprResult = ParseExpression();
> -    DeclResult = nullptr;
> +    ExprResult CondExpr = ParseExpression();
>
>      // If required, convert to a boolean value.
> -    if (!ExprResult.isInvalid() && ConvertToBoolean)
> -      ExprResult
> -        = Actions.ActOnBooleanCondition(getCurScope(), Loc,
> ExprResult.get());
> +    if (CondExpr.isInvalid())
> +      Cond = Sema::ConditionError();
> +    else
> +      Cond = Actions.ActOnCondition(getCurScope(), Loc, CondExpr.get(),
> CK);
>    }
>
>    // If the parser was confused by the condition and we don't have a ')',
> try to
>    // recover by skipping ahead to a semi and bailing out.  If condexp is
>    // semantically invalid but we have well formed code, keep going.
> -  if (ExprResult.isInvalid() && !DeclResult && Tok.isNot(tok::r_paren)) {
> +  if (Cond.isInvalid() && Tok.isNot(tok::r_paren)) {
>      SkipUntil(tok::semi);
>      // Skipping may have stopped if it found the containing ')'.  If so,
> we can
>      // continue parsing the if statement.
> @@ -1132,13 +1131,10 @@ StmtResult Parser::ParseIfStatement(Sour
>    ParseScope IfScope(this, Scope::DeclScope | Scope::ControlScope,
> C99orCXX);
>
>    // Parse the condition.
> -  ExprResult CondExp;
> -  Decl *CondVar = nullptr;
> -  if (ParseParenExprOrCondition(CondExp, CondVar, IfLoc, true))
> +  Sema::ConditionResult Cond;
> +  if (ParseParenExprOrCondition(Cond, IfLoc,
> Sema::ConditionKind::Boolean))
>      return StmtError();
>
> -  FullExprArg FullCondExp(Actions.MakeFullExpr(CondExp.get(), IfLoc));
> -
>    // C99 6.8.4p3 - In C99, the body of the if 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.
> @@ -1221,8 +1217,8 @@ StmtResult Parser::ParseIfStatement(Sour
>    if (ElseStmt.isInvalid())
>      ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
>
> -  return Actions.ActOnIfStmt(IfLoc, FullCondExp, CondVar, ThenStmt.get(),
> -                             ElseLoc, ElseStmt.get());
> +  return Actions.ActOnIfStmt(IfLoc, Cond, ThenStmt.get(), ElseLoc,
> +                             ElseStmt.get());
>  }
>
>  /// ParseSwitchStatement
> @@ -1259,13 +1255,11 @@ StmtResult Parser::ParseSwitchStatement(
>    ParseScope SwitchScope(this, ScopeFlags);
>
>    // Parse the condition.
> -  ExprResult Cond;
> -  Decl *CondVar = nullptr;
> -  if (ParseParenExprOrCondition(Cond, CondVar, SwitchLoc, false))
> +  Sema::ConditionResult Cond;
> +  if (ParseParenExprOrCondition(Cond, SwitchLoc,
> Sema::ConditionKind::Switch))
>      return StmtError();
>
> -  StmtResult Switch
> -    = Actions.ActOnStartOfSwitchStmt(SwitchLoc, Cond.get(), CondVar);
> +  StmtResult Switch = Actions.ActOnStartOfSwitchStmt(SwitchLoc, Cond);
>
>    if (Switch.isInvalid()) {
>      // Skip the switch body.
> @@ -1347,13 +1341,10 @@ StmtResult Parser::ParseWhileStatement(S
>    ParseScope WhileScope(this, ScopeFlags);
>
>    // Parse the condition.
> -  ExprResult Cond;
> -  Decl *CondVar = nullptr;
> -  if (ParseParenExprOrCondition(Cond, CondVar, WhileLoc, true))
> +  Sema::ConditionResult Cond;
> +  if (ParseParenExprOrCondition(Cond, WhileLoc,
> Sema::ConditionKind::Boolean))
>      return StmtError();
>
> -  FullExprArg FullCond(Actions.MakeFullExpr(Cond.get(), WhileLoc));
> -
>    // C99 6.8.5p5 - In C99, the body of the while 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.
> @@ -1374,10 +1365,10 @@ StmtResult Parser::ParseWhileStatement(S
>    InnerScope.Exit();
>    WhileScope.Exit();
>
> -  if ((Cond.isInvalid() && !CondVar) || Body.isInvalid())
> +  if (Cond.isInvalid() || Body.isInvalid())
>      return StmtError();
>
> -  return Actions.ActOnWhileStmt(WhileLoc, FullCond, CondVar, Body.get());
> +  return Actions.ActOnWhileStmt(WhileLoc, Cond, Body.get());
>  }
>
>  /// ParseDoStatement
> @@ -1535,12 +1526,10 @@ StmtResult Parser::ParseForStatement(Sou
>
>    bool ForEach = false, ForRange = false;
>    StmtResult FirstPart;
> -  bool SecondPartIsInvalid = false;
> -  FullExprArg SecondPart(Actions);
> +  Sema::ConditionResult SecondPart;
>    ExprResult Collection;
>    ForRangeInit ForRangeInit;
>    FullExprArg ThirdPart(Actions);
> -  Decl *SecondVar = nullptr;
>
>    if (Tok.is(tok::code_completion)) {
>      Actions.CodeCompleteOrdinaryName(getCurScope(),
> @@ -1645,7 +1634,7 @@ StmtResult Parser::ParseForStatement(Sou
>        Diag(Tok, diag::err_for_range_expected_decl)
>          << FirstPart.get()->getSourceRange();
>        SkipUntil(tok::r_paren, StopBeforeMatch);
> -      SecondPartIsInvalid = true;
> +      SecondPart = Sema::ConditionError();
>      } else {
>        if (!Value.isInvalid()) {
>          Diag(Tok, diag::err_expected_semi_for);
> @@ -1660,29 +1649,28 @@ StmtResult Parser::ParseForStatement(Sou
>
>    // Parse the second part of the for specifier.
>    getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
> -  if (!ForEach && !ForRange) {
> -    assert(!SecondPart.get() && "Shouldn't have a second expression
> yet.");
> +  if (!ForEach && !ForRange && !SecondPart.isInvalid()) {
>      // Parse the second part of the for specifier.
>      if (Tok.is(tok::semi)) {  // for (...;;
>        // no second part.
>      } else if (Tok.is(tok::r_paren)) {
>        // missing both semicolons.
>      } else {
> -      ExprResult Second;
>        if (getLangOpts().CPlusPlus)
> -        ParseCXXCondition(Second, SecondVar, ForLoc, true);
> +        SecondPart = ParseCXXCondition(ForLoc,
> Sema::ConditionKind::Boolean);
>        else {
> -        Second = ParseExpression();
> -        if (!Second.isInvalid())
> -          Second = Actions.ActOnBooleanCondition(getCurScope(), ForLoc,
> -                                                 Second.get());
> +        ExprResult SecondExpr = ParseExpression();
> +        if (SecondExpr.isInvalid())
> +          SecondPart = Sema::ConditionError();
> +        else
> +          SecondPart =
> +              Actions.ActOnCondition(getCurScope(), ForLoc,
> SecondExpr.get(),
> +                                     Sema::ConditionKind::Boolean);
>        }
> -      SecondPartIsInvalid = Second.isInvalid();
> -      SecondPart = Actions.MakeFullExpr(Second.get(), ForLoc);
>      }
>
>      if (Tok.isNot(tok::semi)) {
> -      if (!SecondPartIsInvalid || SecondVar)
> +      if (!SecondPart.isInvalid())
>          Diag(Tok, diag::err_expected_semi_for);
>        else
>          // Skip until semicolon or rparen, don't consume it.
> @@ -1781,8 +1769,8 @@ StmtResult Parser::ParseForStatement(Sou
>      return Actions.FinishCXXForRangeStmt(ForRangeStmt.get(), Body.get());
>
>    return Actions.ActOnForStmt(ForLoc, T.getOpenLocation(),
> FirstPart.get(),
> -                              SecondPart, SecondVar, ThirdPart,
> -                              T.getCloseLocation(), Body.get());
> +                              SecondPart, ThirdPart, T.getCloseLocation(),
> +                              Body.get());
>  }
>
>  /// ParseGotoStatement
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Jun 23 03:41:20 2016
> @@ -10092,10 +10092,10 @@ buildSingleCopyAssignRecursively(Sema &S
>                                      SizeType, VK_LValue, OK_Ordinary,
> Loc);
>
>    // Construct the loop that copies all elements of this array.
> -  return S.ActOnForStmt(Loc, Loc, InitStmt,
> -                        S.MakeFullExpr(Comparison),
> -                        nullptr, S.MakeFullDiscardedValueExpr(Increment),
> -                        Loc, Copy.get());
> +  return S.ActOnForStmt(
> +      Loc, Loc, InitStmt,
> +      S.ActOnCondition(nullptr, Loc, Comparison,
> Sema::ConditionKind::Boolean),
> +      S.MakeFullDiscardedValueExpr(Increment), Loc, Copy.get());
>  }
>
>  static StmtResult
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Jun 23 03:41:20 2016
> @@ -14341,7 +14341,7 @@ void Sema::DiagnoseEqualityWithExtraPare
>      }
>  }
>
> -ExprResult Sema::CheckBooleanCondition(Expr *E, SourceLocation Loc) {
> +ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E) {
>    DiagnoseAssignmentAsCondition(E);
>    if (ParenExpr *parenE = dyn_cast<ParenExpr>(E))
>      DiagnoseEqualityWithExtraParens(parenE);
> @@ -14371,12 +14371,26 @@ ExprResult Sema::CheckBooleanCondition(E
>    return E;
>  }
>
> -ExprResult Sema::ActOnBooleanCondition(Scope *S, SourceLocation Loc,
> -                                       Expr *SubExpr) {
> +Sema::ConditionResult Sema::ActOnCondition(Scope *S, SourceLocation Loc,
> +                                           Expr *SubExpr, ConditionKind
> CK) {
> +  // Empty conditions are valid in for-statements.
>    if (!SubExpr)
> -    return ExprError();
> +    return ConditionResult();
>
> -  return CheckBooleanCondition(SubExpr, Loc);
> +  ExprResult Cond;
> +  switch (CK) {
> +  case ConditionKind::Boolean:
> +    Cond = CheckBooleanCondition(Loc, SubExpr);
> +    break;
> +
> +  case ConditionKind::Switch:
> +    Cond = CheckSwitchCondition(Loc, SubExpr);
> +    break;
> +  }
> +  if (Cond.isInvalid())
> +    return ConditionError();
> +
> +  return ConditionResult(nullptr, MakeFullExpr(Cond.get(), Loc));
>  }
>
>  namespace {
>
> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Thu Jun 23 03:41:20 2016
> @@ -3054,11 +3054,21 @@ void Sema::CheckVirtualDtorCall(CXXDestr
>    }
>  }
>
> +Sema::ConditionResult Sema::ActOnConditionVariable(Decl *ConditionVar,
> +                                                   SourceLocation StmtLoc,
> +                                                   ConditionKind CK) {
> +  ExprResult E =
> +      CheckConditionVariable(cast<VarDecl>(ConditionVar), StmtLoc, CK);
> +  if (E.isInvalid())
> +    return ConditionError();
> +  return ConditionResult(ConditionVar, MakeFullExpr(E.get(), StmtLoc));
> +}
> +
>  /// \brief Check the use of the given variable as a C++ condition in an
> if,
>  /// while, do-while, or switch statement.
>  ExprResult Sema::CheckConditionVariable(VarDecl *ConditionVar,
>                                          SourceLocation StmtLoc,
> -                                        bool ConvertToBoolean) {
> +                                        ConditionKind CK) {
>    if (ConditionVar->isInvalidDecl())
>      return ExprError();
>
> @@ -3082,13 +3092,15 @@ ExprResult Sema::CheckConditionVariable(
>
>    MarkDeclRefReferenced(cast<DeclRefExpr>(Condition.get()));
>
> -  if (ConvertToBoolean) {
> -    Condition = CheckBooleanCondition(Condition.get(), StmtLoc);
> -    if (Condition.isInvalid())
> -      return ExprError();
> +  switch (CK) {
> +  case ConditionKind::Boolean:
> +    return CheckBooleanCondition(StmtLoc, Condition.get());
> +
> +  case ConditionKind::Switch:
> +    return CheckSwitchCondition(StmtLoc, Condition.get());
>    }
>
> -  return Condition;
> +  llvm_unreachable("unexpected condition kind");
>  }
>
>  /// CheckCXXBooleanCondition - Returns true if a conversion to bool is
> invalid.
>
> Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Thu Jun 23 03:41:20 2016
> @@ -6826,12 +6826,11 @@ OMPClause *Sema::ActOnOpenMPIfClause(Ope
>    if (!Condition->isValueDependent() && !Condition->isTypeDependent() &&
>        !Condition->isInstantiationDependent() &&
>        !Condition->containsUnexpandedParameterPack()) {
> -    ExprResult Val = ActOnBooleanCondition(DSAStack->getCurScope(),
> -                                           Condition->getExprLoc(),
> Condition);
> +    ExprResult Val = CheckBooleanCondition(StartLoc, Condition);
>      if (Val.isInvalid())
>        return nullptr;
>
> -    ValExpr = Val.get();
> +    ValExpr = MakeFullExpr(Val.get()).get();
>    }
>
>    return new (Context) OMPIfClause(NameModifier, ValExpr, StartLoc,
> LParenLoc,
> @@ -6846,12 +6845,11 @@ OMPClause *Sema::ActOnOpenMPFinalClause(
>    if (!Condition->isValueDependent() && !Condition->isTypeDependent() &&
>        !Condition->isInstantiationDependent() &&
>        !Condition->containsUnexpandedParameterPack()) {
> -    ExprResult Val = ActOnBooleanCondition(DSAStack->getCurScope(),
> -                                           Condition->getExprLoc(),
> Condition);
> +    ExprResult Val = CheckBooleanCondition(StartLoc, Condition);
>      if (Val.isInvalid())
>        return nullptr;
>
> -    ValExpr = Val.get();
> +    ValExpr = MakeFullExpr(Val.get()).get();
>    }
>
>    return new (Context) OMPFinalClause(ValExpr, StartLoc, LParenLoc,
> EndLoc);
>
> Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Jun 23 03:41:20 2016
> @@ -504,39 +504,30 @@ public:
>  }
>
>  StmtResult
> -Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl
> *CondVar,
> +Sema::ActOnIfStmt(SourceLocation IfLoc, ConditionResult Cond,
>                    Stmt *thenStmt, SourceLocation ElseLoc,
>                    Stmt *elseStmt) {
> -  ExprResult CondResult(CondVal.release());
> -
> -  VarDecl *ConditionVar = nullptr;
> -  if (CondVar) {
> -    ConditionVar = cast<VarDecl>(CondVar);
> -    CondResult = CheckConditionVariable(ConditionVar, IfLoc, true);
> -    CondResult = ActOnFinishFullExpr(CondResult.get(), IfLoc);
> +  auto CondVal = Cond.get();
> +  if (Cond.isInvalid()) {
> +    CondVal.first = nullptr;
> +    CondVal.second = new (Context)
> +        OpaqueValueExpr(SourceLocation(), Context.BoolTy, VK_RValue);
>    }
> -  Expr *ConditionExpr = CondResult.getAs<Expr>();
> -  if (ConditionExpr) {
> -
> -    if (!Diags.isIgnored(diag::warn_comma_operator,
> -                         ConditionExpr->getExprLoc()))
> -      CommaVisitor(*this).Visit(ConditionExpr);
>
> -    DiagnoseUnusedExprResult(thenStmt);
> +  if (!Diags.isIgnored(diag::warn_comma_operator,
> +                       CondVal.second->getExprLoc()))
> +    CommaVisitor(*this).Visit(CondVal.second);
>
> -    if (!elseStmt) {
> -      DiagnoseEmptyStmtBody(ConditionExpr->getLocEnd(), thenStmt,
> -                            diag::warn_empty_if_body);
> -    }
> +  DiagnoseUnusedExprResult(thenStmt);
>
> -    DiagnoseUnusedExprResult(elseStmt);
> -  } else {
> -    // Create a dummy Expr for the condition for error recovery
> -    ConditionExpr = new (Context) OpaqueValueExpr(SourceLocation(),
> -                                                  Context.BoolTy,
> VK_RValue);
> +  if (!elseStmt) {
> +    DiagnoseEmptyStmtBody(CondVal.second->getLocEnd(), thenStmt,
> +                          diag::warn_empty_if_body);
>    }
>
> -  return new (Context) IfStmt(Context, IfLoc, ConditionVar, ConditionExpr,
> +  DiagnoseUnusedExprResult(elseStmt);
> +
> +  return new (Context) IfStmt(Context, IfLoc, CondVal.first,
> CondVal.second,
>                                thenStmt, ElseLoc, elseStmt);
>  }
>
> @@ -599,24 +590,7 @@ static QualType GetTypeBeforeIntegralPro
>    return expr->getType();
>  }
>
> -StmtResult
> -Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, Expr *Cond,
> -                             Decl *CondVar) {
> -  ExprResult CondResult;
> -
> -  VarDecl *ConditionVar = nullptr;
> -  if (CondVar) {
> -    ConditionVar = cast<VarDecl>(CondVar);
> -    CondResult = CheckConditionVariable(ConditionVar, SourceLocation(),
> false);
> -    if (CondResult.isInvalid())
> -      return StmtError();
> -
> -    Cond = CondResult.get();
> -  }
> -
> -  if (!Cond)
> -    return StmtError();
> -
> +ExprResult Sema::CheckSwitchCondition(SourceLocation SwitchLoc, Expr
> *Cond) {
>    class SwitchConvertDiagnoser : public ICEConvertDiagnoser {
>      Expr *Cond;
>
> @@ -664,24 +638,24 @@ Sema::ActOnStartOfSwitchStmt(SourceLocat
>      }
>    } SwitchDiagnoser(Cond);
>
> -  CondResult =
> +  ExprResult CondResult =
>        PerformContextualImplicitConversion(SwitchLoc, Cond,
> SwitchDiagnoser);
> -  if (CondResult.isInvalid()) return StmtError();
> -  Cond = CondResult.get();
> +  if (CondResult.isInvalid())
> +    return ExprError();
>
>    // C99 6.8.4.2p5 - Integer promotions are performed on the controlling
> expr.
> -  CondResult = UsualUnaryConversions(Cond);
> -  if (CondResult.isInvalid()) return StmtError();
> -  Cond = CondResult.get();
> +  return UsualUnaryConversions(CondResult.get());
> +}
>
> -  CondResult = ActOnFinishFullExpr(Cond, SwitchLoc);
> -  if (CondResult.isInvalid())
> +StmtResult
> +Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, ConditionResult
> Cond) {
> +  if (Cond.isInvalid())
>      return StmtError();
> -  Cond = CondResult.get();
>
>    getCurFunction()->setHasBranchIntoScope();
>
> -  SwitchStmt *SS = new (Context) SwitchStmt(Context, ConditionVar, Cond);
> +  SwitchStmt *SS =
> +      new (Context) SwitchStmt(Context, Cond.get().first,
> Cond.get().second);
>    getCurFunction()->SwitchStack.push_back(SS);
>    return SS;
>  }
> @@ -1242,27 +1216,17 @@ Sema::DiagnoseAssignmentEnum(QualType Ds
>      }
>  }
>
> -StmtResult
> -Sema::ActOnWhileStmt(SourceLocation WhileLoc, FullExprArg Cond,
> -                     Decl *CondVar, Stmt *Body) {
> -  ExprResult CondResult(Cond.release());
> -
> -  VarDecl *ConditionVar = nullptr;
> -  if (CondVar) {
> -    ConditionVar = cast<VarDecl>(CondVar);
> -    CondResult = CheckConditionVariable(ConditionVar, WhileLoc, true);
> -    CondResult = ActOnFinishFullExpr(CondResult.get(), WhileLoc);
> -    if (CondResult.isInvalid())
> -      return StmtError();
> -  }
> -  Expr *ConditionExpr = CondResult.get();
> -  if (!ConditionExpr)
> +StmtResult Sema::ActOnWhileStmt(SourceLocation WhileLoc, ConditionResult
> Cond,
> +                                Stmt *Body) {
> +  if (Cond.isInvalid())
>      return StmtError();
> -  CheckBreakContinueBinding(ConditionExpr);
>
> -  if (ConditionExpr &&
> -      !Diags.isIgnored(diag::warn_comma_operator,
> ConditionExpr->getExprLoc()))
> -    CommaVisitor(*this).Visit(ConditionExpr);
> +  auto CondVal = Cond.get();
> +  CheckBreakContinueBinding(CondVal.second);
> +
> +  if (CondVal.second &&
> +      !Diags.isIgnored(diag::warn_comma_operator,
> CondVal.second->getExprLoc()))
> +    CommaVisitor(*this).Visit(CondVal.second);
>
>    DiagnoseUnusedExprResult(Body);
>
> @@ -1270,7 +1234,7 @@ Sema::ActOnWhileStmt(SourceLocation Whil
>      getCurCompoundScope().setHasEmptyLoopBodies();
>
>    return new (Context)
> -      WhileStmt(Context, ConditionVar, ConditionExpr, Body, WhileLoc);
> +      WhileStmt(Context, CondVal.first, CondVal.second, Body, WhileLoc);
>  }
>
>  StmtResult
> @@ -1280,7 +1244,7 @@ Sema::ActOnDoStmt(SourceLocation DoLoc,
>    assert(Cond && "ActOnDoStmt(): missing expression");
>
>    CheckBreakContinueBinding(Cond);
> -  ExprResult CondResult = CheckBooleanCondition(Cond, DoLoc);
> +  ExprResult CondResult = CheckBooleanCondition(DoLoc, Cond);
>    if (CondResult.isInvalid())
>      return StmtError();
>    Cond = CondResult.get();
> @@ -1644,11 +1608,13 @@ void Sema::CheckBreakContinueBinding(Exp
>    }
>  }
>
> -StmtResult
> -Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
> -                   Stmt *First, FullExprArg second, Decl *secondVar,
> -                   FullExprArg third,
> -                   SourceLocation RParenLoc, Stmt *Body) {
> +StmtResult Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation
> LParenLoc,
> +                              Stmt *First, ConditionResult Second,
> +                              FullExprArg third, SourceLocation RParenLoc,
> +                              Stmt *Body) {
> +  if (Second.isInvalid())
> +    return StmtError();
> +
>    if (!getLangOpts().CPlusPlus) {
>      if (DeclStmt *DS = dyn_cast_or_null<DeclStmt>(First)) {
>        // C99 6.8.5p3: The declaration part of a 'for' statement shall only
> @@ -1666,26 +1632,17 @@ Sema::ActOnForStmt(SourceLocation ForLoc
>      }
>    }
>
> -  CheckBreakContinueBinding(second.get());
> +  CheckBreakContinueBinding(Second.get().second);
>    CheckBreakContinueBinding(third.get());
>
> -  CheckForLoopConditionalStatement(*this, second.get(), third.get(),
> Body);
> +  CheckForLoopConditionalStatement(*this, Second.get().second,
> third.get(),
> +                                   Body);
>    CheckForRedundantIteration(*this, third.get(), Body);
>
> -  ExprResult SecondResult(second.release());
> -  VarDecl *ConditionVar = nullptr;
> -  if (secondVar) {
> -    ConditionVar = cast<VarDecl>(secondVar);
> -    SecondResult = CheckConditionVariable(ConditionVar, ForLoc, true);
> -    SecondResult = ActOnFinishFullExpr(SecondResult.get(), ForLoc);
> -    if (SecondResult.isInvalid())
> -      return StmtError();
> -  }
> -
> -  if (SecondResult.get() &&
> +  if (Second.get().second &&
>        !Diags.isIgnored(diag::warn_comma_operator,
> -                       SecondResult.get()->getExprLoc()))
> -    CommaVisitor(*this).Visit(SecondResult.get());
> +                       Second.get().second->getExprLoc()))
> +    CommaVisitor(*this).Visit(Second.get().second);
>
>    Expr *Third  = third.release().getAs<Expr>();
>
> @@ -1696,8 +1653,9 @@ Sema::ActOnForStmt(SourceLocation ForLoc
>    if (isa<NullStmt>(Body))
>      getCurCompoundScope().setHasEmptyLoopBodies();
>
> -  return new (Context) ForStmt(Context, First, SecondResult.get(),
> ConditionVar,
> -                               Third, Body, ForLoc, LParenLoc, RParenLoc);
> +  return new (Context)
> +      ForStmt(Context, First, Second.get().second, Second.get().first,
> Third,
> +              Body, ForLoc, LParenLoc, RParenLoc);
>  }
>
>  /// In an Objective C collection iteration statement:
> @@ -2384,8 +2342,10 @@ Sema::BuildCXXForRangeStmt(SourceLocatio
>      // Build and check __begin != __end expression.
>      NotEqExpr = ActOnBinOp(S, ColonLoc, tok::exclaimequal,
>                             BeginRef.get(), EndRef.get());
> -    NotEqExpr = ActOnBooleanCondition(S, ColonLoc, NotEqExpr.get());
> -    NotEqExpr = ActOnFinishFullExpr(NotEqExpr.get());
> +    if (!NotEqExpr.isInvalid())
> +      NotEqExpr = CheckBooleanCondition(ColonLoc, NotEqExpr.get());
> +    if (!NotEqExpr.isInvalid())
> +      NotEqExpr = ActOnFinishFullExpr(NotEqExpr.get());
>      if (NotEqExpr.isInvalid()) {
>        Diag(RangeLoc, diag::note_for_range_invalid_iterator)
>          << RangeLoc << 0 << BeginRangeRef.get()->getType();
>
> Modified: cfe/trunk/lib/Sema/TreeTransform.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/TreeTransform.h (original)
> +++ cfe/trunk/lib/Sema/TreeTransform.h Thu Jun 23 03:41:20 2016
> @@ -410,6 +410,14 @@ public:
>      return D;
>    }
>
> +  /// \brief Transform the specified condition.
> +  ///
> +  /// By default, this transforms the variable and expression and rebuilds
> +  /// the condition.
> +  Sema::ConditionResult TransformCondition(SourceLocation Loc, VarDecl
> *Var,
> +                                           Expr *Expr,
> +                                           Sema::ConditionKind Kind);
> +
>    /// \brief Transform the attributes associated with the given
> declaration and
>    /// place them on the new declaration.
>    ///
> @@ -1166,10 +1174,9 @@ public:
>    ///
>    /// By default, performs semantic analysis to build the new statement.
>    /// Subclasses may override this routine to provide different behavior.
> -  StmtResult RebuildIfStmt(SourceLocation IfLoc, Sema::FullExprArg Cond,
> -                           VarDecl *CondVar, Stmt *Then,
> -                           SourceLocation ElseLoc, Stmt *Else) {
> -    return getSema().ActOnIfStmt(IfLoc, Cond, CondVar, Then, ElseLoc,
> Else);
> +  StmtResult RebuildIfStmt(SourceLocation IfLoc, Sema::ConditionResult
> Cond,
> +                           Stmt *Then, SourceLocation ElseLoc, Stmt
> *Else) {
> +    return getSema().ActOnIfStmt(IfLoc, Cond, Then, ElseLoc, Else);
>    }
>
>    /// \brief Start building a new switch statement.
> @@ -1177,9 +1184,8 @@ public:
>    /// By default, performs semantic analysis to build the new statement.
>    /// Subclasses may override this routine to provide different behavior.
>    StmtResult RebuildSwitchStmtStart(SourceLocation SwitchLoc,
> -                                    Expr *Cond, VarDecl *CondVar) {
> -    return getSema().ActOnStartOfSwitchStmt(SwitchLoc, Cond,
> -                                            CondVar);
> +                                    Sema::ConditionResult Cond) {
> +    return getSema().ActOnStartOfSwitchStmt(SwitchLoc, Cond);
>    }
>
>    /// \brief Attach the body to the switch statement.
> @@ -1195,9 +1201,9 @@ public:
>    ///
>    /// By default, performs semantic analysis to build the new statement.
>    /// Subclasses may override this routine to provide different behavior.
> -  StmtResult RebuildWhileStmt(SourceLocation WhileLoc, Sema::FullExprArg
> Cond,
> -                              VarDecl *CondVar, Stmt *Body) {
> -    return getSema().ActOnWhileStmt(WhileLoc, Cond, CondVar, Body);
> +  StmtResult RebuildWhileStmt(SourceLocation WhileLoc,
> +                              Sema::ConditionResult Cond, Stmt *Body) {
> +    return getSema().ActOnWhileStmt(WhileLoc, Cond, Body);
>    }
>
>    /// \brief Build a new do-while statement.
> @@ -1216,11 +1222,11 @@ public:
>    /// By default, performs semantic analysis to build the new statement.
>    /// Subclasses may override this routine to provide different behavior.
>    StmtResult RebuildForStmt(SourceLocation ForLoc, SourceLocation
> LParenLoc,
> -                            Stmt *Init, Sema::FullExprArg Cond,
> -                            VarDecl *CondVar, Sema::FullExprArg Inc,
> -                            SourceLocation RParenLoc, Stmt *Body) {
> +                            Stmt *Init, Sema::ConditionResult Cond,
> +                            Sema::FullExprArg Inc, SourceLocation
> RParenLoc,
> +                            Stmt *Body) {
>      return getSema().ActOnForStmt(ForLoc, LParenLoc, Init, Cond,
> -                                  CondVar, Inc, RParenLoc, Body);
> +                                  Inc, RParenLoc, Body);
>    }
>
>    /// \brief Build a new goto statement.
> @@ -3357,6 +3363,31 @@ bool TreeTransform<Derived>::TransformEx
>    return false;
>  }
>
> +template <typename Derived>
> +Sema::ConditionResult TreeTransform<Derived>::TransformCondition(
> +    SourceLocation Loc, VarDecl *Var, Expr *Expr, Sema::ConditionKind
> Kind) {
> +  if (Var) {
> +    VarDecl *ConditionVar = cast_or_null<VarDecl>(
> +        getDerived().TransformDefinition(Var->getLocation(), Var));
> +
> +    if (!ConditionVar)
> +      return Sema::ConditionError();
> +
> +    return getSema().ActOnConditionVariable(ConditionVar, Loc, Kind);
> +  }
> +
> +  if (Expr) {
> +    ExprResult CondExpr = getDerived().TransformExpr(Expr);
> +
> +    if (CondExpr.isInvalid())
> +      return Sema::ConditionError();
> +
> +    return getSema().ActOnCondition(nullptr, Loc, CondExpr.get(), Kind);
> +  }
> +
> +  return Sema::ConditionResult();
> +}
> +
>  template<typename Derived>
>  NestedNameSpecifierLoc
>  TreeTransform<Derived>::TransformNestedNameSpecifierLoc(
> @@ -4962,8 +4993,8 @@ bool TreeTransform<Derived>::TransformEx
>      if (NoexceptExpr.isInvalid())
>        return true;
>
> -    NoexceptExpr = getSema().CheckBooleanCondition(
> -        NoexceptExpr.get(), NoexceptExpr.get()->getLocStart());
> +    // FIXME: This is bogus, a noexcept expression is not a condition.
> +    NoexceptExpr = getSema().CheckBooleanCondition(Loc,
> NoexceptExpr.get());
>      if (NoexceptExpr.isInvalid())
>        return true;
>
> @@ -6195,35 +6226,10 @@ template<typename Derived>
>  StmtResult
>  TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
>    // Transform the condition
> -  ExprResult Cond;
> -  VarDecl *ConditionVar = nullptr;
> -  if (S->getConditionVariable()) {
> -    ConditionVar
> -      = cast_or_null<VarDecl>(
> -                   getDerived().TransformDefinition(
> -
> S->getConditionVariable()->getLocation(),
> -
> S->getConditionVariable()));
> -    if (!ConditionVar)
> -      return StmtError();
> -  } else {
> -    Cond = getDerived().TransformExpr(S->getCond());
> -
> -    if (Cond.isInvalid())
> -      return StmtError();
> -
> -    // Convert the condition to a boolean value.
> -    if (S->getCond()) {
> -      ExprResult CondE = getSema().ActOnBooleanCondition(nullptr,
> S->getIfLoc(),
> -                                                         Cond.get());
> -      if (CondE.isInvalid())
> -        return StmtError();
> -
> -      Cond = CondE.get();
> -    }
> -  }
> -
> -  Sema::FullExprArg FullCond(getSema().MakeFullExpr(Cond.get(),
> S->getIfLoc()));
> -  if (!S->getConditionVariable() && S->getCond() && !FullCond.get())
> +  Sema::ConditionResult Cond = getDerived().TransformCondition(
> +      S->getIfLoc(), S->getConditionVariable(), S->getCond(),
> +      Sema::ConditionKind::Boolean);
> +  if (Cond.isInvalid())
>      return StmtError();
>
>    // Transform the "then" branch.
> @@ -6237,14 +6243,12 @@ TreeTransform<Derived>::TransformIfStmt(
>      return StmtError();
>
>    if (!getDerived().AlwaysRebuild() &&
> -      FullCond.get() == S->getCond() &&
> -      ConditionVar == S->getConditionVariable() &&
> +      Cond.get() == std::make_pair(S->getConditionVariable(),
> S->getCond()) &&
>        Then.get() == S->getThen() &&
>        Else.get() == S->getElse())
>      return S;
>
> -  return getDerived().RebuildIfStmt(S->getIfLoc(), FullCond, ConditionVar,
> -                                    Then.get(),
> +  return getDerived().RebuildIfStmt(S->getIfLoc(), Cond, Then.get(),
>                                      S->getElseLoc(), Else.get());
>  }
>
> @@ -6252,27 +6256,15 @@ template<typename Derived>
>  StmtResult
>  TreeTransform<Derived>::TransformSwitchStmt(SwitchStmt *S) {
>    // Transform the condition.
> -  ExprResult Cond;
> -  VarDecl *ConditionVar = nullptr;
> -  if (S->getConditionVariable()) {
> -    ConditionVar
> -      = cast_or_null<VarDecl>(
> -                   getDerived().TransformDefinition(
> -
> S->getConditionVariable()->getLocation(),
> -
> S->getConditionVariable()));
> -    if (!ConditionVar)
> -      return StmtError();
> -  } else {
> -    Cond = getDerived().TransformExpr(S->getCond());
> -
> -    if (Cond.isInvalid())
> -      return StmtError();
> -  }
> +  Sema::ConditionResult Cond = getDerived().TransformCondition(
> +      S->getSwitchLoc(), S->getConditionVariable(), S->getCond(),
> +      Sema::ConditionKind::Switch);
> +  if (Cond.isInvalid())
> +    return StmtError();
>
>    // Rebuild the switch statement.
>    StmtResult Switch
> -    = getDerived().RebuildSwitchStmtStart(S->getSwitchLoc(), Cond.get(),
> -                                          ConditionVar);
> +    = getDerived().RebuildSwitchStmtStart(S->getSwitchLoc(), Cond);
>    if (Switch.isInvalid())
>      return StmtError();
>
> @@ -6290,36 +6282,10 @@ template<typename Derived>
>  StmtResult
>  TreeTransform<Derived>::TransformWhileStmt(WhileStmt *S) {
>    // Transform the condition
> -  ExprResult Cond;
> -  VarDecl *ConditionVar = nullptr;
> -  if (S->getConditionVariable()) {
> -    ConditionVar
> -      = cast_or_null<VarDecl>(
> -                   getDerived().TransformDefinition(
> -
> S->getConditionVariable()->getLocation(),
> -
> S->getConditionVariable()));
> -    if (!ConditionVar)
> -      return StmtError();
> -  } else {
> -    Cond = getDerived().TransformExpr(S->getCond());
> -
> -    if (Cond.isInvalid())
> -      return StmtError();
> -
> -    if (S->getCond()) {
> -      // Convert the condition to a boolean value.
> -      ExprResult CondE = getSema().ActOnBooleanCondition(nullptr,
> -                                                         S->getWhileLoc(),
> -                                                         Cond.get());
> -      if (CondE.isInvalid())
> -        return StmtError();
> -      Cond = CondE;
> -    }
> -  }
> -
> -  Sema::FullExprArg FullCond(
> -      getSema().MakeFullExpr(Cond.get(), S->getWhileLoc()));
> -  if (!S->getConditionVariable() && S->getCond() && !FullCond.get())
> +  Sema::ConditionResult Cond = getDerived().TransformCondition(
> +      S->getWhileLoc(), S->getConditionVariable(), S->getCond(),
> +      Sema::ConditionKind::Boolean);
> +  if (Cond.isInvalid())
>      return StmtError();
>
>    // Transform the body
> @@ -6328,13 +6294,11 @@ TreeTransform<Derived>::TransformWhileSt
>      return StmtError();
>
>    if (!getDerived().AlwaysRebuild() &&
> -      FullCond.get() == S->getCond() &&
> -      ConditionVar == S->getConditionVariable() &&
> +      Cond.get() == std::make_pair(S->getConditionVariable(),
> S->getCond()) &&
>        Body.get() == S->getBody())
>      return Owned(S);
>
> -  return getDerived().RebuildWhileStmt(S->getWhileLoc(), FullCond,
> -                                       ConditionVar, Body.get());
> +  return getDerived().RebuildWhileStmt(S->getWhileLoc(), Cond,
> Body.get());
>  }
>
>  template<typename Derived>
> @@ -6374,37 +6338,10 @@ TreeTransform<Derived>::TransformForStmt
>      getSema().ActOnOpenMPLoopInitialization(S->getForLoc(), Init.get());
>
>    // Transform the condition
> -  ExprResult Cond;
> -  VarDecl *ConditionVar = nullptr;
> -  if (S->getConditionVariable()) {
> -    ConditionVar
> -      = cast_or_null<VarDecl>(
> -                   getDerived().TransformDefinition(
> -
> S->getConditionVariable()->getLocation(),
> -
> S->getConditionVariable()));
> -    if (!ConditionVar)
> -      return StmtError();
> -  } else {
> -    Cond = getDerived().TransformExpr(S->getCond());
> -
> -    if (Cond.isInvalid())
> -      return StmtError();
> -
> -    if (S->getCond()) {
> -      // Convert the condition to a boolean value.
> -      ExprResult CondE = getSema().ActOnBooleanCondition(nullptr,
> -                                                         S->getForLoc(),
> -                                                         Cond.get());
> -      if (CondE.isInvalid())
> -        return StmtError();
> -
> -      Cond = CondE.get();
> -    }
> -  }
> -
> -  Sema::FullExprArg FullCond(
> -      getSema().MakeFullExpr(Cond.get(), S->getForLoc()));
> -  if (!S->getConditionVariable() && S->getCond() && !FullCond.get())
> +  Sema::ConditionResult Cond = getDerived().TransformCondition(
> +      S->getForLoc(), S->getConditionVariable(), S->getCond(),
> +      Sema::ConditionKind::Boolean);
> +  if (Cond.isInvalid())
>      return StmtError();
>
>    // Transform the increment
> @@ -6423,14 +6360,14 @@ TreeTransform<Derived>::TransformForStmt
>
>    if (!getDerived().AlwaysRebuild() &&
>        Init.get() == S->getInit() &&
> -      FullCond.get() == S->getCond() &&
> +      Cond.get() == std::make_pair(S->getConditionVariable(),
> S->getCond()) &&
>        Inc.get() == S->getInc() &&
>        Body.get() == S->getBody())
>      return S;
>
>    return getDerived().RebuildForStmt(S->getForLoc(), S->getLParenLoc(),
> -                                     Init.get(), FullCond, ConditionVar,
> -                                     FullInc, S->getRParenLoc(),
> Body.get());
> +                                     Init.get(), Cond, FullInc,
> +                                     S->getRParenLoc(), Body.get());
>  }
>
>  template<typename Derived>
> @@ -6924,7 +6861,7 @@ TreeTransform<Derived>::TransformCXXForR
>    if (Cond.isInvalid())
>      return StmtError();
>    if (Cond.get())
> -    Cond = SemaRef.CheckBooleanCondition(Cond.get(), S->getColonLoc());
> +    Cond = SemaRef.CheckBooleanCondition(S->getColonLoc(), Cond.get());
>    if (Cond.isInvalid())
>      return StmtError();
>    if (Cond.get())
>
> Modified: cfe/trunk/test/FixIt/fixit-vexing-parse.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit-vexing-parse.cpp?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/FixIt/fixit-vexing-parse.cpp (original)
> +++ cfe/trunk/test/FixIt/fixit-vexing-parse.cpp Thu Jun 23 03:41:20 2016
> @@ -60,7 +60,7 @@ namespace N {
>      VO m(int (*p)[4]);
>
>      // Don't emit warning and fixit because direct initializer is not
> permitted here.
> -    if (int n(int())){} // expected-error {{function type is not allowed
> here}} expected-error {{condition must have an initializer}}
> +    if (int n(int())){} // expected-error {{function type is not allowed
> here}}
>
>      // CHECK: fix-it:"{{.*}}":{66:8-66:10}:" = {}"
>      U u(); // expected-warning {{function declaration}} expected-note
> {{replace parentheses with an initializer}}
>
> Modified: cfe/trunk/test/Parser/cxx0x-condition.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx0x-condition.cpp?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Parser/cxx0x-condition.cpp (original)
> +++ cfe/trunk/test/Parser/cxx0x-condition.cpp Thu Jun 23 03:41:20 2016
> @@ -23,9 +23,9 @@ void f() {
>
>    if (S b(a)) {} // expected-error {{variable declaration in condition
> cannot have a parenthesized initializer}}
>
> -  if (S b(n)) {} // expected-error {{a function type is not allowed
> here}} expected-error {{must have an initializer}}
> +  if (S b(n)) {} // expected-error {{a function type is not allowed here}}
>    if (S b(n) = 0) {} // expected-error {{a function type is not allowed
> here}}
> -  if (S b(n) == 0) {} // expected-error {{a function type is not allowed
> here}} expected-error {{did you mean '='?}}
> +  if (S b(n) == 0) {} // expected-error {{a function type is not allowed
> here}}
>
>    S s(a);
>    if (S{s}) {} // ok
>
> Modified: cfe/trunk/test/SemaCXX/crashes.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/crashes.cpp?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/crashes.cpp (original)
> +++ cfe/trunk/test/SemaCXX/crashes.cpp Thu Jun 23 03:41:20 2016
> @@ -105,8 +105,7 @@ namespace PR9026 {
>  namespace PR10270 {
>    template<typename T> class C;
>    template<typename T> void f() {
> -    if (C<T> == 1) // expected-error{{expected unqualified-id}} \
> -                   // expected-error{{invalid '==' at end of declaration}}
> +    if (C<T> == 1) // expected-error{{expected unqualified-id}}
>        return;
>    }
>  }
>
> Modified: cfe/trunk/test/SemaCXX/for-range-examples.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/for-range-examples.cpp?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/for-range-examples.cpp (original)
> +++ cfe/trunk/test/SemaCXX/for-range-examples.cpp Thu Jun 23 03:41:20 2016
> @@ -176,9 +176,9 @@ namespace test4 {
>
>      // Make sure these don't crash. Better diagnostics would be nice.
>      for (: {1, 2, 3}) {} // expected-error {{expected expression}}
> expected-error {{expected ';'}}
> -    for (1 : {1, 2, 3}) {} // expected-error {{must declare a variable}}
> expected-warning {{result unused}}
> +    for (1 : {1, 2, 3}) {} // expected-error {{must declare a variable}}
>      for (+x : {1, 2, 3}) {} // expected-error {{undeclared identifier}}
> expected-error {{expected ';'}}
> -    for (+y : {1, 2, 3}) {} // expected-error {{must declare a variable}}
> expected-warning {{result unused}}
> +    for (+y : {1, 2, 3}) {} // expected-error {{must declare a variable}}
>    }
>  }
>
>
> Modified: cfe/trunk/test/SemaObjCXX/foreach.mm
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/foreach.mm?rev=273548&r1=273547&r2=273548&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaObjCXX/foreach.mm (original)
> +++ cfe/trunk/test/SemaObjCXX/foreach.mm Thu Jun 23 03:41:20 2016
> @@ -6,10 +6,8 @@
>  void f(NSArray *a) {
>      id keys;
>      for (int i : a); // expected-error{{selector element type 'int' is
> not a valid object}}
> -    for ((id)2 : a);  // expected-error {{for range declaration must
> declare a variable}} \
> -                      // expected-warning {{expression result unused}}
> -    for (2 : a); // expected-error {{for range declaration must declare a
> variable}} \
> -                 // expected-warning {{expression result unused}}
> +    for ((id)2 : a);  // expected-error {{for range declaration must
> declare a variable}}
> +    for (2 : a); // expected-error {{for range declaration must declare a
> variable}}
>
>    for (id thisKey : keys);
>
> @@ -65,8 +63,7 @@ int main ()
>  @end
>  void test2(NSObject<NSFastEnumeration> *collection) {
>    Test2 *obj;
> -  for (obj.prop : collection) { // expected-error {{for range declaration
> must declare a variable}} \
> -                                // expected-warning {{property access
> result unused - getters should not be used for side effects}}
> +  for (obj.prop : collection) { // expected-error {{for range declaration
> must declare a variable}}
>    }
>  }
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160623/29e477c6/attachment-0001.html>


More information about the cfe-commits mailing list