[clang][Parser][NFC] Simplify ParseParenExprOrCondition

[tbaeder]

Summary

All call-sites of this function pass false for MissingOK, so remove the parameter. Also, all call-sites pass both LParenLoc and RParenLoc, so replace the conditional store with an assertion for both of them.

[aaron.ballman]

LGTM with a minor improvement.

clang/lib/Parse/ParseStmt.cpp

1275–1281

We can do better than an assert.

[shafik]

clang/lib/Parse/ParseStmt.cpp

1286

Nitpick edit for adding bugprone-argument-comment annotation.

clang/include/clang/Parse/Parser.h

StmtResult ParseCompoundStatement(bool isStmtExpr,
                                     unsigned ScopeFlags);
   void ParseCompoundStatementLeadingPragmas();
   void DiagnoseLabelAtEndOfCompoundStatement();
   bool ConsumeNullStmt(StmtVector &Stmts);
   StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
   bool ParseParenExprOrCondition(StmtResult *InitStmt,
                                  Sema::ConditionResult &CondResult,
                                  SourceLocation Loc, Sema::ConditionKind CK,
-                                 bool MissingOK, SourceLocation *LParenLoc,
-                                 SourceLocation *RParenLoc);
+                                 SourceLocation &LParenLoc,
+                                 SourceLocation &RParenLoc);
   StmtResult ParseIfStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseSwitchStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseWhileStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseDoStatement();
   StmtResult ParseForStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseGotoStatement();
   StmtResult ParseContinueStatement();
   StmtResult ParseBreakStatement();

clang/lib/Parse/ParseStmt.cpp

 ///
 /// This function parses and performs error recovery on the specified condition
 /// or expression (depending on whether we're in C++ or C mode).  This function
 /// goes out of its way to recover well.  It returns true if there was a parser
 /// error (the right paren couldn't be found), which indicates that the caller
 /// 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.
-/// Additionally, if LParenLoc and RParenLoc are non-null, it will assign
-/// the location of the outer-most '(' and ')', respectively, to them.
+/// Additionally, it will assign the location of the outer-most '(' and ')',
+/// to LParenLoc and RParenLoc, respectively.
 bool Parser::ParseParenExprOrCondition(StmtResult *InitStmt,
                                        Sema::ConditionResult &Cond,
                                        SourceLocation Loc,
-                                       Sema::ConditionKind CK, bool MissingOK,
-                                       SourceLocation *LParenLoc,
-                                       SourceLocation *RParenLoc) {
+                                       Sema::ConditionKind CK,
+                                       SourceLocation &LParenLoc,
+                                       SourceLocation &RParenLoc) {
   BalancedDelimiterTracker T(*this, tok::l_paren);
   T.consumeOpen();
   SourceLocation Start = Tok.getLocation();
 
   if (getLangOpts().CPlusPlus) {
-    Cond = ParseCXXCondition(InitStmt, Loc, CK, MissingOK);
+    Cond = ParseCXXCondition(InitStmt, Loc, CK, false);
   } else {
     ExprResult CondExpr = ParseExpression();
 
     // If required, convert to a boolean value.
     if (CondExpr.isInvalid())
       Cond = Sema::ConditionError();
     else
       Cond = Actions.ActOnCondition(getCurScope(), Loc, CondExpr.get(), CK,
-                                    MissingOK);
+                                    /*MissingOK=*/false);
   }
 
   // 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 (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.
     if (Tok.isNot(tok::r_paren))
       return true;
   }
 
   if (Cond.isInvalid()) {
     ExprResult CondExpr = Actions.CreateRecoveryExpr(
         Start, Tok.getLocation() == Start ? Start : PrevTokLocation, {},
         Actions.PreferredConditionType(CK));
     if (!CondExpr.isInvalid())
       Cond = Actions.ActOnCondition(getCurScope(), Loc, CondExpr.get(), CK,
-                                    MissingOK);
+                                    /*MissingOK=*/false);
   }
 
   // Either the condition is valid or the rparen is present.
   T.consumeClose();
-
-  if (LParenLoc != nullptr) {
-    *LParenLoc = T.getOpenLocation();
-  }
-  if (RParenLoc != nullptr) {
-    *RParenLoc = T.getCloseLocation();
-  }
+  LParenLoc = T.getOpenLocation();
+  RParenLoc = T.getCloseLocation();
 
   // Check for extraneous ')'s to catch things like "if (foo())) {".  We know
   // that all callers are looking for a statement after the condition, so ")"
   // isn't valid.
   while (Tok.is(tok::r_paren)) {
     Diag(Tok, diag::err_extraneous_rparen_in_condition)
       << FixItHint::CreateRemoval(Tok.getLocation());
     ConsumeParen();
StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
   SourceLocation LParen;
   SourceLocation RParen;
   llvm::Optional<bool> ConstexprCondition;
   if (!IsConsteval) {
 
     if (ParseParenExprOrCondition(&InitStmt, Cond, IfLoc,
                                   IsConstexpr ? Sema::ConditionKind::ConstexprIf
                                               : Sema::ConditionKind::Boolean,
-                                  /*MissingOK=*/false, &LParen, &RParen))
+                                  LParen, RParen))
       return StmtError();
 
     if (IsConstexpr)
       ConstexprCondition = Cond.getKnownValue();
   }
 
   bool IsBracedThen = Tok.is(tok::l_brace);
 
StmtResult Parser::ParseSwitchStatement(SourceLocation *TrailingElseLoc) {
   ParseScope SwitchScope(this, ScopeFlags);
 
   // Parse the condition.
   StmtResult InitStmt;
   Sema::ConditionResult Cond;
   SourceLocation LParen;
   SourceLocation RParen;
   if (ParseParenExprOrCondition(&InitStmt, Cond, SwitchLoc,
-                                Sema::ConditionKind::Switch,
-                                /*MissingOK=*/false, &LParen, &RParen))
+                                Sema::ConditionKind::Switch, LParen, RParen))
     return StmtError();
 
   StmtResult Switch = Actions.ActOnStartOfSwitchStmt(
       SwitchLoc, LParen, InitStmt.get(), Cond, RParen);
 
   if (Switch.isInvalid()) {
     // Skip the switch body.
     // FIXME: This is not optimal recovery, but parsing the body is more
else
     ScopeFlags = Scope::BreakScope | Scope::ContinueScope;
   ParseScope WhileScope(this, ScopeFlags);
 
   // Parse the condition.
   Sema::ConditionResult Cond;
   SourceLocation LParen;
   SourceLocation RParen;
   if (ParseParenExprOrCondition(nullptr, Cond, WhileLoc,
-                                Sema::ConditionKind::Boolean,
-                                /*MissingOK=*/false, &LParen, &RParen))
+                                Sema::ConditionKind::Boolean, LParen, RParen))
     return StmtError();
 
   // 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.
   //
   // C++ 6.5p2:
   // The substatement in an iteration-statement implicitly defines a local scope