diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h --- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h +++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h @@ -33,6 +33,7 @@ llvm::ArrayRef
Providers)>; /// Find and report all references to symbols in a region of code. +/// It will filter out references that are not spelled in the main file. /// /// The AST traversal is rooted at ASTRoots - typically top-level declarations /// of a single source file. diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp --- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp +++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp @@ -24,8 +24,9 @@ // This is duplicated in writeHTMLReport, changes should be mirrored there. tooling::stdlib::Recognizer Recognizer; for (auto *Root : ASTRoots) { - auto &SM = Root->getASTContext().getSourceManager(); walkAST(*Root, [&](SourceLocation Loc, NamedDecl &ND, RefType RT) { + if (!SM.isWrittenInMainFile(SM.getSpellingLoc(Loc))) + return; SymbolReference SymRef{Loc, ND, RT}; if (auto SS = Recognizer(&ND)) { // FIXME: Also report forward decls from main-file, so that the caller @@ -38,6 +39,8 @@ } for (const SymbolReference &MacroRef : MacroRefs) { assert(MacroRef.Target.kind() == Symbol::Macro); + if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation))) + continue; CB(MacroRef, findHeaders(MacroRef.Target.macro().Definition, SM, PI)); } } diff --git a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp --- a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp +++ b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp @@ -527,12 +527,18 @@ HeaderSearch &HS, PragmaIncludes *PI, llvm::raw_ostream &OS) { Reporter R(OS, Ctx, HS, Includes, PI, File); + const auto& SM = Ctx.getSourceManager(); for (Decl *Root : Roots) walkAST(*Root, [&](SourceLocation Loc, const NamedDecl &D, RefType T) { + if(!SM.isWrittenInMainFile(SM.getSpellingLoc(Loc))) + return; R.addRef(SymbolReference{Loc, D, T}); }); - for (const SymbolReference &Ref : MacroRefs) + for (const SymbolReference &Ref : MacroRefs) { + if (!SM.isWrittenInMainFile(SM.getSpellingLoc(Ref.RefLocation))) + continue; R.addRef(Ref); + } R.write(); } diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp --- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp @@ -18,13 +18,16 @@ #include "clang/Tooling/Inclusions/StandardLibrary.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/Support/ScopedPrinter.h" #include "llvm/Testing/Support/Annotations.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include +#include namespace clang::include_cleaner { namespace { +using testing::AllOf; using testing::Pair; using testing::UnorderedElementsAre; @@ -134,6 +137,127 @@ UnorderedElementsAre(Pair(Main.point(), UnorderedElementsAre(HdrFile)))); } +MATCHER_P3(expandedAt, FileID, Offset, SM, "") { + auto [ExpanedFileID, ExpandedOffset] = SM->getDecomposedExpansionLoc(arg); + return ExpanedFileID == FileID && ExpandedOffset == Offset; +} +MATCHER_P3(spelledAt, FileID, Offset, SM, "") { + auto [SpelledFileID, SpelledOffset] = SM->getDecomposedSpellingLoc(arg); + return SpelledFileID == FileID && SpelledOffset == Offset; +} +TEST(WalkUsed, FilterRefsNotSpelledInMainFile) { + // Each test is expected to have a single expected ref of `target` symbol + // (or have none). + // The location in the reported ref is a macro location. $expand points to + // the macro location, and $spell points to the spelled location. + struct { + llvm::StringRef Header; + llvm::StringRef Main; + } TestCases[] = { + // Tests for decl references. + { + /*Header=*/"", + R"cpp( + int target(); + #define CALL_FUNC $spell^target() + + int b = $expand^CALL_FUNC; + )cpp", + }, + {/*Header=*/R"cpp( + int target(); + #define CALL_FUNC target() + )cpp", + // No ref of `target` being reported, as it is not spelled in main file. + "int a = CALL_FUNC;"}, + { + /*Header=*/"", + R"cpp( + int target; + #define PLUS_ONE(X) X + 1 + int a = $expand^PLUS_ONE($spell^target); + )cpp", + }, + { + /*Header*/ R"cpp( + int target; + #define PLUS_ONE(X) X + 1 + )cpp", + R"cpp( + int a = $expand^PLUS_ONE($spell^target); + )cpp", + }, + { + /*Header=*/"#define PLUS_ONE(X) X + 1", + R"cpp( + int target; + int a = $expand^PLUS_ONE($spell^target); + )cpp", + }, + // Tests for macro references, + {/*Header=*/"", + R"cpp( + #define target 1 + #define USE_target $spell^target + int b = $expand^USE_target; + )cpp"}, + {/*Header=*/R"cpp( + #define target 1 + #define USE_target target + )cpp", + // No ref of `target` being reported, it is not spelled in main file. + R"cpp( + int a = USE_target; + )cpp"}, + }; + + for (const auto &T : TestCases) { + llvm::Annotations Main(T.Main); + TestInputs Inputs(Main.code()); + Inputs.ExtraFiles["header.h"] = guard(T.Header); + RecordedPP Recorded; + Inputs.MakeAction = [&]() { + struct RecordAction : public SyntaxOnlyAction { + RecordedPP &Out; + RecordAction(RecordedPP &Out) : Out(Out) {} + bool BeginSourceFileAction(clang::CompilerInstance &CI) override { + auto &PP = CI.getPreprocessor(); + PP.addPPCallbacks(Out.record(PP)); + return true; + } + }; + return std::make_unique(Recorded); + }; + Inputs.ExtraArgs.push_back("-include"); + Inputs.ExtraArgs.push_back("header.h"); + TestAST AST(Inputs); + llvm::SmallVector TopLevelDecls; + for (Decl *D : AST.context().getTranslationUnitDecl()->decls()) + TopLevelDecls.emplace_back(D); + auto &SM = AST.sourceManager(); + + std::optional RefLoc; + walkUsed(TopLevelDecls, Recorded.MacroReferences, + /*PragmaIncludes=*/nullptr, SM, + [&](const SymbolReference &Ref, llvm::ArrayRef
) { + if (!Ref.RefLocation.isMacroID()) + return; + if (llvm::to_string(Ref.Target) == "target") { + ASSERT_FALSE(RefLoc) + << "Expected only one 'target' ref loc per testcase"; + RefLoc = Ref.RefLocation; + } + }); + FileID MainFID = SM.getMainFileID(); + if (RefLoc) { + EXPECT_THAT(*RefLoc, AllOf(expandedAt(MainFID, Main.point("expand"), &SM), + spelledAt(MainFID, Main.point("spell"), &SM))) + << T.Main; + } else { + EXPECT_THAT(Main.points(), testing::IsEmpty()); + } + } +} } // namespace } // namespace clang::include_cleaner