[PATCH] D157434: [include-cleaner] Add a simple heuristic to handle token-pasting symbols.
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 8 13:20:04 PDT 2023
hokein created this revision.
hokein added a reviewer: kadircet.
Herald added a project: All.
hokein requested review of this revision.
Herald added a project: clang-tools-extra.
include-cleaner didn't report the usage of token-pasting symbols, as
these symbols are spelled in a "scratch space" file which is not a real
file, thus we have some false postives of unused includes for these
symbols (e.g. ABSL_FLAG).
This patch adds a simple heuristic to handle this case, we use the
expansion location as a "proxy" reference location, this should work
well on token-pasting formed from macro arguemtns.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D157434
Files:
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -197,6 +197,31 @@
Pair(Code.point("4"), UnorderedElementsAre(MainFile))));
}
+TEST_F(WalkUsedTest, TokenPasting) {
+ llvm::Annotations Code(R"cpp(
+ #define DEFINE_FLAG(name) int FLAG_##name = 0;
+ #define MY_FLAG(name) DEFINE_FLAG(MY_##name)
+
+ #define PASTED_TOKEN X##2
+
+ $1^DEFINE_FLAG(abc);
+ $2^MY_FLAG(abc);
+
+ int $3^TPASTED_TOKEN = 3;
+ )cpp");
+ Inputs.Code = Code.code();
+
+ TestAST AST(Inputs);
+ auto &SM = AST.sourceManager();
+ auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+
+ EXPECT_THAT(offsetToProviders(AST, SM),
+ UnorderedElementsAre(
+ Pair(Code.point("1"), UnorderedElementsAre(MainFile)),
+ Pair(Code.point("2"), UnorderedElementsAre(MainFile)),
+ Pair(Code.point("3"), UnorderedElementsAre(MainFile))));
+}
+
class AnalyzeTest : public testing::Test {
protected:
TestInputs Inputs;
Index: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -519,6 +519,9 @@
const auto& SM = Ctx.getSourceManager();
for (Decl *Root : Roots)
walkAST(*Root, [&](SourceLocation Loc, const NamedDecl &D, RefType T) {
+ if (SM.isWrittenInScratchSpace( SM.getSpellingLoc(Loc)))
+ Loc = SM.getExpansionLoc(Loc);
+
if(!SM.isWrittenInMainFile(SM.getSpellingLoc(Loc)))
return;
R.addRef(SymbolReference{D, Loc, T});
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -40,6 +40,15 @@
tooling::stdlib::Recognizer Recognizer;
for (auto *Root : ASTRoots) {
walkAST(*Root, [&](SourceLocation Loc, NamedDecl &ND, RefType RT) {
+ // If the symbol is spelled as a token paste, it spelling location points
+ // to <scratch space> file which is not a real file. We fallback to use
+ // the expansion location, this herustic is based on the assumption that
+ // most of token pastings are formed with the macro arguement, and it can
+ // allow us to detect uses of symbol defined by the common macro like
+ // `ABSL_FLAG`.
+ if (SM.isWrittenInScratchSpace(SM.getSpellingLoc(Loc)))
+ Loc = SM.getExpansionLoc(Loc);
+
auto FID = SM.getFileID(SM.getSpellingLoc(Loc));
if (FID != SM.getMainFileID() && FID != SM.getPreambleFileID())
return;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D157434.548329.patch
Type: text/x-patch
Size: 2921 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230808/a7a1c273/attachment.bin>
More information about the cfe-commits
mailing list