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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
