[clang-tools-extra] 504aa8a - [include-cleaner] Better support ambiguous std symbols
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 14 01:18:47 PST 2023
Author: Haojian Wu
Date: 2023-02-14T10:18:38+01:00
New Revision: 504aa8ae941ea8e09b89a91cfbbbeb8a7b8fdc24
URL: https://github.com/llvm/llvm-project/commit/504aa8ae941ea8e09b89a91cfbbbeb8a7b8fdc24
DIFF: https://github.com/llvm/llvm-project/commit/504aa8ae941ea8e09b89a91cfbbbeb8a7b8fdc24.diff
LOG: [include-cleaner] Better support ambiguous std symbols
By special-casing them at the moment. The tooling stdlib lib doesn't
support these symbols (most important one is std::move).
Differential Revision: https://reviews.llvm.org/D143906
Added:
Modified:
clang-tools-extra/include-cleaner/lib/FindHeaders.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 030ce61701494..ae2a3a858a38c 100644
--- a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -82,16 +82,68 @@ llvm::StringRef symbolName(const Symbol &S) {
llvm_unreachable("unhandled Symbol kind!");
}
+Hints isPublicHeader(const FileEntry *FE, const PragmaIncludes &PI) {
+ if (PI.isPrivate(FE) || !PI.isSelfContained(FE))
+ return Hints::None;
+ return Hints::PublicHeader;
+}
+
+llvm::SmallVector<Hinted<Header>>
+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);
+ if (!PI)
+ continue;
+ for (const auto *Export : PI->getExporters(H, SM.getFileManager()))
+ Results.emplace_back(Header(Export), isPublicHeader(Export, *PI));
+ }
+ // StandardLibrary returns headers in preference order, so only mark the
+ // first.
+ if (!Results.empty())
+ Results.front().Hint |= Hints::PreferredHeader;
+ return Results;
+}
+
+// Special-case the ambiguous standard library symbols (e.g. std::move) which
+// are not supported by the tooling stdlib lib.
+llvm::SmallVector<Hinted<Header>>
+headersForSpecialSymbol(const Symbol &S, const SourceManager &SM,
+ const PragmaIncludes *PI) {
+ if (S.kind() != Symbol::Declaration || !S.declaration().isInStdNamespace())
+ return {};
+
+ const auto *FD = S.declaration().getAsFunction();
+ if (!FD)
+ return {};
+
+ llvm::StringRef FName = FD->getName();
+ llvm::SmallVector<tooling::stdlib::Header> Headers;
+ if (FName == "move") {
+ if (FD->getNumParams() == 1)
+ // move(T&& t)
+ Headers.push_back(*tooling::stdlib::Header::named("<utility>"));
+ if (FD->getNumParams() == 3)
+ // move(InputIt first, InputIt last, OutputIt dest);
+ Headers.push_back(*tooling::stdlib::Header::named("<algorithm>"));
+ } else if (FName == "remove") {
+ if (FD->getNumParams() == 1)
+ // remove(const char*);
+ Headers.push_back(*tooling::stdlib::Header::named("<cstdio>"));
+ if (FD->getNumParams() == 3)
+ // remove(ForwardIt first, ForwardIt last, const T& value);
+ Headers.push_back(*tooling::stdlib::Header::named("<algorithm>"));
+ }
+ return applyHints(hintedHeadersForStdHeaders(Headers, SM, PI),
+ Hints::CompleteSymbol);
+}
+
} // namespace
llvm::SmallVector<Hinted<Header>> findHeaders(const SymbolLocation &Loc,
const SourceManager &SM,
const PragmaIncludes *PI) {
- auto IsPublicHeader = [&PI](const FileEntry *FE) {
- return (PI->isPrivate(FE) || !PI->isSelfContained(FE))
- ? Hints::None
- : Hints::PublicHeader;
- };
llvm::SmallVector<Hinted<Header>> Results;
switch (Loc.kind()) {
case SymbolLocation::Physical: {
@@ -102,11 +154,11 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const SymbolLocation &Loc,
if (!PI)
return {{FE, Hints::PublicHeader}};
while (FE) {
- Hints CurrentHints = IsPublicHeader(FE);
+ Hints CurrentHints = isPublicHeader(FE, *PI);
Results.emplace_back(FE, CurrentHints);
// FIXME: compute transitive exporter headers.
for (const auto *Export : PI->getExporters(FE, SM.getFileManager()))
- Results.emplace_back(Export, IsPublicHeader(Export));
+ Results.emplace_back(Export, isPublicHeader(Export, *PI));
if (auto Verbatim = PI->getPublic(FE); !Verbatim.empty()) {
Results.emplace_back(Verbatim,
@@ -123,16 +175,7 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const SymbolLocation &Loc,
return Results;
}
case SymbolLocation::Standard: {
- for (const auto &H : Loc.standard().headers()) {
- Results.emplace_back(H, Hints::PublicHeader);
- for (const auto *Export : PI->getExporters(H, SM.getFileManager()))
- Results.emplace_back(Header(Export), IsPublicHeader(Export));
- }
- // StandardLibrary returns headers in preference order, so only mark the
- // first.
- if (!Results.empty())
- Results.front().Hint |= Hints::PreferredHeader;
- return Results;
+ return hintedHeadersForStdHeaders(Loc.standard().headers(), SM, PI);
}
}
llvm_unreachable("unhandled SymbolLocation kind!");
@@ -144,9 +187,11 @@ llvm::SmallVector<Header> headersForSymbol(const Symbol &S,
// Get headers for all the locations providing Symbol. Same header can be
// reached through
diff erent traversals, deduplicate those into a single
// Header by merging their hints.
- llvm::SmallVector<Hinted<Header>> Headers;
- for (auto &Loc : locateSymbol(S))
- Headers.append(applyHints(findHeaders(Loc, SM, PI), Loc.Hint));
+ llvm::SmallVector<Hinted<Header>> Headers =
+ headersForSpecialSymbol(S, SM, PI);
+ if (Headers.empty())
+ for (auto &Loc : locateSymbol(S))
+ Headers.append(applyHints(findHeaders(Loc, SM, PI), Loc.Hint));
// If two Headers probably refer to the same file (e.g. Verbatim(foo.h) and
// Physical(/path/to/foo.h), we won't deduplicate them or merge their hints
llvm::stable_sort(
diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
index f414ff314418d..2efdc7ebbb784 100644
--- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -278,25 +278,31 @@ TEST_F(FindHeadersTest, PreferredHeaderHint) {
class HeadersForSymbolTest : public FindHeadersTest {
protected:
- llvm::SmallVector<Header> headersForFoo() {
+ llvm::SmallVector<Header> headersFor(llvm::StringRef Name) {
struct Visitor : public RecursiveASTVisitor<Visitor> {
const NamedDecl *Out = nullptr;
+ llvm::StringRef Name;
+ Visitor(llvm::StringRef Name) : Name(Name) {}
bool VisitNamedDecl(const NamedDecl *ND) {
- if (ND->getName() == "foo") {
+ if (auto *TD = ND->getDescribedTemplate())
+ ND = TD;
+
+ if (ND->getName() == Name) {
EXPECT_TRUE(Out == nullptr || Out == ND->getCanonicalDecl())
- << "Found multiple matches for foo.";
+ << "Found multiple matches for " << Name << ".";
Out = cast<NamedDecl>(ND->getCanonicalDecl());
}
return true;
}
};
- Visitor V;
+ Visitor V(Name);
V.TraverseDecl(AST->context().getTranslationUnitDecl());
if (!V.Out)
- ADD_FAILURE() << "Couldn't find any decls named foo.";
+ ADD_FAILURE() << "Couldn't find any decls named " << Name << ".";
assert(V.Out);
return headersForSymbol(*V.Out, AST->sourceManager(), &PI);
}
+ llvm::SmallVector<Header> headersForFoo() { return headersFor("foo"); }
};
TEST_F(HeadersForSymbolTest, Deduplicates) {
@@ -430,5 +436,55 @@ TEST_F(HeadersForSymbolTest, PreferPublicOverNameMatchOnPrivate) {
EXPECT_THAT(headersForFoo(), ElementsAre(Header(StringRef("\"public.h\"")),
physicalHeader("foo.h")));
}
+
+TEST_F(HeadersForSymbolTest, AmbiguousStdSymbols) {
+ struct {
+ llvm::StringRef Code;
+ llvm::StringRef Name;
+
+ llvm::StringRef ExpectedHeader;
+ } TestCases[] = {
+ {
+ R"cpp(
+ namespace std {
+ template <typename InputIt, typename OutputIt>
+ constexpr OutputIt move(InputIt first, InputIt last, OutputIt dest);
+ })cpp",
+ "move",
+ "<algorithm>",
+ },
+ {
+ R"cpp(
+ namespace std {
+ template<typename T> constexpr T move(T&& t) noexcept;
+ })cpp",
+ "move",
+ "<utility>",
+ },
+ {
+ R"cpp(
+ namespace std {
+ template<class ForwardIt, class T>
+ ForwardIt remove(ForwardIt first, ForwardIt last, const T& value);
+ })cpp",
+ "remove",
+ "<algorithm>",
+ },
+ {
+ "namespace std { int remove(const char*); }",
+ "remove",
+ "<cstdio>",
+ },
+ };
+
+ for (const auto &T : TestCases) {
+ Inputs.Code = T.Code;
+ buildAST();
+ EXPECT_THAT(headersFor(T.Name),
+ UnorderedElementsAre(
+ Header(*tooling::stdlib::Header::named(T.ExpectedHeader))));
+ }
+}
+
} // namespace
} // namespace clang::include_cleaner
More information about the cfe-commits
mailing list