[clang-tools-extra] 0a315be - [include-cleaner] Dont boost private headers beyond public ones

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-08-09T09:36:48+02:00
New Revision: 0a315be2a46f35bb990dd6c8418a189daec15350

URL: https://github.com/llvm/llvm-project/commit/0a315be2a46f35bb990dd6c8418a189daec15350
DIFF: https://github.com/llvm/llvm-project/commit/0a315be2a46f35bb990dd6c8418a189daec15350.diff

LOG: [include-cleaner] Dont boost private headers beyond public ones

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

Added: 
    

Modified: 
    clang-tools-extra/include-cleaner/lib/TypesInternal.h
    clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
    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 e9f8689e8647e0..ca9b961cbccdad 100644
--- a/clang-tools-extra/include-cleaner/lib/TypesInternal.h
+++ b/clang-tools-extra/include-cleaner/lib/TypesInternal.h
@@ -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.

diff  --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index 3ad632a18c563a..d5f79f54c81db3 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -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

diff  --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
index 9f9ac11a93eb85..cbe1f67d9bf7e5 100644
--- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -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;