[clang-tools-extra] be39dae - [clangd] Remove IWYU handling code that is used only for the old unused-include feature.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 13 04:37:00 PDT 2023
Author: Haojian Wu
Date: 2023-03-13T12:34:34+01:00
New Revision: be39daea846e6b5d43a8ee0c387feb0a556ff386
URL: https://github.com/llvm/llvm-project/commit/be39daea846e6b5d43a8ee0c387feb0a556ff386
DIFF: https://github.com/llvm/llvm-project/commit/be39daea846e6b5d43a8ee0c387feb0a556ff386.diff
LOG: [clangd] Remove IWYU handling code that is used only for the old unused-include feature.
The old implementation has been removed, this patch removes the clangd's IWYU
code that is only used for the old implmentation (include-cleaner library has
better support):
- IWYU export pragma
- IWYU keep pragma
Differential Revision: https://reviews.llvm.org/D145916
Added:
Modified:
clang-tools-extra/clangd/Headers.cpp
clang-tools-extra/clangd/Headers.h
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/HeadersTests.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp
index 7000c1cbf4513..35525a73a2fef 100644
--- 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 @@ class IncludeStructure::RecordHeaders : public PPCallbacks,
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<unsigned>(HID);
@@ -127,47 +124,6 @@ class IncludeStructure::RecordHeaders : public PPCallbacks,
}
}
- 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 @@ class IncludeStructure::RecordHeaders : public PPCallbacks,
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 @@ void IncludeStructure::collect(const CompilerInstance &CI) {
auto &SM = CI.getSourceManager();
MainFileEntry = SM.getFileEntryForID(SM.getMainFileID());
auto Collector = std::make_unique<RecordHeaders>(CI, this);
- CI.getPreprocessor().addCommentHandler(Collector.get());
CI.getPreprocessor().addPPCallbacks(std::move(Collector));
}
diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h
index a21ea851831aa..8abea21a98e0f 100644
--- a/clang-tools-extra/clangd/Headers.h
+++ b/clang-tools-extra/clangd/Headers.h
@@ -73,7 +73,6 @@ struct Inclusion {
int HashLine = 0; // Line number containing the directive, 0-indexed.
SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User;
std::optional<unsigned> 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 @@ class IncludeStructure {
return !NonSelfContained.contains(ID);
}
- bool hasIWYUExport(HeaderID ID) const { return HasIWYUExport.contains(ID); }
-
// Return all transitively reachable files.
llvm::ArrayRef<std::string> allHeaders() const { return RealPathNames; }
@@ -202,9 +199,6 @@ class IncludeStructure {
// Contains HeaderIDs of all non self-contained entries in the
// IncludeStructure.
llvm::DenseSet<HeaderID> NonSelfContained;
- // Contains a set of headers that have either "IWYU pragma: export" or "IWYU
- // pragma: begin_exports".
- llvm::DenseSet<HeaderID> HasIWYUExport;
// Maps written includes to indices in MainFileInclude for easier lookup by
// spelling.
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index a990b89edb230..98135529f259b 100644
--- 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 @@ bool isFilteredByConfig(const Config &Cfg, llvm::StringRef HeaderPath) {
}
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 @@ static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
}
assert(Inc.HeaderID);
auto HID = static_cast<IncludeStructure::HeaderID>(*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 @@ getUnused(ParsedAST &AST,
continue;
auto IncludeID = static_cast<IncludeStructure::HeaderID>(*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
index bc4a5abd0e46a..728b06db684da 100644
--- a/clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -147,7 +147,6 @@ MATCHER_P(written, Name, "") { return arg.Written == Name; }
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 @@ TEST_F(HeadersTest, IncludeDirective) {
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 @@ void foo();
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
index fc4401b8fa0ff..409e3cee791c3 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -300,10 +300,9 @@ TEST(IncludeCleaner, IWYUPragmaExport) {
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
index bee22c8901ead..29ebcb35a88c8 100644
--- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -216,7 +216,6 @@ TEST(PreamblePatchTest, PatchesPreambleIncludes) {
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))));
}
More information about the cfe-commits
mailing list