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;
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;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits