[PATCH] D85696: [AST] add parenthesis locations for IfStmt and SwitchStmt

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Only relatively trivial comments; please feel free to commit once they're addressed.



================
Comment at: clang/include/clang/Sema/Sema.h:4382-4393
+  StmtResult ActOnIfStmt(SourceLocation IfLoc, bool IsConstexpr, Stmt *InitStmt,
+                         ConditionResult Cond, SourceLocation LParenLoc,
+                         SourceLocation RParenLoc, Stmt *ThenVal,
                          SourceLocation ElseLoc, Stmt *ElseVal);
-  StmtResult BuildIfStmt(SourceLocation IfLoc, bool IsConstexpr,
-                         Stmt *InitStmt,
-                         ConditionResult Cond, Stmt *ThenVal,
+  StmtResult BuildIfStmt(SourceLocation IfLoc, bool IsConstexpr, Stmt *InitStmt,
+                         ConditionResult Cond, SourceLocation LParenLoc,
+                         SourceLocation RParenLoc, Stmt *ThenVal,
----------------
Generally we put the parameters to the `ActOn*` functions in syntactic order, so `LParenLoc` should precede `InitStmt` in each of the above functions.


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8028-8029
                                           Sema::ConditionKind::Boolean),
-                         ReturnFalse.get(), SourceLocation(), nullptr);
+                         SourceLocation(), SourceLocation(), ReturnFalse.get(),
+                         SourceLocation(), nullptr);
   }
----------------
This code is generally using `Loc` as the location of all of its components. Perhaps we should do the same for the location of the parentheses?


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8182
       return S.ActOnIfStmt(Loc, /*IsConstexpr=*/false, InitStmt, Cond,
+                           /*LPL=*/SourceLocation(), /*RPL=*/SourceLocation(),
                            ReturnStmt.get(), /*ElseLoc=*/SourceLocation(),
----------------
Likewise here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85696/new/

https://reviews.llvm.org/D85696