[clang-tools-extra] bf6e655 - [include-cleaner] Filter out references that not spelled in the main file.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 9 03:59:44 PST 2022
Author: Haojian Wu
Date: 2022-12-09T12:59:34+01:00
New Revision: bf6e65516223586180d4082735b706bd4602ba11
URL: https://github.com/llvm/llvm-project/commit/bf6e65516223586180d4082735b706bd4602ba11
DIFF: https://github.com/llvm/llvm-project/commit/bf6e65516223586180d4082735b706bd4602ba11.diff
LOG: [include-cleaner] Filter out references that not spelled in the main file.
A HTML report of gtest after this patch:
https://gist.githubusercontent.com/hokein/73eee6f65a803e5702d8388c187524a6/raw/cf05a503519703a2fb87840bb0b270fe11a7a9fd/RecordTest.html
Differential Revision: https://reviews.llvm.org/D138779
Added:
Modified:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
Removed:
################################################################################
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 146c652f730de..f6afaff09cfd9 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<void(const SymbolReference &SymRef,
llvm::ArrayRef<Header> 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 fa3bbaab27c24..9ffa8e7f3a15d 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<Decl *> 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<Decl *> 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 6d165b5d22ded..22debdc5e83d5 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 13f5aad4912a4..72bec106c12e0 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<RecordAction>(Recorded);
+ };
+ Inputs.ExtraArgs.push_back("-include");
+ Inputs.ExtraArgs.push_back("header.h");
+ TestAST AST(Inputs);
+ llvm::SmallVector<Decl *> 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<Header>) {
+ 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
More information about the cfe-commits
mailing list