kbobyrev updated this revision to Diff 428949.
kbobyrev added a comment.
Remove redundant comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125468/new/
https://reviews.llvm.org/D125468
Files:
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
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -544,8 +544,7 @@
EXPECT_TRUE(
ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
- auto Unused = computeUnusedIncludes(AST);
- EXPECT_THAT(Unused, IsEmpty());
+ EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
}
TEST(IncludeCleaner, RecursiveInclusion) {
@@ -576,8 +575,35 @@
findReferencedLocations(AST), AST.getIncludeStructure(),
AST.getCanonicalIncludes(), AST.getSourceManager());
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
- auto Unused = computeUnusedIncludes(AST);
- EXPECT_THAT(Unused, IsEmpty());
+ EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
+}
+
+TEST(IncludeCleaner, IWYUPragmaExport) {
+ TestTU TU;
+ TU.Code = R"cpp(
+ #include "foo.h"
+ )cpp";
+ TU.AdditionalFiles["foo.h"] = R"cpp(
+ #ifndef FOO_H
+ #define FOO_H
+
+ #include "bar.h" // IWYU pragma: export
+
+ #endif
+ )cpp";
+ TU.AdditionalFiles["bar.h"] = guard(R"cpp(
+ void bar() {}
+ )cpp");
+ ParsedAST AST = TU.build();
+
+ auto ReferencedFiles = findReferencedFiles(
+ findReferencedLocations(AST), AST.getIncludeStructure(),
+ AST.getCanonicalIncludes(), AST.getSourceManager());
+ EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
+ // FIXME: This is not correct: foo.h and bar.h are unused but are not
+ // diagnosed as such because we ignore headers with IWYU export pragmas for
+ // now.
+ EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
}
} // namespace
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -406,7 +406,7 @@
#define RECURSIVE_H
#include "recursive.h"
-
+
#endif // RECURSIVE_H
)cpp";
@@ -418,6 +418,32 @@
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[testPath("export.h")] = R"cpp(
+#pragma once
+#include "none.h" // IWYU pragma: export
+)cpp";
+ FS.Files[testPath("begin_exports.h")] = R"cpp(
+#pragma once
+// IWYU pragma: begin_exports
+#include "none.h"
+// IWYU pragma: end_exports
+)cpp";
+ FS.Files[testPath("none.h")] = R"cpp(
+#pragma once
+)cpp";
+
+ auto Includes = collectIncludes();
+ EXPECT_TRUE(Includes.hasIWYUPragmas(getID("export.h", Includes)));
+ EXPECT_TRUE(Includes.hasIWYUPragmas(getID("begin_exports.h", Includes)));
+ EXPECT_FALSE(Includes.hasIWYUPragmas(getID("none.h", Includes)));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -271,9 +271,13 @@
// Headers without include guards have side effects and are not
// self-contained, skip them.
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 actual support for export pragmas.
+ if (AST.getIncludeStructure().hasIWYUPragmas(HID))
+ return false;
auto FE = AST.getSourceManager().getFileManager().getFileRef(
- AST.getIncludeStructure().getRealPath(
- static_cast<IncludeStructure::HeaderID>(*Inc.HeaderID)));
+ AST.getIncludeStructure().getRealPath(HID));
assert(FE);
if (!AST.getPreprocessor().getHeaderSearchInfo().isFileMultipleIncludeGuarded(
&FE->getFileEntry())) {
Index: clang-tools-extra/clangd/Headers.h
===================================================================
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -145,6 +145,10 @@
return !NonSelfContained.contains(ID);
}
+ bool hasIWYUPragmas(HeaderID ID) const {
+ return HasIWYUPragmas.contains(ID);
+ }
+
// Return all transitively reachable files.
llvm::ArrayRef<std::string> allHeaders() const { return RealPathNames; }
@@ -185,6 +189,9 @@
// 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> HasIWYUPragmas;
};
// Calculates insertion edit for including a new header in a file.
Index: clang-tools-extra/clangd/Headers.cpp
===================================================================
--- clang-tools-extra/clangd/Headers.cpp
+++ clang-tools-extra/clangd/Headers.cpp
@@ -24,6 +24,7 @@
const char IWYUPragmaKeep[] = "// IWYU pragma: keep";
const char IWYUPragmaExport[] = "// IWYU pragma: export";
+const char IWYUPragmaBeginExports[] = "// IWYU pragma: begin_exports";
class IncludeStructure::RecordHeaders : public PPCallbacks,
public CommentHandler {
@@ -127,6 +128,14 @@
}
}
+ bool HandleComment(Preprocessor &PP, SourceRange Range) override {
+ if (Range.getBegin().isMacroID())
+ return false;
+ return inMainFile() ? handleCommentInMainFile(PP, Range)
+ : handleCommentInHeaderFile(PP, Range);
+ }
+
+private:
// Given:
//
// #include "foo.h"
@@ -135,15 +144,15 @@
// The order in which the callbacks will be triggered:
//
// 1. InclusionDirective("foo.h")
- // 2. HandleComment("// IWYU pragma: keep")
+ // 2. handleCommentInMainFile("// IWYU pragma: keep")
// 3. InclusionDirective("bar.h")
//
- // HandleComment will store 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.
- bool HandleComment(Preprocessor &PP, SourceRange Range) override {
- if (!inMainFile() || Range.getBegin().isMacroID())
- return false;
+ // 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.
+ bool handleCommentInMainFile(Preprocessor &PP, SourceRange Range) {
+ assert(inMainFile());
+ assert(!Range.getBegin().isMacroID());
bool Err = false;
llvm::StringRef Text = SM.getCharacterData(Range.getBegin(), &Err);
if (Err && !Text.consume_front(IWYUPragmaKeep) &&
@@ -155,7 +164,19 @@
return false;
}
-private:
+ bool handleCommentInHeaderFile(Preprocessor &PP, SourceRange Range) {
+ assert(!inMainFile());
+ assert(!Range.getBegin().isMacroID());
+ bool Err = false;
+ llvm::StringRef Text = SM.getCharacterData(Range.getBegin(), &Err);
+ if (Err && !Text.consume_front(IWYUPragmaExport) &&
+ !Text.consume_front(IWYUPragmaBeginExports))
+ return false;
+ Out->HasIWYUPragmas.insert(
+ *Out->getID(SM.getFileEntryForID(SM.getFileID(Range.getBegin()))));
+ return false;
+ }
+
// Keeps track of include depth for the current file. It's 1 for main file.
int Level = 0;
bool inMainFile() const { return Level == 1; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
