[clang] 17ea41e - Summary: [clang] Provide a way for WhileStmt to report the location of its LParen and RParen.

Vy Nguyen via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 10 18:31:35 PDT 2020


Author: Vy Nguyen
Date: 2020-07-10T21:31:16-04:00
New Revision: 17ea41e472566823e16d3a04661221fbd18d9fae

URL: https://github.com/llvm/llvm-project/commit/17ea41e472566823e16d3a04661221fbd18d9fae
DIFF: https://github.com/llvm/llvm-project/commit/17ea41e472566823e16d3a04661221fbd18d9fae.diff

LOG: Summary: [clang] Provide a way for WhileStmt to report the location of its LParen and RParen.

Summary: This helps avoiding hacks downstream.

Reviewers: shafik

Subscribers: martong, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D83529

Added: 
    

Modified: 
    clang/include/clang/AST/Stmt.h
    clang/include/clang/Parse/Parser.h
    clang/include/clang/Sema/Sema.h
    clang/lib/AST/ASTImporter.cpp
    clang/lib/AST/Stmt.cpp
    clang/lib/Parse/ParseStmt.cpp
    clang/lib/Sema/SemaStmt.cpp
    clang/lib/Sema/TreeTransform.h
    clang/lib/Serialization/ASTReaderStmt.cpp
    clang/lib/Serialization/ASTWriterStmt.cpp
    clang/unittests/AST/SourceLocationTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 01d4301922f8..d3fad58fcf59 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -2277,6 +2277,8 @@ class WhileStmt final : public Stmt,
   enum { VarOffset = 0, BodyOffsetFromCond = 1 };
   enum { NumMandatoryStmtPtr = 2 };
 
+  SourceLocation LParenLoc, RParenLoc;
+
   unsigned varOffset() const { return VarOffset; }
   unsigned condOffset() const { return VarOffset + hasVarStorage(); }
   unsigned bodyOffset() const { return condOffset() + BodyOffsetFromCond; }
@@ -2287,7 +2289,8 @@ class WhileStmt final : public Stmt,
 
   /// Build a while statement.
   WhileStmt(const ASTContext &Ctx, VarDecl *Var, Expr *Cond, Stmt *Body,
-            SourceLocation WL);
+            SourceLocation WL, SourceLocation LParenLoc,
+            SourceLocation RParenLoc);
 
   /// Build an empty while statement.
   explicit WhileStmt(EmptyShell Empty, bool HasVar);
@@ -2295,7 +2298,8 @@ class WhileStmt final : public Stmt,
 public:
   /// Create a while statement.
   static WhileStmt *Create(const ASTContext &Ctx, VarDecl *Var, Expr *Cond,
-                           Stmt *Body, SourceLocation WL);
+                           Stmt *Body, SourceLocation WL,
+                           SourceLocation LParenLoc, SourceLocation RParenLoc);
 
   /// Create an empty while statement optionally with storage for
   /// a condition variable.
@@ -2359,6 +2363,11 @@ class WhileStmt final : public Stmt,
   SourceLocation getWhileLoc() const { return WhileStmtBits.WhileLoc; }
   void setWhileLoc(SourceLocation L) { WhileStmtBits.WhileLoc = L; }
 
+  SourceLocation getLParenLoc() const { return LParenLoc; }
+  void setLParenLoc(SourceLocation L) { LParenLoc = L; }
+  SourceLocation getRParenLoc() const { return RParenLoc; }
+  void setRParenLoc(SourceLocation L) { RParenLoc = L; }
+
   SourceLocation getBeginLoc() const { return getWhileLoc(); }
   SourceLocation getEndLoc() const LLVM_READONLY {
     return getBody()->getEndLoc();

diff  --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 1d75515d494e..e809d87b59a0 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -2071,8 +2071,9 @@ class Parser : public CodeCompletionHandler {
   StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
   bool ParseParenExprOrCondition(StmtResult *InitStmt,
                                  Sema::ConditionResult &CondResult,
-                                 SourceLocation Loc,
-                                 Sema::ConditionKind CK);
+                                 SourceLocation Loc, Sema::ConditionKind CK,
+                                 SourceLocation *LParenLoc = nullptr,
+                                 SourceLocation *RParenLoc = nullptr);
   StmtResult ParseIfStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseSwitchStatement(SourceLocation *TrailingElseLoc);
   StmtResult ParseWhileStatement(SourceLocation *TrailingElseLoc);

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c3bebea0cccb..e75ac185eb2c 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4377,7 +4377,8 @@ class Sema final {
                                     ConditionResult Cond);
   StmtResult ActOnFinishSwitchStmt(SourceLocation SwitchLoc,
                                            Stmt *Switch, Stmt *Body);
-  StmtResult ActOnWhileStmt(SourceLocation WhileLoc, ConditionResult Cond,
+  StmtResult ActOnWhileStmt(SourceLocation WhileLoc, SourceLocation LParenLoc,
+                            ConditionResult Cond, SourceLocation RParenLoc,
                             Stmt *Body);
   StmtResult ActOnDoStmt(SourceLocation DoLoc, Stmt *Body,
                          SourceLocation WhileLoc, SourceLocation CondLParen,

diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index fa2421ee826e..3779e0cb872b 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6117,11 +6117,13 @@ ExpectedStmt ASTNodeImporter::VisitWhileStmt(WhileStmt *S) {
   auto ToCond = importChecked(Err, S->getCond());
   auto ToBody = importChecked(Err, S->getBody());
   auto ToWhileLoc = importChecked(Err, S->getWhileLoc());
+  auto ToLParenLoc = importChecked(Err, S->getLParenLoc());
+  auto ToRParenLoc = importChecked(Err, S->getRParenLoc());
   if (Err)
     return std::move(Err);
 
   return WhileStmt::Create(Importer.getToContext(), ToConditionVariable, ToCond,
-                           ToBody, ToWhileLoc);
+                           ToBody, ToWhileLoc, ToLParenLoc, ToRParenLoc);
 }
 
 ExpectedStmt ASTNodeImporter::VisitDoStmt(DoStmt *S) {

diff  --git a/clang/lib/AST/Stmt.cpp b/clang/lib/AST/Stmt.cpp
index ce76d4941b32..25e685be3e9b 100644
--- a/clang/lib/AST/Stmt.cpp
+++ b/clang/lib/AST/Stmt.cpp
@@ -1012,7 +1012,8 @@ void SwitchStmt::setConditionVariable(const ASTContext &Ctx, VarDecl *V) {
 }
 
 WhileStmt::WhileStmt(const ASTContext &Ctx, VarDecl *Var, Expr *Cond,
-                     Stmt *Body, SourceLocation WL)
+                     Stmt *Body, SourceLocation WL, SourceLocation LParenLoc,
+                     SourceLocation RParenLoc)
     : Stmt(WhileStmtClass) {
   bool HasVar = Var != nullptr;
   WhileStmtBits.HasVar = HasVar;
@@ -1023,6 +1024,8 @@ WhileStmt::WhileStmt(const ASTContext &Ctx, VarDecl *Var, Expr *Cond,
     setConditionVariable(Ctx, Var);
 
   setWhileLoc(WL);
+  setLParenLoc(LParenLoc);
+  setRParenLoc(RParenLoc);
 }
 
 WhileStmt::WhileStmt(EmptyShell Empty, bool HasVar)
@@ -1031,12 +1034,14 @@ WhileStmt::WhileStmt(EmptyShell Empty, bool HasVar)
 }
 
 WhileStmt *WhileStmt::Create(const ASTContext &Ctx, VarDecl *Var, Expr *Cond,
-                             Stmt *Body, SourceLocation WL) {
+                             Stmt *Body, SourceLocation WL,
+                             SourceLocation LParenLoc,
+                             SourceLocation RParenLoc) {
   bool HasVar = Var != nullptr;
   void *Mem =
       Ctx.Allocate(totalSizeToAlloc<Stmt *>(NumMandatoryStmtPtr + HasVar),
                    alignof(WhileStmt));
-  return new (Mem) WhileStmt(Ctx, Var, Cond, Body, WL);
+  return new (Mem) WhileStmt(Ctx, Var, Cond, Body, WL, LParenLoc, RParenLoc);
 }
 
 WhileStmt *WhileStmt::CreateEmpty(const ASTContext &Ctx, bool HasVar) {

diff  --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 3299a059c437..89a6a2b829ae 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1156,10 +1156,14 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
 /// 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.
 bool Parser::ParseParenExprOrCondition(StmtResult *InitStmt,
                                        Sema::ConditionResult &Cond,
                                        SourceLocation Loc,
-                                       Sema::ConditionKind CK) {
+                                       Sema::ConditionKind CK,
+                                       SourceLocation *LParenLoc,
+                                       SourceLocation *RParenLoc) {
   BalancedDelimiterTracker T(*this, tok::l_paren);
   T.consumeOpen();
 
@@ -1189,6 +1193,13 @@ bool Parser::ParseParenExprOrCondition(StmtResult *InitStmt,
   // Otherwise the condition is valid or the rparen is present.
   T.consumeClose();
 
+  if (LParenLoc != nullptr) {
+    *LParenLoc = T.getOpenLocation();
+  }
+  if (RParenLoc != nullptr) {
+    *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.
@@ -1582,8 +1593,10 @@ StmtResult Parser::ParseWhileStatement(SourceLocation *TrailingElseLoc) {
 
   // Parse the condition.
   Sema::ConditionResult Cond;
+  SourceLocation LParen;
+  SourceLocation RParen;
   if (ParseParenExprOrCondition(nullptr, Cond, WhileLoc,
-                                Sema::ConditionKind::Boolean))
+                                Sema::ConditionKind::Boolean, &LParen, &RParen))
     return StmtError();
 
   // C99 6.8.5p5 - In C99, the body of the while statement is a scope, even if
@@ -1613,7 +1626,7 @@ StmtResult Parser::ParseWhileStatement(SourceLocation *TrailingElseLoc) {
   if (Cond.isInvalid() || Body.isInvalid())
     return StmtError();
 
-  return Actions.ActOnWhileStmt(WhileLoc, Cond, Body.get());
+  return Actions.ActOnWhileStmt(WhileLoc, LParen, Cond, RParen, Body.get());
 }
 
 /// ParseDoStatement

diff  --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index a22a1116eb0b..73f3183c163f 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -1328,8 +1328,9 @@ Sema::DiagnoseAssignmentEnum(QualType DstType, QualType SrcType,
     }
 }
 
-StmtResult Sema::ActOnWhileStmt(SourceLocation WhileLoc, ConditionResult Cond,
-                                Stmt *Body) {
+StmtResult Sema::ActOnWhileStmt(SourceLocation WhileLoc,
+                                SourceLocation LParenLoc, ConditionResult Cond,
+                                SourceLocation RParenLoc, Stmt *Body) {
   if (Cond.isInvalid())
     return StmtError();
 
@@ -1344,7 +1345,7 @@ StmtResult Sema::ActOnWhileStmt(SourceLocation WhileLoc, ConditionResult Cond,
     getCurCompoundScope().setHasEmptyLoopBodies();
 
   return WhileStmt::Create(Context, CondVal.first, CondVal.second, Body,
-                           WhileLoc);
+                           WhileLoc, LParenLoc, RParenLoc);
 }
 
 StmtResult

diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 1b7c22f0901b..ae0e9f1119b4 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -1347,9 +1347,10 @@ class TreeTransform {
   ///
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide 
diff erent behavior.
-  StmtResult RebuildWhileStmt(SourceLocation WhileLoc,
-                              Sema::ConditionResult Cond, Stmt *Body) {
-    return getSema().ActOnWhileStmt(WhileLoc, Cond, Body);
+  StmtResult RebuildWhileStmt(SourceLocation WhileLoc, SourceLocation LParenLoc,
+                              Sema::ConditionResult Cond,
+                              SourceLocation RParenLoc, Stmt *Body) {
+    return getSema().ActOnWhileStmt(WhileLoc, LParenLoc, Cond, RParenLoc, Body);
   }
 
   /// Build a new do-while statement.
@@ -7335,7 +7336,8 @@ TreeTransform<Derived>::TransformWhileStmt(WhileStmt *S) {
       Body.get() == S->getBody())
     return Owned(S);
 
-  return getDerived().RebuildWhileStmt(S->getWhileLoc(), Cond, Body.get());
+  return getDerived().RebuildWhileStmt(S->getWhileLoc(), S->getLParenLoc(),
+                                       Cond, S->getRParenLoc(), Body.get());
 }
 
 template<typename Derived>

diff  --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index e3bac703f9f7..a40c5499a6d7 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -271,6 +271,8 @@ void ASTStmtReader::VisitWhileStmt(WhileStmt *S) {
     S->setConditionVariable(Record.getContext(), readDeclAs<VarDecl>());
 
   S->setWhileLoc(readSourceLocation());
+  S->setLParenLoc(readSourceLocation());
+  S->setRParenLoc(readSourceLocation());
 }
 
 void ASTStmtReader::VisitDoStmt(DoStmt *S) {

diff  --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index 941665ff0cba..0767b3a24bf2 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -194,6 +194,8 @@ void ASTStmtWriter::VisitWhileStmt(WhileStmt *S) {
     Record.AddDeclRef(S->getConditionVariable());
 
   Record.AddSourceLocation(S->getWhileLoc());
+  Record.AddSourceLocation(S->getLParenLoc());
+  Record.AddSourceLocation(S->getRParenLoc());
   Code = serialization::STMT_WHILE;
 }
 

diff  --git a/clang/unittests/AST/SourceLocationTest.cpp b/clang/unittests/AST/SourceLocationTest.cpp
index cb96afed64d3..32dc382aa05c 100644
--- a/clang/unittests/AST/SourceLocationTest.cpp
+++ b/clang/unittests/AST/SourceLocationTest.cpp
@@ -60,6 +60,59 @@ TEST(RangeVerifier, WrongRange) {
   EXPECT_FALSE(Verifier.match("int i;", varDecl()));
 }
 
+class WhileParenLocationVerifier : public MatchVerifier<WhileStmt> {
+  unsigned ExpectLParenLine = 0, ExpectLParenColumn = 0;
+  unsigned ExpectRParenLine = 0, ExpectRParenColumn = 0;
+
+public:
+  void expectLocations(unsigned LParenLine, unsigned LParenColumn,
+                       unsigned RParenLine, unsigned RParenColumn) {
+    ExpectLParenLine = LParenLine;
+    ExpectLParenColumn = LParenColumn;
+    ExpectRParenLine = RParenLine;
+    ExpectRParenColumn = RParenColumn;
+  }
+
+protected:
+  void verify(const MatchFinder::MatchResult &Result,
+              const WhileStmt &Node) override {
+    SourceLocation LParenLoc = Node.getLParenLoc();
+    SourceLocation RParenLoc = Node.getRParenLoc();
+    unsigned LParenLine =
+        Result.SourceManager->getSpellingLineNumber(LParenLoc);
+    unsigned LParenColumn =
+        Result.SourceManager->getSpellingColumnNumber(LParenLoc);
+    unsigned RParenLine =
+        Result.SourceManager->getSpellingLineNumber(RParenLoc);
+    unsigned RParenColumn =
+        Result.SourceManager->getSpellingColumnNumber(RParenLoc);
+
+    if (LParenLine != ExpectLParenLine || LParenColumn != ExpectLParenColumn ||
+        RParenLine != ExpectRParenLine || RParenColumn != ExpectRParenColumn) {
+      std::string MsgStr;
+      llvm::raw_string_ostream Msg(MsgStr);
+      Msg << "Expected LParen Location <" << ExpectLParenLine << ":"
+          << ExpectLParenColumn << ">, found <";
+      LParenLoc.print(Msg, *Result.SourceManager);
+      Msg << ">\n";
+
+      Msg << "Expected RParen Location <" << ExpectRParenLine << ":"
+          << ExpectRParenColumn << ">, found <";
+      RParenLoc.print(Msg, *Result.SourceManager);
+      Msg << ">";
+
+      this->setFailure(Msg.str());
+    }
+  }
+};
+
+TEST(LocationVerifier, WhileParenLoc) {
+  WhileParenLocationVerifier Verifier;
+  Verifier.expectLocations(1, 17, 1, 38);
+  EXPECT_TRUE(Verifier.match("void f() { while(true/*some comment*/) {} }",
+                             whileStmt()));
+}
+
 class LabelDeclRangeVerifier : public RangeVerifier<LabelStmt> {
 protected:
   SourceRange getRange(const LabelStmt &Node) override {


        


More information about the cfe-commits mailing list