kadircet created this revision.
kadircet added a reviewer: VitaNuo.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
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).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D157400
Files:
clang-tools-extra/include-cleaner/lib/TypesInternal.h
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -465,6 +465,22 @@
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;
Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -441,9 +441,9 @@
};
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
Index: clang-tools-extra/include-cleaner/lib/TypesInternal.h
===================================================================
--- clang-tools-extra/include-cleaner/lib/TypesInternal.h
+++ clang-tools-extra/include-cleaner/lib/TypesInternal.h
@@ -69,15 +69,15 @@
/// 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.
Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -465,6 +465,22 @@
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;
Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -441,9 +441,9 @@
};
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
Index: clang-tools-extra/include-cleaner/lib/TypesInternal.h
===================================================================
--- clang-tools-extra/include-cleaner/lib/TypesInternal.h
+++ clang-tools-extra/include-cleaner/lib/TypesInternal.h
@@ -69,15 +69,15 @@
/// 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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
