kbobyrev created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D125930
Files:
clang-tools-extra/clangd/Headers.cpp
clang-tools-extra/clangd/Headers.h
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
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
@@ -582,6 +582,10 @@
TestTU TU;
TU.Code = R"cpp(
#include "foo.h"
+
+ void foo() {
+ bar();
+ }
)cpp";
TU.AdditionalFiles["foo.h"] = R"cpp(
#ifndef FOO_H
@@ -600,8 +604,6 @@
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());
}
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -258,18 +258,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] = "";
@@ -418,31 +406,43 @@
EXPECT_FALSE(Includes.isSelfContained(getID("pp_depend.h", Includes)));
}
-TEST_F(HeadersTest, HasIWYUPragmas) {
+TEST_F(HeadersTest, IWYUPragmaKeep) {
FS.Files[MainFile] = R"cpp(
-#include "export.h"
-#include "begin_exports.h"
-#include "none.h"
+#include "bar.h" // IWYU pragma: keep
+#include "foo.h"
)cpp";
- FS.Files["export.h"] = R"cpp(
-#pragma once
-#include "none.h" // IWYU pragma: export
+
+ EXPECT_THAT(
+ collectIncludes().MainFileIncludes,
+ UnorderedElementsAre(AllOf(written("\"foo.h\""), hasPragmaKeep(false)),
+ AllOf(written("\"bar.h\""), hasPragmaKeep(true))));
+}
+
+TEST_F(HeadersTest, IWYUPragmaExport) {
+ FS.Files[MainFile] = R"cpp(
+// WHATEVER COMMENT
+#include "umbrella.h"
+
+void foo() {
+ bar();
+}
)cpp";
- FS.Files["begin_exports.h"] = R"cpp(
+ FS.Files["umbrella.h"] = R"cpp(
#pragma once
-// IWYU pragma: begin_exports
-#include "none.h"
-// IWYU pragma: end_exports
+
+#include "lib.h" // IWYU pragma: export
)cpp";
- FS.Files["none.h"] = R"cpp(
+ FS.Files["lib.h"] = R"cpp(
#pragma once
-// Not a pragma.
+
+void bar() {}
)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)));
+ auto LibExporters = Includes.exporters(getID("lib.h", Includes));
+ ASSERT_TRUE(LibExporters);
+ EXPECT_THAT(*LibExporters,
+ UnorderedElementsAre(getID("umbrella.h", Includes)));
}
} // namespace
Index: clang-tools-extra/clangd/IncludeCleaner.h
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -67,15 +67,15 @@
/// Retrieves IDs of all files containing SourceLocations from \p Locs.
/// The output only includes things SourceManager sees as files (not macro IDs).
/// This can include <built-in>, <scratch space> etc that are not true files.
-/// \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).
+/// \p HeadersResponsible 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<std::vector<FileID>(FileID)> HeadersResponsible,
llvm::function_ref<Optional<StringRef>(FileID)> UmbrellaHeader);
ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs,
const IncludeStructure &Includes,
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -24,6 +24,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringSet.h"
@@ -31,6 +32,7 @@
#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
#include <functional>
+#include <queue>
namespace clang {
namespace clangd {
@@ -270,10 +272,6 @@
}
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);
@@ -301,8 +299,8 @@
// its first includer that is self-contained. This is the header users can
// include, so it will be responsible for bringing the symbols from given
// header into the scope.
-FileID headerResponsible(FileID ID, const SourceManager &SM,
- const IncludeStructure &Includes) {
+std::vector<FileID> headersResponsible(FileID ID, const SourceManager &SM,
+ const IncludeStructure &Includes) {
// Unroll the chain of non self-contained headers until we find the one that
// can be included.
for (const FileEntry *FE = SM.getFileEntryForID(ID); ID != SM.getMainFileID();
@@ -319,7 +317,24 @@
// on its includer.
ID = SM.getFileID(SM.getIncludeLoc(ID));
}
- return ID;
+ std::vector<FileID> Result = {ID};
+ std::queue<IncludeStructure::HeaderID> Queue;
+ Queue.push(*Includes.getID(SM.getFileEntryForID(ID)));
+ llvm::DenseSet<IncludeStructure::HeaderID> Visited;
+ while (!Queue.empty()) {
+ IncludeStructure::HeaderID HID = Queue.front();
+ Visited.insert(HID);
+ Result.push_back(HID);
+ auto Exporters = Includes.exporters(HID);
+ if (!Exporters)
+ continue;
+ for (const auto &Exporter : *Exporters) {
+ if (Visited.find(Exporter) != Visited.end())
+ continue;
+ Queue.push(Exporter);
+ }
+ }
+ return Result;
}
} // namespace
@@ -343,7 +358,7 @@
ReferencedFiles findReferencedFiles(
const ReferencedLocations &Locs, const SourceManager &SM,
- llvm::function_ref<FileID(FileID)> HeaderResponsible,
+ llvm::function_ref<std::vector<FileID>(FileID)> HeadersResponsible,
llvm::function_ref<Optional<StringRef>(FileID)> UmbrellaHeader) {
std::vector<SourceLocation> Sorted{Locs.User.begin(), Locs.User.end()};
llvm::sort(Sorted); // Group by FileID.
@@ -365,7 +380,9 @@
llvm::DenseSet<FileID> UserFiles;
llvm::StringSet<> PublicHeaders;
for (FileID ID : Builder.Files) {
- UserFiles.insert(HeaderResponsible(ID));
+ for (auto HID : HeadersResponsible(ID)) {
+ UserFiles.insert(HID);
+ }
if (auto PublicHeader = UmbrellaHeader(ID)) {
PublicHeaders.insert(*PublicHeader);
}
@@ -387,7 +404,7 @@
return findReferencedFiles(
Locs, SM,
[&SM, &Includes](FileID ID) {
- return headerResponsible(ID, SM, Includes);
+ return headersResponsible(ID, SM, Includes);
},
[&SM, &CanonIncludes](FileID ID) -> Optional<StringRef> {
auto Entry = SM.getFileEntryRefForID(ID);
Index: clang-tools-extra/clangd/Headers.h
===================================================================
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -23,11 +23,13 @@
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem/UniqueID.h"
#include <string>
+#include <vector>
namespace clang {
namespace clangd {
@@ -145,13 +147,11 @@
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; }
+ llvm::Optional<llvm::ArrayRef<HeaderID>> exporters(HeaderID ID) const;
+
// Return all transitively reachable files, and their minimum include depth.
// All transitive includes (absolute paths), with their minimum include depth.
// Root --> 0, #included file --> 1, etc.
@@ -189,9 +189,7 @@
// 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;
+ llvm::DenseMap<HeaderID, std::vector<HeaderID>> Exporters;
};
// 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
@@ -9,12 +9,15 @@
#include "Headers.h"
#include "Preamble.h"
#include "SourceCode.h"
+#include "support/Logger.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Path.h"
#include <cstring>
@@ -89,6 +92,18 @@
auto IncludingID = Out->getOrCreateID(*IncludingFileEntry),
IncludedID = Out->getOrCreateID(*File);
Out->IncludeChildren[IncludingID].push_back(IncludedID);
+ // FIXME: This has to be off-by-two instead of off-by-one for some reason.
+ int HashLine =
+ SM.getLineNumber(SM.getFileID(HashLoc), SM.getFileOffset(HashLoc)) -
+ 2;
+ HashLoc.dump(SM);
+ if (IncludingID == LastFileID && HashLine == LastPragmaExportLine) {
+ dlog("{0} exports {1}", IncludingFileEntry->getName(), File->getName());
+ auto It = Out->Exporters.find(IncludedID);
+ if (It == Out->Exporters.end())
+ It = Out->Exporters.insert({IncludedID, {}}).first;
+ It->second.push_back(IncludingID);
+ }
}
}
@@ -133,6 +148,8 @@
llvm::StringRef Text = SM.getCharacterData(Range.getBegin(), &Err);
if (Err)
return false;
+ unsigned Offset = SM.getFileOffset(Range.getBegin());
+ int Line = SM.getLineNumber(SM.getMainFileID(), Offset) - 1;
if (inMainFile()) {
// Given:
//
@@ -153,19 +170,13 @@
if (!Text.startswith(IWYUPragmaExport) &&
!Text.startswith(IWYUPragmaKeep))
return false;
- unsigned Offset = SM.getFileOffset(Range.getBegin());
- LastPragmaKeepInMainFileLine =
- SM.getLineNumber(SM.getMainFileID(), Offset) - 1;
+ LastPragmaKeepInMainFileLine = Line;
} 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->HasIWYUExport.insert(
- *Out->getID(SM.getFileEntryForID(SM.getFileID(Range.getBegin()))));
+ if (Text.startswith(IWYUPragmaExport)) {
+ LastFileID = Out->getOrCreateID(
+ *SM.getFileEntryRefForID(SM.getFileID(Range.getBegin())));
+ LastPragmaExportLine = Line;
+ }
}
return false;
}
@@ -186,6 +197,9 @@
// The last line "IWYU pragma: keep" was seen in the main file, 0-indexed.
int LastPragmaKeepInMainFileLine = -1;
+
+ HeaderID LastFileID = static_cast<HeaderID>(0);
+ int LastPragmaExportLine = -1;
};
bool isLiteralInclude(llvm::StringRef Include) {
@@ -271,6 +285,14 @@
return Result;
}
+llvm::Optional<llvm::ArrayRef<IncludeStructure::HeaderID>>
+IncludeStructure::exporters(IncludeStructure::HeaderID ID) const {
+ auto It = Exporters.find(ID);
+ if (It == Exporters.end())
+ return llvm::None;
+ return llvm::Optional<llvm::ArrayRef<IncludeStructure::HeaderID>>(It->second);
+}
+
llvm::DenseMap<IncludeStructure::HeaderID, unsigned>
IncludeStructure::includeDepth(HeaderID Root) const {
// Include depth 0 is the main file only.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
