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

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 8 08:02:22 PDT 2023


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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D157400.548230.patch
Type: text/x-patch
Size: 3090 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230808/334e9af6/attachment.bin>


More information about the cfe-commits mailing list