diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h --- a/clang-tools-extra/clangd/Headers.h +++ b/clang-tools-extra/clangd/Headers.h @@ -73,7 +73,6 @@ int HashLine = 0; // Line number containing the directive, 0-indexed. SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User; std::optional HeaderID; - bool BehindPragmaKeep = false; // Has IWYU pragma: keep right after. }; llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Inclusion &); bool operator==(const Inclusion &LHS, const Inclusion &RHS); @@ -155,8 +154,6 @@ return !NonSelfContained.contains(ID); } - bool hasIWYUExport(HeaderID ID) const { return HasIWYUExport.contains(ID); } - // Return all transitively reachable files. llvm::ArrayRef allHeaders() const { return RealPathNames; } @@ -202,9 +199,6 @@ // Contains HeaderIDs of all non self-contained entries in the // IncludeStructure. llvm::DenseSet NonSelfContained; - // Contains a set of headers that have either "IWYU pragma: export" or "IWYU - // pragma: begin_exports". - llvm::DenseSet HasIWYUExport; // Maps written includes to indices in MainFileInclude for easier lookup by // spelling. diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp --- a/clang-tools-extra/clangd/Headers.cpp +++ b/clang-tools-extra/clangd/Headers.cpp @@ -25,8 +25,7 @@ namespace clang { namespace clangd { -class IncludeStructure::RecordHeaders : public PPCallbacks, - public CommentHandler { +class IncludeStructure::RecordHeaders : public PPCallbacks { public: RecordHeaders(const CompilerInstance &CI, IncludeStructure *Out) : SM(CI.getSourceManager()), @@ -61,8 +60,6 @@ SM.getLineNumber(SM.getFileID(HashLoc), Inc.HashOffset) - 1; Inc.FileKind = FileKind; Inc.Directive = IncludeTok.getIdentifierInfo()->getPPKeywordID(); - if (LastPragmaKeepInMainFileLine == Inc.HashLine) - Inc.BehindPragmaKeep = true; if (File) { IncludeStructure::HeaderID HID = Out->getOrCreateID(*File); Inc.HeaderID = static_cast(HID); @@ -127,47 +124,6 @@ } } - bool HandleComment(Preprocessor &PP, SourceRange Range) override { - auto Pragma = - tooling::parseIWYUPragma(SM.getCharacterData(Range.getBegin())); - if (!Pragma) - return false; - - if (inMainFile()) { - // Given: - // - // #include "foo.h" - // #include "bar.h" // IWYU pragma: keep - // - // The order in which the callbacks will be triggered: - // - // 1. InclusionDirective("foo.h") - // 2. handleCommentInMainFile("// IWYU pragma: keep") - // 3. InclusionDirective("bar.h") - // - // This code stores the last location of "IWYU pragma: keep" (or export) - // comment in the main file, so that when InclusionDirective is called, it - // will know that the next inclusion is behind the IWYU pragma. - // FIXME: Support "IWYU pragma: begin_exports" and "IWYU pragma: - // end_exports". - if (!Pragma->startswith("export") && !Pragma->startswith("keep")) - return false; - unsigned Offset = SM.getFileOffset(Range.getBegin()); - LastPragmaKeepInMainFileLine = - SM.getLineNumber(SM.getMainFileID(), Offset) - 1; - } else { - // Memorize headers that have export pragmas in them. Include Cleaner - // does not support them properly yet, so they will be not marked as - // unused. - // FIXME: Once IncludeCleaner supports export pragmas, remove this. - if (!Pragma->startswith("export") && !Pragma->startswith("begin_exports")) - return false; - Out->HasIWYUExport.insert( - *Out->getID(SM.getFileEntryForID(SM.getFileID(Range.getBegin())))); - } - return false; - } - private: // Keeps track of include depth for the current file. It's 1 for main file. int Level = 0; @@ -181,9 +137,6 @@ bool InBuiltinFile = false; IncludeStructure *Out; - - // The last line "IWYU pragma: keep" was seen in the main file, 0-indexed. - int LastPragmaKeepInMainFileLine = -1; }; bool isLiteralInclude(llvm::StringRef Include) { @@ -234,7 +187,6 @@ auto &SM = CI.getSourceManager(); MainFileEntry = SM.getFileEntryForID(SM.getMainFileID()); auto Collector = std::make_unique(CI, this); - CI.getPreprocessor().addCommentHandler(Collector.get()); CI.getPreprocessor().addPPCallbacks(std::move(Collector)); } 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 @@ -15,6 +15,7 @@ #include "SourceCode.h" #include "URI.h" #include "clang-include-cleaner/Analysis.h" +#include "clang-include-cleaner/Record.h" #include "clang-include-cleaner/Types.h" #include "support/Logger.h" #include "support/Path.h" @@ -90,10 +91,10 @@ } static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, - const Config &Cfg) { - if (Inc.BehindPragmaKeep) + const Config &Cfg, + const include_cleaner::PragmaIncludes *PI) { + if (PI && PI->shouldKeep(Inc.HashLine + 1)) return false; - // FIXME(kirillbobyrev): We currently do not support the umbrella headers. // System headers are likely to be standard library headers. // Until we have good support for umbrella headers, don't warn about them. @@ -104,10 +105,6 @@ } assert(Inc.HeaderID); auto HID = static_cast(*Inc.HeaderID); - // FIXME: Ignore the headers with IWYU export pragmas for now, remove this - // check when we have more precise tracking of exported headers. - if (AST.getIncludeStructure().hasIWYUExport(HID)) - return false; auto FE = AST.getSourceManager().getFileManager().getFileRef( AST.getIncludeStructure().getRealPath(HID)); assert(FE); @@ -333,7 +330,7 @@ continue; auto IncludeID = static_cast(*MFI.HeaderID); bool Used = ReferencedFiles.contains(IncludeID); - if (!Used && !mayConsiderUnused(MFI, AST, Cfg)) { + if (!Used && !mayConsiderUnused(MFI, AST, Cfg, AST.getPragmaIncludes())) { dlog("{0} was not used, but is not eligible to be diagnosed as unused", MFI.Written); continue; diff --git a/clang-tools-extra/clangd/unittests/HeadersTests.cpp b/clang-tools-extra/clangd/unittests/HeadersTests.cpp --- a/clang-tools-extra/clangd/unittests/HeadersTests.cpp +++ b/clang-tools-extra/clangd/unittests/HeadersTests.cpp @@ -147,7 +147,6 @@ MATCHER_P(resolved, Name, "") { return arg.Resolved == Name; } MATCHER_P(includeLine, N, "") { return arg.HashLine == N; } MATCHER_P(directive, D, "") { return arg.Directive == D; } -MATCHER_P(hasPragmaKeep, H, "") { return arg.BehindPragmaKeep == H; } MATCHER_P2(Distance, File, D, "") { if (arg.getFirst() != File) @@ -293,18 +292,6 @@ directive(tok::pp_include_next))); } -TEST_F(HeadersTest, IWYUPragmaKeep) { - FS.Files[MainFile] = R"cpp( -#include "bar.h" // IWYU pragma: keep -#include "foo.h" -)cpp"; - - EXPECT_THAT( - collectIncludes().MainFileIncludes, - UnorderedElementsAre(AllOf(written("\"foo.h\""), hasPragmaKeep(false)), - AllOf(written("\"bar.h\""), hasPragmaKeep(true)))); -} - TEST_F(HeadersTest, InsertInclude) { std::string Path = testPath("sub/bar.h"); FS.Files[Path] = ""; @@ -457,33 +444,6 @@ EXPECT_FALSE(Includes.isSelfContained(getID("pp_depend.h", Includes))); } -TEST_F(HeadersTest, HasIWYUPragmas) { - FS.Files[MainFile] = R"cpp( -#include "export.h" -#include "begin_exports.h" -#include "none.h" -)cpp"; - FS.Files["export.h"] = R"cpp( -#pragma once -#include "none.h" // IWYU pragma: export -)cpp"; - FS.Files["begin_exports.h"] = R"cpp( -#pragma once -// IWYU pragma: begin_exports -#include "none.h" -// IWYU pragma: end_exports -)cpp"; - FS.Files["none.h"] = R"cpp( -#pragma once -// Not a pragma. -)cpp"; - - auto Includes = collectIncludes(); - EXPECT_TRUE(Includes.hasIWYUExport(getID("export.h", Includes))); - EXPECT_TRUE(Includes.hasIWYUExport(getID("begin_exports.h", Includes))); - EXPECT_FALSE(Includes.hasIWYUExport(getID("none.h", Includes))); -} - } // namespace } // namespace clangd } // namespace clang 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 @@ -300,10 +300,9 @@ ParsedAST AST = TU.build(); EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); - // FIXME: This is not correct: foo.h is unused but is not diagnosed as such - // because we ignore headers with IWYU export pragmas for now. IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST); - EXPECT_THAT(Findings.UnusedIncludes, IsEmpty()); + EXPECT_THAT(Findings.UnusedIncludes, + ElementsAre(Pointee(writtenInclusion("\"foo.h\"")))); } TEST(IncludeCleaner, NoDiagsForObjC) { diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -216,7 +216,6 @@ Field(&Inclusion::Written, "\"a.h\""), Field(&Inclusion::Resolved, testPath("a.h")), Field(&Inclusion::HeaderID, testing::Not(testing::Eq(std::nullopt))), - Field(&Inclusion::BehindPragmaKeep, true), Field(&Inclusion::FileKind, SrcMgr::CharacteristicKind::C_User)))); }