Skip to content

Commit bf6e655

Browse files
committed
[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
1 parent b3df1ce commit bf6e655

File tree

4 files changed

+129
-3
lines changed

4 files changed

+129
-3
lines changed

clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ using UsedSymbolCB = llvm::function_ref<void(const SymbolReference &SymRef,
4040
llvm::ArrayRef<Header> Providers)>;
4141

4242
/// Find and report all references to symbols in a region of code.
43+
/// It only reports references from main file.
4344
///
4445
/// The AST traversal is rooted at ASTRoots - typically top-level declarations
4546
/// of a single source file.

clang-tools-extra/include-cleaner/lib/Analysis.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,9 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
4242
// This is duplicated in writeHTMLReport, changes should be mirrored there.
4343
tooling::stdlib::Recognizer Recognizer;
4444
for (auto *Root : ASTRoots) {
45-
auto &SM = Root->getASTContext().getSourceManager();
4645
walkAST(*Root, [&](SourceLocation Loc, NamedDecl &ND, RefType RT) {
46+
if (!SM.isWrittenInMainFile(SM.getSpellingLoc(Loc)))
47+
return;
4748
// FIXME: Most of the work done here is repetative. It might be useful to
4849
// have a cache/batching.
4950
SymbolReference SymRef{Loc, ND, RT};
@@ -52,7 +53,9 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
5253
}
5354
for (const SymbolReference &MacroRef : MacroRefs) {
5455
assert(MacroRef.Target.kind() == Symbol::Macro);
55-
CB(MacroRef, headersForSymbol(MacroRef.Target, SM, PI));
56+
if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation)))
57+
continue;
58+
CB(MacroRef, findHeaders(MacroRef.Target.macro().Definition, SM, PI));
5659
}
5760
}
5861

clang-tools-extra/include-cleaner/lib/HTMLReport.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,12 +518,18 @@ void writeHTMLReport(FileID File, const include_cleaner::Includes &Includes,
518518
HeaderSearch &HS, PragmaIncludes *PI,
519519
llvm::raw_ostream &OS) {
520520
Reporter R(OS, Ctx, HS, Includes, PI, File);
521+
const auto& SM = Ctx.getSourceManager();
521522
for (Decl *Root : Roots)
522523
walkAST(*Root, [&](SourceLocation Loc, const NamedDecl &D, RefType T) {
524+
if(!SM.isWrittenInMainFile(SM.getSpellingLoc(Loc)))
525+
return;
523526
R.addRef(SymbolReference{Loc, D, T});
524527
});
525-
for (const SymbolReference &Ref : MacroRefs)
528+
for (const SymbolReference &Ref : MacroRefs) {
529+
if (!SM.isWrittenInMainFile(SM.getSpellingLoc(Ref.RefLocation)))
530+
continue;
526531
R.addRef(Ref);
532+
}
527533
R.write();
528534
}
529535

clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "clang/Tooling/Inclusions/StandardLibrary.h"
1919
#include "llvm/ADT/ArrayRef.h"
2020
#include "llvm/ADT/SmallVector.h"
21+
#include "llvm/Support/ScopedPrinter.h"
2122
#include "llvm/Testing/Support/Annotations.h"
2223
#include "gmock/gmock.h"
2324
#include "gtest/gtest.h"
@@ -27,6 +28,7 @@ namespace clang::include_cleaner {
2728
namespace {
2829
using testing::Contains;
2930
using testing::ElementsAre;
31+
using testing::AllOf;
3032
using testing::Pair;
3133
using testing::UnorderedElementsAre;
3234

@@ -252,5 +254,119 @@ TEST(FixIncludes, Basic) {
252254
)cpp");
253255
}
254256

257+
MATCHER_P3(expandedAt, FileID, Offset, SM, "") {
258+
auto [ExpanedFileID, ExpandedOffset] = SM->getDecomposedExpansionLoc(arg);
259+
return ExpanedFileID == FileID && ExpandedOffset == Offset;
260+
}
261+
MATCHER_P3(spelledAt, FileID, Offset, SM, "") {
262+
auto [SpelledFileID, SpelledOffset] = SM->getDecomposedSpellingLoc(arg);
263+
return SpelledFileID == FileID && SpelledOffset == Offset;
264+
}
265+
TEST(WalkUsed, FilterRefsNotSpelledInMainFile) {
266+
// Each test is expected to have a single expected ref of `target` symbol
267+
// (or have none).
268+
// The location in the reported ref is a macro location. $expand points to
269+
// the macro location, and $spell points to the spelled location.
270+
struct {
271+
llvm::StringRef Header;
272+
llvm::StringRef Main;
273+
} TestCases[] = {
274+
// Tests for decl references.
275+
{
276+
/*Header=*/"int target();",
277+
R"cpp(
278+
#define CALL_FUNC $spell^target()
279+
280+
int b = $expand^CALL_FUNC;
281+
)cpp",
282+
},
283+
{/*Header=*/R"cpp(
284+
int target();
285+
#define CALL_FUNC target()
286+
)cpp",
287+
// No ref of `target` being reported, as it is not spelled in main file.
288+
"int a = CALL_FUNC;"},
289+
{
290+
/*Header=*/R"cpp(
291+
int target();
292+
#define PLUS_ONE(X) X() + 1
293+
)cpp",
294+
R"cpp(
295+
int a = $expand^PLUS_ONE($spell^target);
296+
)cpp",
297+
},
298+
{
299+
/*Header=*/R"cpp(
300+
int target();
301+
#define PLUS_ONE(X) X() + 1
302+
)cpp",
303+
R"cpp(
304+
int a = $expand^PLUS_ONE($spell^target);
305+
)cpp",
306+
},
307+
// Tests for macro references
308+
{/*Header=*/"#define target 1",
309+
R"cpp(
310+
#define USE_target $spell^target
311+
int b = $expand^USE_target;
312+
)cpp"},
313+
{/*Header=*/R"cpp(
314+
#define target 1
315+
#define USE_target target
316+
)cpp",
317+
// No ref of `target` being reported, it is not spelled in main file.
318+
R"cpp(
319+
int a = USE_target;
320+
)cpp"},
321+
};
322+
323+
for (const auto &T : TestCases) {
324+
llvm::Annotations Main(T.Main);
325+
TestInputs Inputs(Main.code());
326+
Inputs.ExtraFiles["header.h"] = guard(T.Header);
327+
RecordedPP Recorded;
328+
Inputs.MakeAction = [&]() {
329+
struct RecordAction : public SyntaxOnlyAction {
330+
RecordedPP &Out;
331+
RecordAction(RecordedPP &Out) : Out(Out) {}
332+
bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
333+
auto &PP = CI.getPreprocessor();
334+
PP.addPPCallbacks(Out.record(PP));
335+
return true;
336+
}
337+
};
338+
return std::make_unique<RecordAction>(Recorded);
339+
};
340+
Inputs.ExtraArgs.push_back("-include");
341+
Inputs.ExtraArgs.push_back("header.h");
342+
TestAST AST(Inputs);
343+
llvm::SmallVector<Decl *> TopLevelDecls;
344+
for (Decl *D : AST.context().getTranslationUnitDecl()->decls())
345+
TopLevelDecls.emplace_back(D);
346+
auto &SM = AST.sourceManager();
347+
348+
SourceLocation RefLoc;
349+
walkUsed(TopLevelDecls, Recorded.MacroReferences,
350+
/*PragmaIncludes=*/nullptr, SM,
351+
[&](const SymbolReference &Ref, llvm::ArrayRef<Header>) {
352+
if (!Ref.RefLocation.isMacroID())
353+
return;
354+
if (llvm::to_string(Ref.Target) == "target") {
355+
ASSERT_TRUE(RefLoc.isInvalid())
356+
<< "Expected only one 'target' ref loc per testcase";
357+
RefLoc = Ref.RefLocation;
358+
}
359+
});
360+
FileID MainFID = SM.getMainFileID();
361+
if (RefLoc.isValid()) {
362+
EXPECT_THAT(RefLoc, AllOf(expandedAt(MainFID, Main.point("expand"), &SM),
363+
spelledAt(MainFID, Main.point("spell"), &SM)))
364+
<< T.Main;
365+
} else {
366+
EXPECT_THAT(Main.points(), testing::IsEmpty());
367+
}
368+
}
369+
}
370+
255371
} // namespace
256372
} // namespace clang::include_cleaner

0 commit comments

Comments
 (0)