Skip to content

Commit

Permalink
[include-cleaner] Filter out references that not spelled in the main …
Browse files Browse the repository at this point in the history
  • Loading branch information
hokein committed Dec 9, 2022
1 parent b3df1ce commit bf6e655
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 3 deletions.
Expand Up @@ -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.
Expand Down
7 changes: 5 additions & 2 deletions clang-tools-extra/include-cleaner/lib/Analysis.cpp
Expand Up @@ -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};
Expand All @@ -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));
}
}

Expand Down
8 changes: 7 additions & 1 deletion clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
Expand Up @@ -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();
}

Expand Down
116 changes: 116 additions & 0 deletions clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
Expand Up @@ -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"
Expand All @@ -27,6 +28,7 @@ namespace clang::include_cleaner {
namespace {
using testing::Contains;
using testing::ElementsAre;
using testing::AllOf;
using testing::Pair;
using testing::UnorderedElementsAre;

Expand Down Expand Up @@ -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

0 comments on commit bf6e655

Please sign in to comment.