https://github.com/kadircet created https://github.com/llvm/llvm-project/pull/175532
Previously we completely ignored these references as we couldn't detect whether some pieces of concat'd token originated from main file and we wanted to prevent false positives. Unfortunately these are resulting in false negatives in certain cases and are breaking builds. After this change, include-cleaner will treat such references as ambigious to prevent deletion of likely-used headers (if they're already directly included), while still giving user the opportunity to explicitly delete them. From 2d0cf74aed982e220c41ccc2240d4d3c681a06f1 Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya <[email protected]> Date: Mon, 12 Jan 2026 13:53:58 +0100 Subject: [PATCH] [include-cleaner] Report refs from macro-concat'd tokens as ambigious Previously we completely ignored these references as we couldn't detect whether some pieces of concat'd token originated from main file and we wanted to prevent false positives. Unfortunately these are resulting in false negatives in certain cases and are breaking builds. After this change, include-cleaner will treat such references as ambigious to prevent deletion of likely-used headers (if they're already directly included), while still giving user the opportunity to explicitly delete them. --- .../include-cleaner/lib/Analysis.cpp | 14 +++++++++- .../unittests/AnalysisTest.cpp | 26 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp index a1781f4e24f2e..e48a380211af0 100644 --- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp +++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp @@ -58,7 +58,19 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots, tooling::stdlib::Recognizer Recognizer; for (auto *Root : ASTRoots) { walkAST(*Root, [&](SourceLocation Loc, NamedDecl &ND, RefType RT) { - auto FID = SM.getFileID(SM.getSpellingLoc(Loc)); + auto SpellLoc = SM.getSpellingLoc(Loc); + // Tokens resulting from macro concatenation ends up in scratch space and + // clang currently doesn't have a good/simple APIs for tracking where + // pieces of a concataned token originated from. + // So we use the macro expansion location instead, and downgrade reference + // type to ambigious to prevent false negatives. + if (SM.isWrittenInScratchSpace(SpellLoc)) { + Loc = SM.getExpansionLoc(Loc); + if (RT == RefType::Explicit) + RT = RefType::Ambiguous; + SpellLoc = SM.getSpellingLoc(Loc); + } + auto FID = SM.getFileID(SpellLoc); if (FID != SM.getMainFileID() && FID != SM.getPreambleFileID()) return; // FIXME: Most of the work done here is repetitive. It might be useful to diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp index 74321c312cb71..ba5a3fbbcaeb2 100644 --- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp @@ -680,5 +680,31 @@ TEST_F(WalkUsedTest, IgnoresIdentityMacros) { // FIXME: we should have a reference from stdin to header.h Pair(Code.point("bar"), UnorderedElementsAre(MainFile)))); } + +TEST_F(WalkUsedTest, MacroConcat) { + llvm::Annotations Code(R"cpp( + #include "header.h" + void f() { + $xyz^FOO(xyz) = 1; + $bar^BAR = 2; + } + )cpp"); + Inputs.Code = Code.code(); + Inputs.ExtraFiles["header.h"] = guard(R"cpp( + #define FOO(x) FLAGS_##x + #define BAR FOO(bb) + + int FLAGS_xyz; + int FLAGS_bb; + )cpp"); + + TestAST AST(Inputs); + auto &SM = AST.sourceManager(); + auto Header = *SM.getFileManager().getOptionalFileRef("header.h"); + EXPECT_THAT( + offsetToProviders(AST), + AllOf(Contains(Pair(Code.point("bar"), UnorderedElementsAre(Header))), + Contains(Pair(Code.point("xyz"), UnorderedElementsAre(Header))))); +} } // namespace } // namespace clang::include_cleaner _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
