Author: Kirill Bobyrev Date: 2022-03-31T17:59:52+02:00 New Revision: f43c4c5be29b4111bb953371b8ca83a4511fb1c1
URL: https://github.com/llvm/llvm-project/commit/f43c4c5be29b4111bb953371b8ca83a4511fb1c1 DIFF: https://github.com/llvm/llvm-project/commit/f43c4c5be29b4111bb953371b8ca83a4511fb1c1.diff LOG: Revert "[clangd] IncludeCleaner: Add support for IWYU pragma private" This reverts commit 4cb38bfe76b7ef157485338623c931d04d17b958. Awkwardly enough, this builds Windows buildbots: http://45.33.8.238/win/55402/step_9.txt It is yet unclear why this is happening but I will need more time to diagnose the issue. Added: Modified: clang-tools-extra/clangd/IncludeCleaner.cpp clang-tools-extra/clangd/IncludeCleaner.h clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index 14cc3409cae49..04dbf12410cf7 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -12,7 +12,6 @@ #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" @@ -24,7 +23,6 @@ #include "clang/Lex/Preprocessor.h" #include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/STLFunctionalExtras.h" -#include "llvm/ADT/StringSet.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Path.h" @@ -305,10 +303,9 @@ ReferencedLocations findReferencedLocations(ParsedAST &AST) { &AST.getTokens()); } -ReferencedFiles findReferencedFiles( - const ReferencedLocations &Locs, const SourceManager &SM, - llvm::function_ref<FileID(FileID)> HeaderResponsible, - llvm::function_ref<Optional<StringRef>(FileID)> UmbrellaHeader) { +ReferencedFiles +findReferencedFiles(const ReferencedLocations &Locs, const SourceManager &SM, + llvm::function_ref<FileID(FileID)> HeaderResponsible) { std::vector<SourceLocation> Sorted{Locs.User.begin(), Locs.User.end()}; llvm::sort(Sorted); // Group by FileID. ReferencedFilesBuilder Builder(SM); @@ -327,55 +324,33 @@ ReferencedFiles findReferencedFiles( // 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<FileID> UserFiles; - llvm::StringSet<> PublicHeaders; - for (FileID ID : Builder.Files) { + for (FileID ID : Builder.Files) UserFiles.insert(HeaderResponsible(ID)); - if (auto PublicHeader = UmbrellaHeader(ID)) { - PublicHeaders.insert(*PublicHeader); - } - } llvm::DenseSet<tooling::stdlib::Header> StdlibFiles; for (const auto &Symbol : Locs.Stdlib) for (const auto &Header : Symbol.headers()) StdlibFiles.insert(Header); - return {std::move(UserFiles), std::move(StdlibFiles), - std::move(PublicHeaders)}; + return {std::move(UserFiles), std::move(StdlibFiles)}; } 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); - }, - [&SM, &CanonIncludes](FileID ID) -> Optional<StringRef> { - const FileEntry *Entry = SM.getFileEntryForID(ID); - if (Entry) { - auto PublicHeader = CanonIncludes.mapHeader(Entry->getName()); - if (!PublicHeader.empty()) { - return PublicHeader; - } - } - return llvm::None; - }); + return findReferencedFiles(Locs, SM, [&SM, &Includes](FileID ID) { + return headerResponsible(ID, SM, Includes); + }); } std::vector<const Inclusion *> getUnused(ParsedAST &AST, - const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles, - const llvm::StringSet<> &ReferencedPublicHeaders) { + const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles) { trace::Span Tracer("IncludeCleaner::getUnused"); std::vector<const Inclusion *> Unused; for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) { if (!MFI.HeaderID) continue; - if (ReferencedPublicHeaders.contains(MFI.Written)) - continue; auto IncludeID = static_cast<IncludeStructure::HeaderID>(*MFI.HeaderID); bool Used = ReferencedFiles.contains(IncludeID); if (!Used && !mayConsiderUnused(MFI, AST)) { @@ -425,12 +400,11 @@ std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST) { const auto &SM = AST.getSourceManager(); auto Refs = findReferencedLocations(AST); - auto ReferencedFiles = - findReferencedFiles(Refs, AST.getIncludeStructure(), - AST.getCanonicalIncludes(), AST.getSourceManager()); + auto ReferencedFileIDs = findReferencedFiles(Refs, AST.getIncludeStructure(), + AST.getSourceManager()); auto ReferencedHeaders = - translateToHeaderIDs(ReferencedFiles, AST.getIncludeStructure(), SM); - return getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas); + translateToHeaderIDs(ReferencedFileIDs, AST.getIncludeStructure(), SM); + return getUnused(AST, ReferencedHeaders); } std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST, diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h index 4ce31baaa067a..183f84f2f3bfe 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.h +++ b/clang-tools-extra/clangd/IncludeCleaner.h @@ -13,6 +13,9 @@ /// 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 @@ -20,12 +23,10 @@ #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 <vector> namespace clang { @@ -57,11 +58,6 @@ ReferencedLocations findReferencedLocations(ParsedAST &AST); struct ReferencedFiles { llvm::DenseSet<FileID> User; llvm::DenseSet<tooling::stdlib::Header> Stdlib; - /// Files responsible for the symbols referenced in the main file and defined - /// in private headers (private headers have IWYU pragma: private, include - /// "public.h"). We store spelling of the public header (with quotes or angle - /// brackets) files here to avoid dealing with full filenames and visibility. - llvm::StringSet<> SpelledUmbrellas; }; /// Retrieves IDs of all files containing SourceLocations from \p Locs. @@ -70,16 +66,11 @@ struct ReferencedFiles { /// \p HeaderResponsible returns the public header that should be included given /// symbols from a file with the given FileID (example: public headers should be /// preferred to non self-contained and private headers). -/// \p UmbrellaHeader returns the public public header is responsible for -/// providing symbols from a file with the given FileID (example: MyType.h -/// should be included instead of MyType_impl.h). -ReferencedFiles findReferencedFiles( - const ReferencedLocations &Locs, const SourceManager &SM, - llvm::function_ref<FileID(FileID)> HeaderResponsible, - llvm::function_ref<Optional<StringRef>(FileID)> UmbrellaHeader); +ReferencedFiles +findReferencedFiles(const ReferencedLocations &Locs, const SourceManager &SM, + llvm::function_ref<FileID(FileID)> HeaderResponsible); ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs, const IncludeStructure &Includes, - const CanonicalIncludes &CanonIncludes, const SourceManager &SM); /// Maps FileIDs to the internal IncludeStructure representation (HeaderIDs). @@ -92,8 +83,7 @@ translateToHeaderIDs(const ReferencedFiles &Files, /// In unclear cases, headers are not marked as unused. std::vector<const Inclusion *> getUnused(ParsedAST &AST, - const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles, - const llvm::StringSet<> &ReferencedPublicHeaders); + const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles); std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST); diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp index ffecea5713215..b6bf7695f8873 100644 --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -9,7 +9,6 @@ #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" @@ -281,9 +280,8 @@ TEST(IncludeCleaner, Stdlib) { ReferencedLocations Locs = findReferencedLocations(AST); EXPECT_THAT(Locs.Stdlib, ElementsAreArray(WantSyms)); - ReferencedFiles Files = - findReferencedFiles(Locs, AST.getIncludeStructure(), - AST.getCanonicalIncludes(), AST.getSourceManager()); + ReferencedFiles Files = findReferencedFiles(Locs, AST.getIncludeStructure(), + AST.getSourceManager()); EXPECT_THAT(Files.Stdlib, ElementsAreArray(WantHeaders)); } } @@ -394,8 +392,8 @@ TEST(IncludeCleaner, VirtualBuffers) { auto &SM = AST.getSourceManager(); auto &Includes = AST.getIncludeStructure(); - auto ReferencedFiles = findReferencedFiles( - findReferencedLocations(AST), Includes, AST.getCanonicalIncludes(), SM); + auto ReferencedFiles = + findReferencedFiles(findReferencedLocations(AST), Includes, SM); llvm::StringSet<> ReferencedFileNames; for (FileID FID : ReferencedFiles.User) ReferencedFileNames.insert( @@ -414,9 +412,7 @@ TEST(IncludeCleaner, VirtualBuffers) { EXPECT_THAT(ReferencedHeaderNames, ElementsAre(testPath("macros.h"))); // Sanity check. - EXPECT_THAT( - getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas), - IsEmpty()); + EXPECT_THAT(getUnused(AST, ReferencedHeaders), IsEmpty()); } TEST(IncludeCleaner, DistinctUnguardedInclusions) { @@ -445,9 +441,9 @@ TEST(IncludeCleaner, DistinctUnguardedInclusions) { ParsedAST AST = TU.build(); - auto ReferencedFiles = findReferencedFiles( - findReferencedLocations(AST), AST.getIncludeStructure(), - AST.getCanonicalIncludes(), AST.getSourceManager()); + auto ReferencedFiles = + findReferencedFiles(findReferencedLocations(AST), + AST.getIncludeStructure(), AST.getSourceManager()); llvm::StringSet<> ReferencedFileNames; auto &SM = AST.getSourceManager(); for (FileID FID : ReferencedFiles.User) @@ -479,9 +475,9 @@ TEST(IncludeCleaner, NonSelfContainedHeaders) { ParsedAST AST = TU.build(); - auto ReferencedFiles = findReferencedFiles( - findReferencedLocations(AST), AST.getIncludeStructure(), - AST.getCanonicalIncludes(), AST.getSourceManager()); + auto ReferencedFiles = + findReferencedFiles(findReferencedLocations(AST), + AST.getIncludeStructure(), AST.getSourceManager()); llvm::StringSet<> ReferencedFileNames; auto &SM = AST.getSourceManager(); for (FileID FID : ReferencedFiles.User) @@ -497,31 +493,15 @@ TEST(IncludeCleaner, IWYUPragmas) { 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.getCanonicalIncludes(), AST.getSourceManager()); - EXPECT_EQ(ReferencedFiles.SpelledUmbrellas.size(), 1u); - EXPECT_EQ(ReferencedFiles.SpelledUmbrellas.begin()->getKey(), "\"public.h\""); - EXPECT_EQ(ReferencedFiles.User.size(), 2u); - EXPECT_TRUE( - ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID())); - EXPECT_TRUE( - ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID())); + auto ReferencedFiles = + findReferencedFiles(findReferencedLocations(AST), + AST.getIncludeStructure(), AST.getSourceManager()); + EXPECT_TRUE(ReferencedFiles.User.empty()); EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); - auto Unused = computeUnusedIncludes(AST); - EXPECT_THAT(Unused, IsEmpty()); } } // namespace _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
