Author: Kadir Cetinkaya Date: 2023-09-06T14:46:33+02:00 New Revision: 73b2c86d95dc510d6972aaa4b44688aebebd89f1
URL: https://github.com/llvm/llvm-project/commit/73b2c86d95dc510d6972aaa4b44688aebebd89f1 DIFF: https://github.com/llvm/llvm-project/commit/73b2c86d95dc510d6972aaa4b44688aebebd89f1.diff LOG: [include-cleaner] Weaken signal for boosting preferred headers Putting preferred header signal above completeness implied we would uprank forward declarations above complete ones in certain cases. This can be desired in cases where: - Complete definition is private. But this case is already governed by publicness signal. - The library indeed intends to provide a forward declaring interface, like iosfwd vs ostream. In all other cases, upranking is undesired as it means we've picked up prefered headerness signal by mistake from an unrelated declaration to the library. This change regresses the behavior for libraries that intentionally provide a forward declaring interface. But that wasn't something we intended to support explicitly, it was working incidentally when the forward declaring header had a similar name to the symbol. Moreover, include-cleaner deliberately discourages forward-declarations, so not working in this case is also more aligned with rest of the components. Differential Revision: https://reviews.llvm.org/D159441 Added: Modified: clang-tools-extra/include-cleaner/lib/TypesInternal.h clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/include-cleaner/lib/TypesInternal.h b/clang-tools-extra/include-cleaner/lib/TypesInternal.h index ca9b961cbccdad..0bd2766c900abc 100644 --- a/clang-tools-extra/include-cleaner/lib/TypesInternal.h +++ b/clang-tools-extra/include-cleaner/lib/TypesInternal.h @@ -66,13 +66,13 @@ enum class Hints : uint8_t { /// Symbol is directly originating from this header, rather than being /// exported or included transitively. OriginHeader = 1 << 0, - /// Provides a generally-usable definition for the symbol. (a function decl, - /// or class definition and not a forward declaration of a template). - CompleteSymbol = 1 << 1, /// Header providing the symbol is explicitly marked as preferred, with an /// IWYU private pragma that points at this provider or header and symbol has /// ~the same name. - PreferredHeader = 1 << 2, + PreferredHeader = 1 << 1, + /// Provides a generally-usable definition for the symbol. (a function decl, + /// or class definition and not a forward declaration of a template). + CompleteSymbol = 1 << 2, /// Symbol is provided by a public file. Only absent in the cases where file /// is explicitly marked as such, non self-contained or IWYU private /// pragmas. diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp index b8f30c25e5c11b..89910e72beb463 100644 --- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp @@ -344,7 +344,7 @@ TEST_F(HeadersForSymbolTest, RankByName) { } TEST_F(HeadersForSymbolTest, Ranking) { - // Sorting is done over (canonical, public, complete, origin)-tuple. + // Sorting is done over (public, complete, canonical, origin)-tuple. Inputs.Code = R"cpp( #include "private.h" #include "public.h" @@ -363,11 +363,11 @@ TEST_F(HeadersForSymbolTest, Ranking) { )cpp"); Inputs.ExtraFiles["public_complete.h"] = guard("struct foo {};"); buildAST(); - EXPECT_THAT(headersForFoo(), ElementsAre(Header("\"canonical.h\""), - physicalHeader("public_complete.h"), - physicalHeader("public.h"), - physicalHeader("exporter.h"), - physicalHeader("private.h"))); + EXPECT_THAT(headersForFoo(), + ElementsAre(physicalHeader("public_complete.h"), + Header("\"canonical.h\""), physicalHeader("public.h"), + physicalHeader("exporter.h"), + physicalHeader("private.h"))); } TEST_F(HeadersForSymbolTest, PreferPublicOverComplete) { @@ -391,14 +391,11 @@ TEST_F(HeadersForSymbolTest, PreferNameMatch) { #include "public_complete.h" #include "test/foo.fwd.h" )cpp"; - Inputs.ExtraFiles["public_complete.h"] = guard(R"cpp( - struct foo {}; - )cpp"); + Inputs.ExtraFiles["public_complete.h"] = guard("struct foo {};"); Inputs.ExtraFiles["test/foo.fwd.h"] = guard("struct foo;"); buildAST(); - EXPECT_THAT(headersForFoo(), - ElementsAre(physicalHeader("test/foo.fwd.h"), - physicalHeader("public_complete.h"))); + EXPECT_THAT(headersForFoo(), ElementsAre(physicalHeader("public_complete.h"), + physicalHeader("test/foo.fwd.h"))); } TEST_F(HeadersForSymbolTest, MainFile) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
