[PATCH] D42623: [clang-tidy] Add a Lexer util to get the source text of a statement

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 29 05:12:25 PST 2018


aaron.ballman added inline comments.


================
Comment at: clang-tidy/utils/LexerUtils.cpp:39
+Optional<StringRef> getStmtText(const Stmt* Statement, const SourceManager& SM) {
+  if (!Statement) {
+    return llvm::NoneType();
----------------
You should elide the curly braces here.


================
Comment at: clang-tidy/utils/LexerUtils.cpp:44
+  bool Error = false;
+  auto Ret = Lexer::getSourceText(
+    CharSourceRange::getTokenRange(Statement->getSourceRange()),
----------------
Please don't use `auto` here, as the type is not spelled out in the initialization.


================
Comment at: clang-tidy/utils/LexerUtils.h:26
+/// Get source code text for statement.
+Optional<StringRef> getStmtText(const Stmt* Statement, const SourceManager& SM);
+
----------------
Formatting (elsewhere as well).

You should run the patch under clang-format to handle this sort of thing.


================
Comment at: unittests/clang-tidy/LexerUtilsTest.cpp:31
+TEST_F(GetStmtTextTest, SimpleStatement) {
+  auto FooIdent = &Unit->getASTContext().Idents.getOwn("foo");
+  auto FooDeclName = Unit->getASTContext().DeclarationNames.getIdentifier(FooIdent);
----------------
Don't use `auto` unless the type is spelled out in the init, such as with `dyn_cast`


================
Comment at: unittests/clang-tidy/LexerUtilsTest.cpp:34-35
+  auto FooLookup = Unit->getASTContext().getTranslationUnitDecl()->lookup(FooDeclName);
+  auto FooFunction = dyn_cast_or_null<FunctionDecl>(FooLookup.front());
+  EXPECT_TRUE(FooFunction);
+
----------------
Can `FooLookup.front()` actually be null? I think this should use `cast<>` and assert if given null or something other than a `FunctionDecl`.


================
Comment at: unittests/clang-tidy/LexerUtilsTest.cpp:37-38
+
+  auto FooBody = dyn_cast_or_null<CompoundStmt>(FooFunction->getBody());
+  EXPECT_TRUE(FooBody);
+  EXPECT_TRUE(FooBody->body_begin());
----------------
I think you can use `cast<>` here instead of `dyn_cast_or_null<>`, and it will assert for you if the body isn't valid.


================
Comment at: unittests/clang-tidy/LexerUtilsTest.cpp:43
+
+
+  auto Res = utils::lexer::getStmtText(DeclStmt, SM);
----------------
Spurious newline.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42623





More information about the cfe-commits mailing list