diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h --- a/clang-tools-extra/clangd/IncludeCleaner.h +++ b/clang-tools-extra/clangd/IncludeCleaner.h @@ -13,9 +13,6 @@ /// to provide useful warnings in most popular scenarios but not 1:1 exact /// feature compatibility. /// -/// FIXME(kirillbobyrev): Add support for IWYU pragmas. -/// FIXME(kirillbobyrev): Add support for standard library headers. -/// //===----------------------------------------------------------------------===// #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INCLUDECLEANER_H @@ -23,10 +20,12 @@ #include "Headers.h" #include "ParsedAST.h" +#include "index/CanonicalIncludes.h" #include "clang/Basic/SourceLocation.h" #include "clang/Tooling/Inclusions/StandardLibrary.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/STLFunctionalExtras.h" +#include "llvm/ADT/StringSet.h" #include namespace clang { @@ -59,6 +58,7 @@ struct ReferencedFiles { llvm::DenseSet User; llvm::DenseSet Stdlib; + llvm::StringSet<> PublicHeaders; }; /// Retrieves IDs of all files containing SourceLocations from \p Locs. @@ -69,9 +69,12 @@ /// preferred to non self-contained and private headers). ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs, const SourceManager &SM, - llvm::function_ref HeaderResponsible); + llvm::function_ref &, + llvm::StringSet<> &)> + AddFile); ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs, const IncludeStructure &Includes, + const CanonicalIncludes &CanonIncludes, const SourceManager &SM); /// Maps FileIDs to the internal IncludeStructure representation (HeaderIDs). diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -12,6 +12,7 @@ #include "ParsedAST.h" #include "Protocol.h" #include "SourceCode.h" +#include "index/CanonicalIncludes.h" #include "support/Logger.h" #include "support/Trace.h" #include "clang/AST/ASTContext.h" @@ -307,7 +308,9 @@ ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs, const SourceManager &SM, - llvm::function_ref HeaderResponsible) { + llvm::function_ref &, + llvm::StringSet<> &)> + AddFile) { std::vector Sorted{Locs.User.begin(), Locs.User.end()}; llvm::sort(Sorted); // Group by FileID. ReferencedFilesBuilder Builder(SM); @@ -326,23 +329,40 @@ // non-self-contained FileIDs as used. Perform this on FileIDs rather than // HeaderIDs, as each inclusion of a non-self-contained file is distinct. llvm::DenseSet UserFiles; + llvm::StringSet<> PublicHeaders; for (FileID ID : Builder.Files) - UserFiles.insert(HeaderResponsible(ID)); + AddFile(ID, UserFiles, PublicHeaders); llvm::DenseSet StdlibFiles; for (const auto &Symbol : Locs.Stdlib) for (const auto &Header : Symbol.headers()) StdlibFiles.insert(Header); - return {std::move(UserFiles), std::move(StdlibFiles)}; + return {std::move(UserFiles), std::move(StdlibFiles), + std::move(PublicHeaders)}; } ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs, const IncludeStructure &Includes, + const CanonicalIncludes &CanonIncludes, const SourceManager &SM) { - return findReferencedFiles(Locs, SM, [&SM, &Includes](FileID ID) { - return headerResponsible(ID, SM, Includes); - }); + return findReferencedFiles( + Locs, SM, + [&SM, &Includes, &CanonIncludes](FileID ID, + llvm::DenseSet &UserFiles, + llvm::StringSet<> &PublicHeaders) { + auto FID = headerResponsible(ID, SM, Includes); + const FileEntry *Entry = SM.getFileEntryForID(FID); + if (Entry) { + auto PublicHeader = CanonIncludes.mapHeader(Entry->getName()); + if (!PublicHeader.empty()) { + PublicHeaders.insert(PublicHeader); + return; + } + } else { + UserFiles.insert(FID); + } + }); } std::vector @@ -402,8 +422,9 @@ const auto &SM = AST.getSourceManager(); auto Refs = findReferencedLocations(AST); - auto ReferencedFileIDs = findReferencedFiles(Refs, AST.getIncludeStructure(), - AST.getSourceManager()); + auto ReferencedFileIDs = + findReferencedFiles(Refs, AST.getIncludeStructure(), + AST.getCanonicalIncludes(), AST.getSourceManager()); auto ReferencedHeaders = translateToHeaderIDs(ReferencedFileIDs, AST.getIncludeStructure(), SM); return getUnused(AST, ReferencedHeaders); diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -9,6 +9,7 @@ #include "Annotations.h" #include "IncludeCleaner.h" #include "SourceCode.h" +#include "TestFS.h" #include "TestTU.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/Testing/Support/SupportHelpers.h" @@ -266,8 +267,9 @@ ReferencedLocations Locs = findReferencedLocations(AST); EXPECT_THAT(Locs.Stdlib, ElementsAreArray(WantSyms)); - ReferencedFiles Files = findReferencedFiles(Locs, AST.getIncludeStructure(), - AST.getSourceManager()); + ReferencedFiles Files = + findReferencedFiles(Locs, AST.getIncludeStructure(), + AST.getCanonicalIncludes(), AST.getSourceManager()); EXPECT_THAT(Files.Stdlib, ElementsAreArray(WantHeaders)); } } @@ -378,8 +380,8 @@ auto &SM = AST.getSourceManager(); auto &Includes = AST.getIncludeStructure(); - auto ReferencedFiles = - findReferencedFiles(findReferencedLocations(AST), Includes, SM); + auto ReferencedFiles = findReferencedFiles( + findReferencedLocations(AST), Includes, AST.getCanonicalIncludes(), SM); llvm::StringSet<> ReferencedFileNames; for (FileID FID : ReferencedFiles.User) ReferencedFileNames.insert( @@ -427,9 +429,9 @@ ParsedAST AST = TU.build(); - auto ReferencedFiles = - findReferencedFiles(findReferencedLocations(AST), - AST.getIncludeStructure(), AST.getSourceManager()); + auto ReferencedFiles = findReferencedFiles( + findReferencedLocations(AST), AST.getIncludeStructure(), + AST.getCanonicalIncludes(), AST.getSourceManager()); llvm::StringSet<> ReferencedFileNames; auto &SM = AST.getSourceManager(); for (FileID FID : ReferencedFiles.User) @@ -461,9 +463,9 @@ ParsedAST AST = TU.build(); - auto ReferencedFiles = - findReferencedFiles(findReferencedLocations(AST), - AST.getIncludeStructure(), AST.getSourceManager()); + auto ReferencedFiles = findReferencedFiles( + findReferencedLocations(AST), AST.getIncludeStructure(), + AST.getCanonicalIncludes(), AST.getSourceManager()); llvm::StringSet<> ReferencedFileNames; auto &SM = AST.getSourceManager(); for (FileID FID : ReferencedFiles.User) @@ -479,13 +481,23 @@ TestTU TU; TU.Code = R"cpp( #include "behind_keep.h" // IWYU pragma: keep + #include "public.h" + + void bar() { foo(); } )cpp"; TU.AdditionalFiles["behind_keep.h"] = guard(""); + TU.AdditionalFiles["public.h"] = guard("#include \"private.h\""); + TU.AdditionalFiles["private.h"] = guard(R"cpp( + // IWYU pragma: private, include "public.h" + void foo() {} + )cpp"); ParsedAST AST = TU.build(); - auto ReferencedFiles = - findReferencedFiles(findReferencedLocations(AST), - AST.getIncludeStructure(), AST.getSourceManager()); + auto ReferencedFiles = findReferencedFiles( + findReferencedLocations(AST), AST.getIncludeStructure(), + AST.getCanonicalIncludes(), AST.getSourceManager()); + EXPECT_EQ(ReferencedFiles.PublicHeaders.size(), 1u); + EXPECT_EQ(ReferencedFiles.PublicHeaders.begin()->getKey(), "\"public.h\""); EXPECT_TRUE(ReferencedFiles.User.empty()); EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); }