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

Dmitri Gribenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 10 09:50:39 PDT 2020


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

Could you take a look at test failures and check if they are relevant? `linux > Clang.AST::ast-dump-attr.cpp` looks extremely close to the area you're working on.

I'm not quite comfortable adding this code without unittests. We don't have a great way to unit test the AST, but I think we should try. I think `clang/unittests/AST/MatchVerifier.h` could help here. The test could match a while loop with AST matchers, and then assert that the source locations are what we expect. The while loop could come from the code directly, or be instantiated from a template; also test what happens for parse errors.



================
Comment at: clang/include/clang/AST/Stmt.h:2292
   WhileStmt(const ASTContext &Ctx, VarDecl *Var, Expr *Cond, Stmt *Body,
-            SourceLocation WL);
+            SourceLocation WL, SourceLocation LP, SourceLocation RP);
 
----------------
I'd prefer to unabbreviate here. (use `LParenLoc` as well)


================
Comment at: clang/include/clang/AST/Stmt.h:2300
   static WhileStmt *Create(const ASTContext &Ctx, VarDecl *Var, Expr *Cond,
-                           Stmt *Body, SourceLocation WL);
+                           Stmt *Body, SourceLocation WL, SourceLocation LP,
+                           SourceLocation RP);
----------------
I'd prefer to unabbreviate here. (use `LParenLoc` as well)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83529





More information about the cfe-commits mailing list