[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
This commit is contained in:
Haojian Wu 2023-02-13 14:46:07 +01:00
parent 1ba93c3c30
commit 504aa8ae94
2 changed files with 126 additions and 25 deletions

View File

@ -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 different 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(

View File

@ -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