Skip to content

Commit

Permalink
[include-cleaner] Dont boost private headers beyond public ones
Browse files Browse the repository at this point in the history
Private headers should be the last resort, even if they match the name
of a symbol. It's pretty common in umrella headers to have internal file names
that match the symbol (e.g. Eigen::Matrix, declared in private header Matrix.h,
and exposed in umbrella header Eigen/Core).

Differential Revision: https://reviews.llvm.org/D157400
  • Loading branch information
kadircet committed Aug 9, 2023
1 parent 27d6161 commit 0a315be
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 9 deletions.
12 changes: 6 additions & 6 deletions clang-tools-extra/include-cleaner/lib/TypesInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,15 @@ enum class Hints : uint8_t {
/// 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,
/// 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.
PublicHeader = 1 << 2,
/// 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 << 3,
LLVM_MARK_AS_BITMASK_ENUM(PreferredHeader),
PreferredHeader = 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.
PublicHeader = 1 << 3,
LLVM_MARK_AS_BITMASK_ENUM(PublicHeader),
};
LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
/// A wrapper to augment values with hints.
Expand Down
6 changes: 3 additions & 3 deletions clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,9 @@ TEST(Hints, Ordering) {
};
EXPECT_LT(Hinted(Hints::None), Hinted(Hints::CompleteSymbol));
EXPECT_LT(Hinted(Hints::CompleteSymbol), Hinted(Hints::PublicHeader));
EXPECT_LT(Hinted(Hints::PublicHeader), Hinted(Hints::PreferredHeader));
EXPECT_LT(Hinted(Hints::CompleteSymbol | Hints::PublicHeader),
Hinted(Hints::PreferredHeader));
EXPECT_LT(Hinted(Hints::PreferredHeader), Hinted(Hints::PublicHeader));
EXPECT_LT(Hinted(Hints::CompleteSymbol | Hints::PreferredHeader),
Hinted(Hints::PublicHeader));
}

// Test ast traversal & redecl selection end-to-end for templates, as explicit
Expand Down
16 changes: 16 additions & 0 deletions clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,22 @@ TEST_F(HeadersForSymbolTest, PreferPublicOverNameMatchOnPrivate) {
physicalHeader("foo.h")));
}

TEST_F(HeadersForSymbolTest, PublicOverPrivateWithoutUmbrella) {
Inputs.Code = R"cpp(
#include "bar.h"
#include "foo.h"
)cpp";
Inputs.ExtraFiles["bar.h"] =
guard(R"cpp(#include "foo.h" // IWYU pragma: export)cpp");
Inputs.ExtraFiles["foo.h"] = guard(R"cpp(
// IWYU pragma: private
struct foo {};
)cpp");
buildAST();
EXPECT_THAT(headersForFoo(),
ElementsAre(physicalHeader("bar.h"), physicalHeader("foo.h")));
}

TEST_F(HeadersForSymbolTest, AmbiguousStdSymbols) {
struct {
llvm::StringRef Code;
Expand Down

0 comments on commit 0a315be

Please sign in to comment.