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

Peter Collingbourne via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 23 11:20:07 PDT 2016


I reverted this change in r273589 and added a test for the pattern that
Nico mentioned in r273590.

Peter

On Thu, Jun 23, 2016 at 8:15 AM, Nico Weber via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> 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
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>


-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160623/245a7e9e/attachment-0001.html>


More information about the cfe-commits mailing list