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 index 146c652f730de7..f6afaff09cfd91 100644 --- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h +++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h @@ -40,6 +40,7 @@ using UsedSymbolCB = llvm::function_ref Providers)>; /// Find and report all references to symbols in a region of code. +/// It only reports references from 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 index fa3bbaab27c240..9ffa8e7f3a15d3 100644 --- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp +++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp @@ -42,8 +42,9 @@ void walkUsed(llvm::ArrayRef ASTRoots, // 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; // FIXME: Most of the work done here is repetative. It might be useful to // have a cache/batching. SymbolReference SymRef{Loc, ND, RT}; @@ -52,7 +53,9 @@ void walkUsed(llvm::ArrayRef ASTRoots, } for (const SymbolReference &MacroRef : MacroRefs) { assert(MacroRef.Target.kind() == Symbol::Macro); - CB(MacroRef, headersForSymbol(MacroRef.Target, SM, PI)); + 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 index 6d165b5d22ded7..22debdc5e83d5e 100644 --- a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp +++ b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp @@ -518,12 +518,18 @@ void writeHTMLReport(FileID File, const include_cleaner::Includes &Includes, 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 index 13f5aad4912a41..72bec106c12e0d 100644 --- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp @@ -18,6 +18,7 @@ #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" @@ -27,6 +28,7 @@ namespace clang::include_cleaner { namespace { using testing::Contains; using testing::ElementsAre; +using testing::AllOf; using testing::Pair; using testing::UnorderedElementsAre; @@ -252,5 +254,119 @@ TEST(FixIncludes, Basic) { )cpp"); } +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=*/"int target();", + R"cpp( + #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 + )cpp", + R"cpp( + 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", + }, + // Tests for macro references + {/*Header=*/"#define target 1", + R"cpp( + #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(); + + SourceLocation 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_TRUE(RefLoc.isInvalid()) + << "Expected only one 'target' ref loc per testcase"; + RefLoc = Ref.RefLocation; + } + }); + FileID MainFID = SM.getMainFileID(); + if (RefLoc.isValid()) { + 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