https://github.com/arixsnow updated https://github.com/llvm/llvm-project/pull/230133
>From 6179f67a9207f812af3fc99542aa85f5db2eeef2 Mon Sep 17 00:00:00 2001 From: Arka Mondal <[email protected]> Date: Thu, 8 Oct 2026 22:48:03 +0900 Subject: [PATCH 1/3] [include-cleaner] Report project declarations of standard library names The stdlib Recognizer matches by name and namespace only, so a recognized symbol was attributed to the standard header alone. In a freestanding project, a header that declares strcmp and is included only for it was reported unused, and removing it breaks the build. A user-defined log2 and its locals (#93335) and a C macro named I (#172816) were reported as missing an include. locateDecl and locateMacro now also return the physical locations of the declarations or the macro definition, after the standard library location and with no hints, so standard headers still rank first. Implicit declarations are skipped: clang creates one at the call site of an undeclared library function, and it would hide the missing include. Declarations that do not complete the symbol are skipped too: a forward declaration could leave the header with the definition reported unused. headersForSymbol drops physical locations in system headers when a standard library location exists, since those are normally the standard library's own declarations. Resolves the FIXME left in HeadersForSymbolTest.StandardHeaders by the revert of #94923 (ae0813f2d722). Fixes #172816. Assisted-by: Claude Code --- .../clangd/unittests/IncludeCleanerTests.cpp | 22 ++++++- clang-tools-extra/docs/ReleaseNotes.md | 11 ++++ .../include-cleaner/lib/FindHeaders.cpp | 11 +++- .../include-cleaner/lib/LocateSymbol.cpp | 21 ++++-- .../unittests/AnalysisTest.cpp | 65 +++++++++++++++---- .../unittests/FindHeadersTest.cpp | 41 +++++++++--- .../unittests/LocateSymbolTest.cpp | 21 +++++- .../clang-tidy/checkers/misc/Inputs/kstring.h | 3 + .../checkers/misc/include-cleaner.c | 10 +++ 9 files changed, 174 insertions(+), 31 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/kstring.h create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.c diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp index 5b9db8af33f12..72bde1ae73fee 100644 --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -100,6 +100,25 @@ TEST(IncludeCleaner, StdlibUnused) { ElementsAre(Pointee(writtenInclusion("<queue>")))); } +TEST(IncludeCleaner, StdlibNamesDeclaredByProject) { + auto TU = TestTU::withCode(R"c( + #include "sys/stddef.h" + #include "sys/string.h" + struct s { int a; }; + int x = offsetof(struct s, a); + int foo(const char *a, const char *b) { return strcmp(a, b); } + )c"); + TU.Filename = "main.c"; + TU.AdditionalFiles["sys/stddef.h"] = + guard("#define offsetof(t, m) __builtin_offsetof(t, m)"); + TU.AdditionalFiles["sys/string.h"] = + guard("int strcmp(const char *, const char *);"); + auto AST = TU.build(); + IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST); + EXPECT_THAT(Findings.UnusedIncludes, IsEmpty()); + EXPECT_THAT(Findings.MissingIncludes, IsEmpty()); +} + TEST(IncludeCleaner, GetUnusedHeaders) { llvm::StringLiteral MainFile = R"cpp( #include "a.h" @@ -296,7 +315,8 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) { // IWYU pragma: private, include "public.h" void foobar(); )cpp"); - TU.AdditionalFiles["header.h"] = guard(R"cpp( + TU.AdditionalFiles["header.h"] = guard("#include \"vector_impl.h\""); + TU.AdditionalFiles["vector_impl.h"] = guard(R"cpp( namespace std { class vector {}; } )cpp"); diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 8b32c22542aeb..ac4e46c8c3bd9 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -71,6 +71,10 @@ infrastructure are described first, followed by tool-specific sections. Custom checks are subject to `FastCheckFilter`, and therefore require `FastCheckFilter: Loose` or `None` to run. +- A header that declares a name also found in the standard library, such as + `strcmp` in a freestanding project's own string header, is no longer + reported as an unused include. Removing the include broke the build. + #### Semantic Highlighting #### Compile flags @@ -242,6 +246,13 @@ infrastructure are described first, followed by tool-specific sections. - No longer diagnoses parameters of `main`, whose signature is fixed by the standard. +- Improved {doc}`misc-include-cleaner + <clang-tidy/checks/misc/include-cleaner>` check by treating a project's own + declarations of standard library names, such as `strcmp` in a freestanding + project's string header, as providers. Such headers are no longer reported + as unused, and uses of those names are no longer reported as missing an + include. + - Fixed an infinite loop in {doc}`misc-multiple-inheritance <clang-tidy/checks/misc/multiple-inheritance>` when checking a class that inherits from itself or has a circular inheritance graph. diff --git a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp index 47f3c37501cdc..84171e300abff 100644 --- a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp +++ b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp @@ -251,8 +251,17 @@ llvm::SmallVector<Header> headersForSymbol(const Symbol &S, if (auto SpecialHeaders = headersForSpecialSymbol(S, SM, PI)) { Headers = std::move(*SpecialHeaders); } else { - for (auto &Loc : locateSymbol(S, PP.getLangOpts())) + auto Locations = locateSymbol(S, PP.getLangOpts()); + bool IsStdlibSymbol = llvm::any_of(Locations, [](const auto &Loc) { + return Loc.kind() == SymbolLocation::Standard; + }); + for (auto &Loc : Locations) { + // The stdlib providers already cover declarations in system headers. + if (IsStdlibSymbol && Loc.kind() == SymbolLocation::Physical && + (Loc.physical().isInvalid() || SM.isInSystemHeader(Loc.physical()))) + continue; Headers.append(applyHints(findHeaders(Loc, SM, PI), Loc.Hint)); + } } // If two Headers probably refer to the same file (e.g. Verbatim(foo.h) and // Physical(/path/to/foo.h), we won't deduplicate them or merge their hints diff --git a/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp b/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp index 1b4d8c2733cf8..c0bf6886e8ec1 100644 --- a/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp +++ b/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp @@ -47,9 +47,9 @@ Hints declHints(const Decl *D) { std::vector<Hinted<SymbolLocation>> locateDecl(const Decl &D) { std::vector<Hinted<SymbolLocation>> Result; - // FIXME: Should we also provide physical locations? - if (auto SS = tooling::stdlib::Recognizer()(&D)) - return {{*SS, Hints::CompleteSymbol}}; + auto SS = tooling::stdlib::Recognizer()(&D); + if (SS) + Result.push_back({*SS, Hints::CompleteSymbol}); // FIXME: Signal foreign decls, e.g. a forward declaration not owned by a // library. Some useful signals could be derived by checking the DeclContext. // Most incidental forward decls look like: @@ -57,16 +57,23 @@ std::vector<Hinted<SymbolLocation>> locateDecl(const Decl &D) { // class SourceManager; // likely an incidental forward decl. // namespace my_own_ns {} // } - for (auto *Redecl : D.redecls()) - Result.push_back({Redecl->getLocation(), declHints(Redecl)}); + for (auto *Redecl : D.redecls()) { + Hints H = declHints(Redecl); + if (SS) { + if (Redecl->isImplicit() || H != Hints::CompleteSymbol) + continue; + // Rank below the stdlib providers. + H = Hints::None; + } + Result.push_back({Redecl->getLocation(), H}); + } return Result; } std::vector<Hinted<SymbolLocation>> locateMacro(const Macro &M, const tooling::stdlib::Lang L) { - // FIXME: Should we also provide physical locations? if (auto SS = tooling::stdlib::Symbol::named("", M.Name->getName(), L)) - return {{*SS, Hints::CompleteSymbol}}; + return {{*SS, Hints::CompleteSymbol}, {M.Definition, Hints::None}}; return {{M.Definition, Hints::CompleteSymbol}}; } } // namespace diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp index 5df8792c93b2e..4bf125208ec22 100644 --- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp @@ -123,18 +123,19 @@ TEST_F(WalkUsedTest, Basic) { auto MainFile = Header(*SM.getFileEntryRefForID(SM.getMainFileID())); auto VectorSTL = Header(*tooling::stdlib::Header::named("<vector>")); auto UtilitySTL = Header(*tooling::stdlib::Header::named("<utility>")); - EXPECT_THAT( - offsetToProviders(AST), - UnorderedElementsAre( - Pair(Code.point("bar"), UnorderedElementsAre(MainFile)), - Pair(Code.point("private"), - UnorderedElementsAre(PublicFile, PrivateFile)), - Pair(Code.point("foo"), UnorderedElementsAre(HeaderFile)), - Pair(Code.point("vector"), UnorderedElementsAre(VectorSTL)), - Pair(Code.point("vconstructor"), UnorderedElementsAre(VectorSTL)), - Pair(Code.point("v"), UnorderedElementsAre(MainFile)), - Pair(Code.point("builtin"), testing::IsEmpty()), - Pair(Code.point("move"), UnorderedElementsAre(UtilitySTL)))); + EXPECT_THAT(offsetToProviders(AST), + UnorderedElementsAre( + Pair(Code.point("bar"), UnorderedElementsAre(MainFile)), + Pair(Code.point("private"), + UnorderedElementsAre(PublicFile, PrivateFile)), + Pair(Code.point("foo"), UnorderedElementsAre(HeaderFile)), + Pair(Code.point("vector"), + UnorderedElementsAre(VectorSTL, HeaderFile)), + Pair(Code.point("vconstructor"), + UnorderedElementsAre(VectorSTL, HeaderFile)), + Pair(Code.point("v"), UnorderedElementsAre(MainFile)), + Pair(Code.point("builtin"), testing::IsEmpty()), + Pair(Code.point("move"), UnorderedElementsAre(UtilitySTL)))); } TEST_F(WalkUsedTest, MultipleProviders) { @@ -319,6 +320,46 @@ TEST_F(AnalyzeTest, ResourceDirIsIgnored) { EXPECT_THAT(Results.Missing, testing::IsEmpty()); } +TEST_F(AnalyzeTest, StandardNamesDeclaredByProject) { + Inputs.ExtraArgs.push_back("-xc"); + Inputs.Code = R"cpp( +#include "sys/stddef.h" +#include "sys/string.h" + +struct s { int a; }; +int x = offsetof(struct s, a); +int foo(const char *a, const char *b) { return strcmp(a, b); } +)cpp"; + Inputs.ExtraFiles["sys/stddef.h"] = + guard("#define offsetof(t, m) __builtin_offsetof(t, m)"); + Inputs.ExtraFiles["sys/string.h"] = + guard("int strcmp(const char *, const char *);"); + TestAST AST(Inputs); + auto Decls = AST.context().getTranslationUnitDecl()->decls(); + auto Results = + analyze(std::vector<Decl *>{Decls.begin(), Decls.end()}, + PP.MacroReferences, PP.Includes, &PI, AST.preprocessor()); + EXPECT_THAT(Results.Unused, testing::IsEmpty()); + EXPECT_THAT(Results.Missing, testing::IsEmpty()); +} + +TEST_F(AnalyzeTest, UndeclaredLibraryFunctionIsMissing) { + Inputs.ExtraArgs.push_back("-xc"); + Inputs.ErrorOK = true; + Inputs.Code = R"cpp( +int foo(void) { return strlen("x"); } +)cpp"; + TestAST AST(Inputs); + auto Decls = AST.context().getTranslationUnitDecl()->decls(); + auto Results = + analyze(std::vector<Decl *>{Decls.begin(), Decls.end()}, + PP.MacroReferences, PP.Includes, &PI, AST.preprocessor()); + EXPECT_THAT(Results.Missing, + ElementsAre(Pair(testing::_, + Header(*tooling::stdlib::Header::named( + "<string.h>", tooling::stdlib::Lang::C))))); +} + TEST_F(AnalyzeTest, DifferentHeaderSameSpelling) { Inputs.ExtraArgs.push_back("-Ifoo"); Inputs.ExtraArgs.push_back("-Ifoo_inner"); diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp index 0ac243937e6e4..c05781589d6c9 100644 --- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp @@ -625,14 +625,39 @@ TEST_F(HeadersForSymbolTest, StandardHeaders) { )cpp"; Inputs.ExtraFiles["stdlib_internal.h"] = "void assert();"; buildAST(); - EXPECT_THAT( - headersFor("assert"), - // Respect the ordering from the stdlib mapping. - // FIXME: Report physical locations too, stdlib_internal.h and main-file - // should also be candidates. But they should be down-ranked compared to - // stdlib providers. - UnorderedElementsAre(tooling::stdlib::Header::named("<cassert>"), - tooling::stdlib::Header::named("<assert.h>"))); + EXPECT_THAT(headersFor("assert"), + // Respect the ordering from the stdlib mapping. + ElementsAre(tooling::stdlib::Header::named("<cassert>"), + tooling::stdlib::Header::named("<assert.h>"), + physicalHeader("input.mm"), + physicalHeader("stdlib_internal.h"))); +} + +TEST_F(HeadersForSymbolTest, StandardHeadersDeclaredInSystemHeader) { + Inputs.Code = R"cpp( + #include <assert_impl.h> + void foo() { assert(); } + )cpp"; + Inputs.ExtraFiles["assert_impl.h"] = guard("void assert();"); + Inputs.ExtraArgs.push_back("-isystem."); + buildAST(); + EXPECT_THAT(headersFor("assert"), + ElementsAre(tooling::stdlib::Header::named("<cassert>"), + tooling::stdlib::Header::named("<assert.h>"))); +} + +TEST_F(HeadersForSymbolTest, StandardNameForwardDeclaredByProject) { + Inputs.Code = R"cpp( + #include "fwd.h" + #include "vector_impl.h" + )cpp"; + Inputs.ExtraFiles["fwd.h"] = guard("namespace std { class vector; }"); + Inputs.ExtraFiles["vector_impl.h"] = + guard("namespace std { class vector {}; }"); + buildAST(); + EXPECT_THAT(headersFor("vector"), + ElementsAre(tooling::stdlib::Header::named("<vector>"), + physicalHeader("vector_impl.h"))); } TEST_F(HeadersForSymbolTest, StdlibLangForMacros) { diff --git a/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp b/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp index 428b2df109689..206deb3d9be88 100644 --- a/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp @@ -132,9 +132,10 @@ TEST(LocateSymbol, Stdlib) { ElementsAre(*tooling::stdlib::Symbol::named("std::", "vector"))); } { - LocateExample Test("#define assert(x)\nvoid foo() { assert(true); }"); + LocateExample Test("#define ^assert(x)\nvoid foo() { assert(true); }"); EXPECT_THAT(locateSymbol(Test.findMacro("assert"), Test.langOpts()), - ElementsAre(*tooling::stdlib::Symbol::named("", "assert"))); + ElementsAre(*tooling::stdlib::Symbol::named("", "assert"), + Test.points().front())); } } @@ -157,6 +158,22 @@ TEST(LocateSymbol, CompleteSymbolHint) { *tooling::stdlib::Symbol::named("std::", "vector"), Hints::CompleteSymbol))); } + { + LocateExample Test("namespace std { struct ^vector {}; }"); + EXPECT_THAT(locateSymbol(Test.findDecl("vector"), Test.langOpts()), + ElementsAre(HintedSymbol(*tooling::stdlib::Symbol::named( + "std::", "vector"), + Hints::CompleteSymbol), + HintedSymbol(Test.points().front(), Hints::None))); + } + { + LocateExample Test("#define ^assert(x)"); + EXPECT_THAT( + locateSymbol(Test.findMacro("assert"), Test.langOpts()), + ElementsAre(HintedSymbol(*tooling::stdlib::Symbol::named("", "assert"), + Hints::CompleteSymbol), + HintedSymbol(Test.points().front(), Hints::None))); + } { // macros are always complete. LocateExample Test("#define ^FOO"); diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/kstring.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/kstring.h new file mode 100644 index 0000000000000..e19a108f8a674 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/kstring.h @@ -0,0 +1,3 @@ +#pragma once + +int strcmp(const char *, const char *); diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.c b/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.c new file mode 100644 index 0000000000000..1a73c15a32572 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.c @@ -0,0 +1,10 @@ +// RUN: %check_clang_tidy %s misc-include-cleaner %t -- -- -I%S/Inputs -ffreestanding + +// These names are declared here or in kstring.h, so nothing is reported. +#include "kstring.h" + +#define I 42 + +int log2(int x) { return x; } + +int foo(const char *a, const char *b) { return strcmp(a, b) + log2(I); } >From 45f03bb7247dd07a3ea7a401b48efc96c0016b5e Mon Sep 17 00:00:00 2001 From: Arka Mondal <[email protected]> Date: Fri, 9 Oct 2026 15:31:26 +0900 Subject: [PATCH 2/3] Move the release note to an include-cleaner section --- clang-tools-extra/docs/ReleaseNotes.md | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index b7867283dcb03..a796235d555b8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -71,10 +71,6 @@ infrastructure are described first, followed by tool-specific sections. Custom checks are subject to `FastCheckFilter`, and therefore require `FastCheckFilter: Loose` or `None` to run. -- A header that declares a name also found in the standard library, such as - `strcmp` in a freestanding project's own string header, is no longer - reported as an unused include. Removing the include broke the build. - #### Semantic Highlighting #### Compile flags @@ -132,6 +128,14 @@ infrastructure are described first, followed by tool-specific sections. ### Improvements to clang-query +### Improvements to include-cleaner + +- A project's own declarations of standard library names, such as `strcmp` in + a freestanding project's string header, now count as providers. Such headers + are no longer reported as unused, and uses of those names are no longer + reported as missing an include. This applies to clangd and the + {doc}`misc-include-cleaner <clang-tidy/checks/misc/include-cleaner>` check. + ### Improvements to clang-tidy - Improved {program}`check_clang_tidy.py` by adding support of @@ -249,13 +253,6 @@ infrastructure are described first, followed by tool-specific sections. - No longer diagnoses parameters of `main`, whose signature is fixed by the standard. -- Improved {doc}`misc-include-cleaner - <clang-tidy/checks/misc/include-cleaner>` check by treating a project's own - declarations of standard library names, such as `strcmp` in a freestanding - project's string header, as providers. Such headers are no longer reported - as unused, and uses of those names are no longer reported as missing an - include. - - Fixed an infinite loop in {doc}`misc-multiple-inheritance <clang-tidy/checks/misc/multiple-inheritance>` when checking a class that inherits from itself or has a circular inheritance graph. >From 6101eae69f8db58bddc195b164f0c211ebbfa409 Mon Sep 17 00:00:00 2001 From: arkamondal <[email protected]> Date: Sun, 11 Oct 2026 17:15:02 +0900 Subject: [PATCH 3/3] Update clang-tools-extra/docs/ReleaseNotes.md Co-authored-by: EugeneZelenko <[email protected]> --- clang-tools-extra/docs/ReleaseNotes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index a796235d555b8..0d1682e509fcd 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -133,7 +133,7 @@ infrastructure are described first, followed by tool-specific sections. - A project's own declarations of standard library names, such as `strcmp` in a freestanding project's string header, now count as providers. Such headers are no longer reported as unused, and uses of those names are no longer - reported as missing an include. This applies to clangd and the + reported as missing an include. This applies to {program}`clangd` and the {doc}`misc-include-cleaner <clang-tidy/checks/misc/include-cleaner>` check. ### Improvements to clang-tidy _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
