[clang-tools-extra] 5933d26 - [include-cleaner] Add a signal to down-rank exporting headers
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 5 06:41:25 PDT 2023
Author: Kadir Cetinkaya
Date: 2023-07-05T15:37:17+02:00
New Revision: 5933d265b72a8e9aade5edd68998a00dc4fbb359
URL: https://github.com/llvm/llvm-project/commit/5933d265b72a8e9aade5edd68998a00dc4fbb359
DIFF: https://github.com/llvm/llvm-project/commit/5933d265b72a8e9aade5edd68998a00dc4fbb359.diff
LOG: [include-cleaner] Add a signal to down-rank exporting headers
Currently exporter can have same relevance signals as the origin header
when name match signals don't trigger.
This patch introduces a tie braker signal to boost origin headers in
such cases, this is deliberately introduced with lower significance than
public-ness to make sure we still prefer a public-exporter instead of a
private-origin header.
Differential Revision: https://reviews.llvm.org/D154349
Added:
Modified:
clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
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/FindHeaders.cpp b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
index 1eff19cb44445b..a1d9d3b5fb2154 100644
--- a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -96,7 +96,7 @@ hintedHeadersForStdHeaders(llvm::ArrayRef<tooling::stdlib::Header> Headers,
const SourceManager &SM, const PragmaIncludes *PI) {
llvm::SmallVector<Hinted<Header>> Results;
for (const auto &H : Headers) {
- Results.emplace_back(H, Hints::PublicHeader);
+ Results.emplace_back(H, Hints::PublicHeader | Hints::OriginHeader);
if (!PI)
continue;
for (const auto *Export : PI->getExporters(H, SM.getFileManager()))
@@ -186,10 +186,12 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const SymbolLocation &Loc,
if (!FE)
return {};
if (!PI)
- return {{FE, Hints::PublicHeader}};
+ return {{FE, Hints::PublicHeader | Hints::OriginHeader}};
+ bool IsOrigin = true;
while (FE) {
- Hints CurrentHints = isPublicHeader(FE, *PI);
- Results.emplace_back(FE, CurrentHints);
+ Results.emplace_back(FE,
+ isPublicHeader(FE, *PI) |
+ (IsOrigin ? Hints::OriginHeader : Hints::None));
// FIXME: compute transitive exporter headers.
for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
Results.emplace_back(Export, isPublicHeader(Export, *PI));
@@ -205,6 +207,7 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const SymbolLocation &Loc,
// Walkup the include stack for non self-contained headers.
FID = SM.getDecomposedIncludedLoc(FID).first;
FE = SM.getFileEntryForID(FID);
+ IsOrigin = false;
}
return Results;
}
diff --git a/clang-tools-extra/include-cleaner/lib/TypesInternal.h b/clang-tools-extra/include-cleaner/lib/TypesInternal.h
index 09a3933577a94f..e9f8689e8647e0 100644
--- a/clang-tools-extra/include-cleaner/lib/TypesInternal.h
+++ b/clang-tools-extra/include-cleaner/lib/TypesInternal.h
@@ -63,17 +63,20 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolLocation &);
/// Hints are sorted in ascending order of relevance.
enum class Hints : uint8_t {
None = 0x00,
+ /// Symbol is directly originating from this header, rather than being
+ /// exported or included transitively.
+ OriginHeader = 1 << 0,
/// Provides a generally-usable definition for the symbol. (a function decl,
/// or class definition and not a forward declaration of a template).
- CompleteSymbol = 1 << 0,
+ 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 << 1,
+ 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 << 2,
+ PreferredHeader = 1 << 3,
LLVM_MARK_AS_BITMASK_ENUM(PreferredHeader),
};
LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
@@ -86,6 +89,11 @@ template <typename T> struct Hinted : public T {
bool operator<(const Hinted<T> &Other) const {
return static_cast<int>(Hint) < static_cast<int>(Other.Hint);
}
+
+ friend llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+ const Hinted<T> &H) {
+ return OS << static_cast<int>(H.Hint) << " - " << static_cast<T>(H);
+ }
};
} // namespace clang::include_cleaner
diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index 5b5f77b5fdea80..b6521d56bcff44 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -422,8 +422,12 @@ TEST(WalkUsed, FilterRefsNotSpelledInMainFile) {
}
}
+struct Tag {
+ friend llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Tag &T) {
+ return OS << "Anon Tag";
+ }
+};
TEST(Hints, Ordering) {
- struct Tag {};
auto Hinted = [](Hints Hints) {
return clang::include_cleaner::Hinted<Tag>({}, Hints);
};
diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
index 964f4c6c9190de..9f9ac11a93eb85 100644
--- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -8,17 +8,21 @@
#include "AnalysisInternal.h"
#include "TypesInternal.h"
+#include "clang-include-cleaner/Analysis.h"
#include "clang-include-cleaner/Record.h"
#include "clang-include-cleaner/Types.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/FileEntry.h"
#include "clang/Basic/FileManager.h"
+#include "clang/Basic/LLVM.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/Testing/TestAST.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <cassert>
#include <memory>
namespace clang::include_cleaner {
@@ -253,11 +257,11 @@ TEST_F(FindHeadersTest, PublicHeaderHint) {
EXPECT_THAT(
findHeaders("private.inc"),
UnorderedElementsAre(
- HintedHeader(physicalHeader("private.inc"), Hints::None),
+ HintedHeader(physicalHeader("private.inc"), Hints::OriginHeader),
HintedHeader(physicalHeader("public.h"), Hints::PublicHeader)));
EXPECT_THAT(findHeaders("private.h"),
- UnorderedElementsAre(
- HintedHeader(physicalHeader("private.h"), Hints::None)));
+ UnorderedElementsAre(HintedHeader(physicalHeader("private.h"),
+ Hints::OriginHeader)));
}
TEST_F(FindHeadersTest, PreferredHeaderHint) {
@@ -269,11 +273,12 @@ TEST_F(FindHeadersTest, PreferredHeaderHint) {
)cpp");
buildAST();
// Headers explicitly marked should've preferred signal.
- EXPECT_THAT(findHeaders("private.h"),
- UnorderedElementsAre(
- HintedHeader(physicalHeader("private.h"), Hints::None),
- HintedHeader(Header("\"public.h\""),
- Hints::PreferredHeader | Hints::PublicHeader)));
+ EXPECT_THAT(
+ findHeaders("private.h"),
+ UnorderedElementsAre(
+ HintedHeader(physicalHeader("private.h"), Hints::OriginHeader),
+ HintedHeader(Header("\"public.h\""),
+ Hints::PreferredHeader | Hints::PublicHeader)));
}
class HeadersForSymbolTest : public FindHeadersTest {
@@ -339,11 +344,12 @@ TEST_F(HeadersForSymbolTest, RankByName) {
}
TEST_F(HeadersForSymbolTest, Ranking) {
- // Sorting is done over (canonical, public, complete) triplet.
+ // Sorting is done over (canonical, public, complete, origin)-tuple.
Inputs.Code = R"cpp(
#include "private.h"
#include "public.h"
#include "public_complete.h"
+ #include "exporter.h"
)cpp";
Inputs.ExtraFiles["public.h"] = guard(R"cpp(
struct foo;
@@ -352,11 +358,15 @@ TEST_F(HeadersForSymbolTest, Ranking) {
// IWYU pragma: private, include "canonical.h"
struct foo;
)cpp");
+ Inputs.ExtraFiles["exporter.h"] = guard(R"cpp(
+ #include "private.h" // IWYU pragma: export
+ )cpp");
Inputs.ExtraFiles["public_complete.h"] = guard("struct foo {};");
buildAST();
EXPECT_THAT(headersForFoo(), ElementsAre(Header("\"canonical.h\""),
physicalHeader("public_complete.h"),
physicalHeader("public.h"),
+ physicalHeader("exporter.h"),
physicalHeader("private.h")));
}
@@ -424,6 +434,24 @@ TEST_F(HeadersForSymbolTest, PreferExporterOfPrivate) {
physicalHeader("private.h")));
}
+TEST_F(HeadersForSymbolTest, ExporterIsDownRanked) {
+ Inputs.Code = R"cpp(
+ #include "exporter.h"
+ #include "zoo.h"
+ )cpp";
+ // Deliberately named as zoo to make sure it doesn't get name-match boost and
+ // also gets lexicographically bigger order than "exporter".
+ Inputs.ExtraFiles["zoo.h"] = guard(R"cpp(
+ struct foo {};
+ )cpp");
+ Inputs.ExtraFiles["exporter.h"] = guard(R"cpp(
+ #include "zoo.h" // IWYU pragma: export
+ )cpp");
+ buildAST();
+ EXPECT_THAT(headersForFoo(), ElementsAre(physicalHeader("zoo.h"),
+ physicalHeader("exporter.h")));
+}
+
TEST_F(HeadersForSymbolTest, PreferPublicOverNameMatchOnPrivate) {
Inputs.Code = R"cpp(
#include "foo.h"
@@ -496,6 +524,5 @@ TEST_F(HeadersForSymbolTest, StandardHeaders) {
tooling::stdlib::Header::named("<assert.h>")));
}
-
} // namespace
} // namespace clang::include_cleaner
More information about the cfe-commits
mailing list