Skip to content

Commit 73b2c86

Browse files
committed
[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
1 parent b93d2d3 commit 73b2c86

File tree

2 files changed

+13
-16
lines changed

2 files changed

+13
-16
lines changed

clang-tools-extra/include-cleaner/lib/TypesInternal.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,13 @@ enum class Hints : uint8_t {
6666
/// Symbol is directly originating from this header, rather than being
6767
/// exported or included transitively.
6868
OriginHeader = 1 << 0,
69-
/// Provides a generally-usable definition for the symbol. (a function decl,
70-
/// or class definition and not a forward declaration of a template).
71-
CompleteSymbol = 1 << 1,
7269
/// Header providing the symbol is explicitly marked as preferred, with an
7370
/// IWYU private pragma that points at this provider or header and symbol has
7471
/// ~the same name.
75-
PreferredHeader = 1 << 2,
72+
PreferredHeader = 1 << 1,
73+
/// Provides a generally-usable definition for the symbol. (a function decl,
74+
/// or class definition and not a forward declaration of a template).
75+
CompleteSymbol = 1 << 2,
7676
/// Symbol is provided by a public file. Only absent in the cases where file
7777
/// is explicitly marked as such, non self-contained or IWYU private
7878
/// pragmas.

clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ TEST_F(HeadersForSymbolTest, RankByName) {
344344
}
345345

346346
TEST_F(HeadersForSymbolTest, Ranking) {
347-
// Sorting is done over (canonical, public, complete, origin)-tuple.
347+
// Sorting is done over (public, complete, canonical, origin)-tuple.
348348
Inputs.Code = R"cpp(
349349
#include "private.h"
350350
#include "public.h"
@@ -363,11 +363,11 @@ TEST_F(HeadersForSymbolTest, Ranking) {
363363
)cpp");
364364
Inputs.ExtraFiles["public_complete.h"] = guard("struct foo {};");
365365
buildAST();
366-
EXPECT_THAT(headersForFoo(), ElementsAre(Header("\"canonical.h\""),
367-
physicalHeader("public_complete.h"),
368-
physicalHeader("public.h"),
369-
physicalHeader("exporter.h"),
370-
physicalHeader("private.h")));
366+
EXPECT_THAT(headersForFoo(),
367+
ElementsAre(physicalHeader("public_complete.h"),
368+
Header("\"canonical.h\""), physicalHeader("public.h"),
369+
physicalHeader("exporter.h"),
370+
physicalHeader("private.h")));
371371
}
372372

373373
TEST_F(HeadersForSymbolTest, PreferPublicOverComplete) {
@@ -391,14 +391,11 @@ TEST_F(HeadersForSymbolTest, PreferNameMatch) {
391391
#include "public_complete.h"
392392
#include "test/foo.fwd.h"
393393
)cpp";
394-
Inputs.ExtraFiles["public_complete.h"] = guard(R"cpp(
395-
struct foo {};
396-
)cpp");
394+
Inputs.ExtraFiles["public_complete.h"] = guard("struct foo {};");
397395
Inputs.ExtraFiles["test/foo.fwd.h"] = guard("struct foo;");
398396
buildAST();
399-
EXPECT_THAT(headersForFoo(),
400-
ElementsAre(physicalHeader("test/foo.fwd.h"),
401-
physicalHeader("public_complete.h")));
397+
EXPECT_THAT(headersForFoo(), ElementsAre(physicalHeader("public_complete.h"),
398+
physicalHeader("test/foo.fwd.h")));
402399
}
403400

404401
TEST_F(HeadersForSymbolTest, MainFile) {

0 commit comments

Comments
 (0)