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 @@ -145,6 +145,10 @@ return !NonSelfContained.contains(ID); } + bool hasIWYUExport(HeaderID ID) const { + return HasIWYUPragmas.contains(ID); + } + // Return all transitively reachable files. llvm::ArrayRef allHeaders() const { return RealPathNames; } @@ -185,6 +189,9 @@ // 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 HasIWYUPragmas; }; // Calculates insertion edit for including a new header in a file. 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 @@ -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,31 +128,45 @@ } } - // Given: - // - // #include "foo.h" - // #include "bar.h" // IWYU pragma: keep - // - // The order in which the callbacks will be triggered: - // - // 1. InclusionDirective("foo.h") - // 2. HandleComment("// 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; bool Err = false; llvm::StringRef Text = SM.getCharacterData(Range.getBegin(), &Err); - if (Err && !Text.consume_front(IWYUPragmaKeep) && - !Text.consume_front(IWYUPragmaExport)) + if (Err) return false; - unsigned Offset = SM.getFileOffset(Range.getBegin()); - LastPragmaKeepInMainFileLine = - SM.getLineNumber(SM.getFileID(Range.getBegin()), Offset) - 1; + 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 (!Text.startswith(IWYUPragmaExport) && + !Text.startswith(IWYUPragmaKeep)) + return false; + unsigned Offset = SM.getFileOffset(Range.getBegin()); + LastPragmaKeepInMainFileLine = + SM.getLineNumber(SM.getMainFileID(), Offset) - 1; + } else { + // Memorize headers that 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 (!Text.startswith(IWYUPragmaExport) && + !Text.startswith(IWYUPragmaBeginExports)) + return false; + Out->HasIWYUPragmas.insert( + *Out->getID(SM.getFileEntryForID(SM.getFileID(Range.getBegin())))); + } return false; } @@ -237,8 +252,7 @@ return It->second; } -IncludeStructure::HeaderID -IncludeStructure::getOrCreateID(FileEntryRef Entry) { +IncludeStructure::HeaderID IncludeStructure::getOrCreateID(FileEntryRef Entry) { // Main file's FileEntry was not known at IncludeStructure creation time. if (&Entry.getFileEntry() == MainFileEntry) { if (RealPathNames.front().empty()) 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 @@ -268,13 +268,17 @@ return true; return false; } - // Headers without include guards have side effects and are not - // self-contained, skip them. 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( - static_cast(*Inc.HeaderID))); + AST.getIncludeStructure().getRealPath(HID)); assert(FE); + // Headers without include guards have side effects and are not + // self-contained, skip them. if (!AST.getPreprocessor().getHeaderSearchInfo().isFileMultipleIncludeGuarded( &FE->getFileEntry())) { dlog("{0} doesn't have header guard and will not be considered unused", 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 @@ -406,7 +406,7 @@ #define RECURSIVE_H #include "recursive.h" - + #endif // RECURSIVE_H )cpp"; @@ -418,6 +418,33 @@ 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 @@ -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,34 @@ 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 is unused but is not diagnosed as such + // because we ignore headers with IWYU export pragmas for now. + EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty()); } } // namespace