Skip to content

Commit d19ba74

Browse files
committed
[Includecleaner] Introduce RefType to ast walking
RefTypes are distinct categories for each reference to a symbol. They are signals indicating strength of a reference, that'll enable different decision making based on the finding being provided. There are 3 kinds of ref types: - Explicit, the reference is spelled in the code. - Implicit, the reference is not directly visible in the code. - Ambigious, the reference exists but can't be proven as used (e.g. overloads brought in by a using decl but not used by the code). Differential Revision: https://reviews.llvm.org/D135859
1 parent aa37342 commit d19ba74

File tree

11 files changed

+165
-84
lines changed

11 files changed

+165
-84
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ namespace include_cleaner {
2626
///
2727
/// References occur at a particular location, refer to a single symbol, and
2828
/// that symbol may be provided by several headers.
29-
/// FIXME: Provide signals about the reference type and providing headers so the
30-
/// caller can filter and rank the results.
31-
using UsedSymbolCB = llvm::function_ref<void(
32-
SourceLocation RefLoc, Symbol Target, llvm::ArrayRef<Header> Providers)>;
29+
/// FIXME: Provide signals about the providing headers so the caller can filter
30+
/// and rank the results.
31+
using UsedSymbolCB = llvm::function_ref<void(SymbolReference SymRef,
32+
llvm::ArrayRef<Header> Providers)>;
3333

3434
/// Find and report all references to symbols in a region of code.
3535
///

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,7 @@ struct Macro {
4242
/// The location of the Name where the macro is defined.
4343
SourceLocation Definition;
4444

45-
bool operator==(const Macro &S) const {
46-
return Definition == S.Definition;
47-
}
45+
bool operator==(const Macro &S) const { return Definition == S.Definition; }
4846
};
4947

5048
/// An entity that can be referenced in the code.
@@ -78,12 +76,24 @@ struct Symbol {
7876
};
7977
llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Symbol &);
8078

79+
/// Indicates the relation between the reference and the target.
80+
enum class RefType {
81+
/// Target is named by the reference, e.g. function call.
82+
Explicit,
83+
/// Target isn't spelled, e.g. default constructor call in `Foo f;`
84+
Implicit,
85+
/// Target's use can't be proven, e.g. a candidate for an unresolved overload.
86+
Ambiguous,
87+
};
88+
8189
/// Indicates that a piece of code refers to a symbol.
8290
struct SymbolReference {
83-
/// The symbol referred to.
84-
Symbol Symbol;
8591
/// The point in the code that refers to the symbol.
8692
SourceLocation RefLocation;
93+
/// The symbol referred to.
94+
Symbol Target;
95+
/// Relation type between the reference location and the target.
96+
RefType RT;
8797
};
8898
llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolReference &);
8999

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "clang-include-cleaner/Analysis.h"
10-
#include "clang-include-cleaner/Types.h"
1110
#include "AnalysisInternal.h"
11+
#include "clang-include-cleaner/Types.h"
1212
#include "clang/AST/ASTContext.h"
1313
#include "clang/Basic/SourceManager.h"
1414
#include "clang/Tooling/Inclusions/StandardLibrary.h"
@@ -32,17 +32,17 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots, UsedSymbolCB CB) {
3232
tooling::stdlib::Recognizer Recognizer;
3333
for (auto *Root : ASTRoots) {
3434
auto &SM = Root->getASTContext().getSourceManager();
35-
walkAST(*Root, [&](SourceLocation Loc, NamedDecl &ND) {
35+
walkAST(*Root, [&](SourceLocation Loc, NamedDecl &ND, RefType RT) {
3636
if (auto SS = Recognizer(&ND)) {
3737
// FIXME: Also report forward decls from main-file, so that the caller
3838
// can decide to insert/ignore a header.
39-
return CB(Loc, Symbol(*SS), toHeader(SS->headers()));
39+
return CB({Loc, Symbol(*SS), RT}, toHeader(SS->headers()));
4040
}
4141
// FIXME: Extract locations from redecls.
4242
// FIXME: Handle IWYU pragmas, non self-contained files.
4343
// FIXME: Handle macro locations.
4444
if (auto *FE = SM.getFileEntryForID(SM.getFileID(ND.getLocation())))
45-
return CB(Loc, Symbol(ND), {Header(FE)});
45+
return CB({Loc, Symbol(ND), RT}, {Header(FE)});
4646
});
4747
}
4848
// FIXME: Handle references of macros.

clang-tools-extra/include-cleaner/lib/AnalysisInternal.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#ifndef CLANG_INCLUDE_CLEANER_ANALYSISINTERNAL_H
2222
#define CLANG_INCLUDE_CLEANER_ANALYSISINTERNAL_H
2323

24+
#include "clang-include-cleaner/Types.h"
2425
#include "clang/Basic/SourceLocation.h"
2526
#include "llvm/ADT/STLFunctionalExtras.h"
2627

@@ -37,10 +38,13 @@ namespace include_cleaner {
3738
/// the primary location of the AST node found under Root.
3839
/// - the NamedDecl is the symbol referenced. It is canonical, rather than e.g.
3940
/// the redecl actually found by lookup.
41+
/// - the RefType describes the relation between the SourceLocation and the
42+
/// NamedDecl.
4043
///
4144
/// walkAST is typically called once per top-level declaration in the file
4245
/// being analyzed, in order to find all references within it.
43-
void walkAST(Decl &Root, llvm::function_ref<void(SourceLocation, NamedDecl &)>);
46+
void walkAST(Decl &Root,
47+
llvm::function_ref<void(SourceLocation, NamedDecl &, RefType)>);
4448

4549
/// Write an HTML summary of the analysis to the given stream.
4650
/// FIXME: Once analysis has a public API, this should be public too.

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
//===----------------------------------------------------------------------===//
1515

1616
#include "AnalysisInternal.h"
17+
#include "clang-include-cleaner/Analysis.h"
1718
#include "clang/AST/ASTContext.h"
1819
#include "clang/AST/Decl.h"
1920
#include "clang/AST/PrettyPrinter.h"
@@ -212,8 +213,9 @@ void writeHTMLReport(FileID File, llvm::ArrayRef<Decl *> Roots, ASTContext &Ctx,
212213
llvm::raw_ostream &OS) {
213214
Reporter R(OS, Ctx, File);
214215
for (Decl *Root : Roots)
215-
walkAST(*Root,
216-
[&](SourceLocation Loc, const NamedDecl &D) { R.addRef(Loc, D); });
216+
walkAST(*Root, [&](SourceLocation Loc, const NamedDecl &D, RefType) {
217+
R.addRef(Loc, D);
218+
});
217219
R.write();
218220
}
219221

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "clang-include-cleaner/Record.h"
10+
#include "clang-include-cleaner/Types.h"
1011
#include "clang/AST/ASTConsumer.h"
1112
#include "clang/AST/ASTContext.h"
1213
#include "clang/AST/DeclGroup.h"
@@ -86,8 +87,9 @@ class PPRecorder : public PPCallbacks {
8687
if (MI.isBuiltinMacro())
8788
return; // __FILE__ is not a reference.
8889
Recorded.MacroReferences.push_back(
89-
SymbolReference{Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()},
90-
Tok.getLocation()});
90+
SymbolReference{Tok.getLocation(),
91+
Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()},
92+
RefType::Explicit});
9193
}
9294

9395
bool Active = false;
@@ -254,5 +256,4 @@ std::unique_ptr<PPCallbacks> RecordedPP::record(const Preprocessor &PP) {
254256
return std::make_unique<PPRecorder>(*this, PP);
255257
}
256258

257-
258259
} // namespace clang::include_cleaner

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Include &I) {
4646
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const SymbolReference &R) {
4747
// We can't decode the Location without SourceManager. Its raw representation
4848
// isn't completely useless (and distinguishes SymbolReference from Symbol).
49-
return OS << R.Symbol << "@0x"
49+
return OS << R.Target << "@0x"
5050
<< llvm::utohexstr(R.RefLocation.getRawEncoding(),
5151
/*Width=*/CHAR_BIT *
5252
sizeof(SourceLocation::UIntTy));

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

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "AnalysisInternal.h"
10+
#include "clang-include-cleaner/Analysis.h"
1011
#include "clang/AST/Decl.h"
1112
#include "clang/AST/DeclCXX.h"
1213
#include "clang/AST/Expr.h"
@@ -16,15 +17,21 @@
1617
#include "clang/AST/Type.h"
1718
#include "clang/AST/TypeLoc.h"
1819
#include "clang/Basic/SourceLocation.h"
20+
#include "clang/Basic/SourceManager.h"
1921
#include "llvm/ADT/STLExtras.h"
2022
#include "llvm/Support/Casting.h"
2123

2224
namespace clang::include_cleaner {
2325
namespace {
24-
using DeclCallback = llvm::function_ref<void(SourceLocation, NamedDecl &)>;
26+
using DeclCallback =
27+
llvm::function_ref<void(SourceLocation, NamedDecl &, RefType)>;
2528

2629
class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
2730
DeclCallback Callback;
31+
// Whether we're traversing declarations coming from a header file.
32+
// This helps figure out whether certain symbols can be assumed as unused
33+
// (e.g. overloads brought into an implementation file, but not used).
34+
bool IsHeader = false;
2835

2936
bool handleTemplateName(SourceLocation Loc, TemplateName TN) {
3037
// For using-templates, only mark the alias.
@@ -36,14 +43,16 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
3643
return true;
3744
}
3845

39-
void report(SourceLocation Loc, NamedDecl *ND) {
46+
void report(SourceLocation Loc, NamedDecl *ND,
47+
RefType RT = RefType::Explicit) {
4048
if (!ND || Loc.isInvalid())
4149
return;
42-
Callback(Loc, *cast<NamedDecl>(ND->getCanonicalDecl()));
50+
Callback(Loc, *cast<NamedDecl>(ND->getCanonicalDecl()), RT);
4351
}
4452

4553
public:
46-
ASTWalker(DeclCallback Callback) : Callback(Callback) {}
54+
ASTWalker(DeclCallback Callback, bool IsHeader)
55+
: Callback(Callback), IsHeader(IsHeader) {}
4756

4857
bool VisitDeclRefExpr(DeclRefExpr *DRE) {
4958
report(DRE->getLocation(), DRE->getFoundDecl());
@@ -56,23 +65,31 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
5665
}
5766

5867
bool VisitCXXConstructExpr(CXXConstructExpr *E) {
59-
report(E->getLocation(), E->getConstructor());
68+
report(E->getLocation(), E->getConstructor(),
69+
E->getParenOrBraceRange().isValid() ? RefType::Explicit
70+
: RefType::Implicit);
6071
return true;
6172
}
6273

6374
bool VisitOverloadExpr(OverloadExpr *E) {
6475
// Since we can't prove which overloads are used, report all of them.
65-
// FIXME: Provide caller with the ability to make a decision for such uses.
66-
llvm::for_each(E->decls(),
67-
[this, E](NamedDecl *D) { report(E->getNameLoc(), D); });
76+
llvm::for_each(E->decls(), [this, E](NamedDecl *D) {
77+
report(E->getNameLoc(), D, RefType::Ambiguous);
78+
});
6879
return true;
6980
}
7081

7182
bool VisitUsingDecl(UsingDecl *UD) {
72-
// FIXME: Provide caller with the ability to tell apart used/non-used
73-
// targets.
74-
for (const auto *Shadow : UD->shadows())
75-
report(UD->getLocation(), Shadow->getTargetDecl());
83+
for (const auto *Shadow : UD->shadows()) {
84+
auto *TD = Shadow->getTargetDecl();
85+
auto IsUsed = TD->isUsed() || TD->isReferenced();
86+
// We ignore unused overloads inside implementation files, as the ones in
87+
// headers might still be used by the dependents of the header.
88+
if (!IsUsed && !IsHeader)
89+
continue;
90+
report(UD->getLocation(), TD,
91+
IsUsed ? RefType::Explicit : RefType::Ambiguous);
92+
}
7693
return true;
7794
}
7895

@@ -135,7 +152,14 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
135152
} // namespace
136153

137154
void walkAST(Decl &Root, DeclCallback Callback) {
138-
ASTWalker(Callback).TraverseDecl(&Root);
155+
auto &AST = Root.getASTContext();
156+
auto &SM = AST.getSourceManager();
157+
// If decl isn't written in main file, assume it's coming from an include,
158+
// hence written in a header.
159+
bool IsRootedAtHeader =
160+
AST.getLangOpts().IsHeaderFile ||
161+
!SM.isWrittenInMainFile(SM.getSpellingLoc(Root.getLocation()));
162+
ASTWalker(Callback, IsRootedAtHeader).TraverseDecl(&Root);
139163
}
140164

141165
} // namespace clang::include_cleaner

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,12 @@ TEST(WalkUsed, Basic) {
5050

5151
auto &SM = AST.sourceManager();
5252
llvm::DenseMap<size_t, std::vector<Header>> OffsetToProviders;
53-
walkUsed(TopLevelDecls, [&](SourceLocation RefLoc, Symbol S,
54-
llvm::ArrayRef<Header> Providers) {
55-
auto [FID, Offset] = SM.getDecomposedLoc(RefLoc);
56-
EXPECT_EQ(FID, SM.getMainFileID());
57-
OffsetToProviders.try_emplace(Offset, Providers.vec());
58-
});
53+
walkUsed(TopLevelDecls,
54+
[&](SymbolReference SymRef, llvm::ArrayRef<Header> Providers) {
55+
auto [FID, Offset] = SM.getDecomposedLoc(SymRef.RefLocation);
56+
EXPECT_EQ(FID, SM.getMainFileID());
57+
OffsetToProviders.try_emplace(Offset, Providers.vec());
58+
});
5959
auto HeaderFile = Header(AST.fileManager().getFile("header.h").get());
6060
auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
6161
auto VectorSTL = Header(tooling::stdlib::Header::named("<vector>").value());

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,15 +185,15 @@ TEST_F(RecordPPTest, CapturesMacroRefs) {
185185
SM.translateFile(AST.fileManager().getFile("header.h").get()),
186186
Header.point("def"));
187187
ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));
188-
Symbol OrigX = Recorded.MacroReferences.front().Symbol;
188+
Symbol OrigX = Recorded.MacroReferences.front().Target;
189189
EXPECT_EQ("X", OrigX.macro().Name->getName());
190190
EXPECT_EQ(Def, OrigX.macro().Definition);
191191

192192
std::vector<unsigned> RefOffsets;
193193
std::vector<unsigned> ExpOffsets; // Expansion locs of refs in macro locs.
194194
std::vector<SourceLocation> RefMacroLocs;
195195
for (const auto &Ref : Recorded.MacroReferences) {
196-
if (Ref.Symbol == OrigX) {
196+
if (Ref.Target == OrigX) {
197197
auto [FID, Off] = SM.getDecomposedLoc(Ref.RefLocation);
198198
if (FID == SM.getMainFileID()) {
199199
RefOffsets.push_back(Off);

0 commit comments

Comments
 (0)