VitaNuo marked an inline comment as done.
VitaNuo added inline comments.
================
Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:161
+TEST_F(FindHeadersTest, TargetIsArgumentExpandedFromMacroInHeader) {
+ llvm::Annotations MainFile(R"cpp(
----------------
hokein wrote:
> these 3 newly-added tests are similar, instead of duplicating them, we can
> use a table-gen style test, see an
> [example](https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp#L265)
Great idea, thanks!
================
Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:186
+ std::vector<Header> Headers;
+ walkUsed(
+ TopLevelDecls, {}, nullptr, AST->sourceManager(),
----------------
hokein wrote:
> The intention of the test is to test the function `findHeaders`, we're
> testing it in an implicit way (`findHeaders` is hidden inside the
> `walkUsed`), there is no code here explicitly calling this function.
>
> We can make it clearer (no need to use walkUsed):
> 1. we get a `Foo` Decl AST node from the testcase.
> 2. with the AST node from step1, we cam call the `findHeaders` directly in
> the test code.
>
> for 1, we can just write a simple RecursiveASTVisitor, and capture a
> NamedDecl which called Foo, something like:
>
> ```
> struct Visitor : RecursiveASTVisitor<Visitor> {
> const NamedDecl *Out = nullptr;
> bool VisitNamedDecl(const NamedDecl *ND) {
> if (ND->getName() == "Foo") {
> EXPECT_TRUE(Out == nullptr);
> Out = ND;
> }
> return true;
> }
> };
> ```
Ok, I've attempted to follow your advice now, please have a look.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139716/new/
https://reviews.llvm.org/D139716
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
