[llvm] b06df22 - [clangd] Follow-up on rGdea48079b90d
Kirill Bobyrev via llvm-commits
llvm-commits at lists.llvm.org
Sun Oct 3 23:39:44 PDT 2021
Author: Kirill Bobyrev
Date: 2021-10-04T08:39:24+02:00
New Revision: b06df223826e7bf7da4597af30a04259975f4a6a
URL: https://github.com/llvm/llvm-project/commit/b06df223826e7bf7da4597af30a04259975f4a6a
DIFF: https://github.com/llvm/llvm-project/commit/b06df223826e7bf7da4597af30a04259975f4a6a.diff
LOG: [clangd] Follow-up on rGdea48079b90d
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D110925
Added:
Modified:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/Headers.cpp
clang-tools-extra/clangd/Headers.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/HeadersTests.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
llvm/include/llvm/Support/FileSystem/UniqueID.h
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 512959316e18e..2d01a163431e2 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1203,7 +1203,7 @@ bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
loadMainFilePreambleMacros(Clang->getPreprocessor(), Input.Preamble);
if (Includes)
Clang->getPreprocessor().addPPCallbacks(
- collectIncludeStructureCallback(Clang->getSourceManager(), Includes));
+ Includes->collect(Clang->getSourceManager()));
if (llvm::Error Err = Action.Execute()) {
log("Execute() failed when running codeComplete for {0}: {1}",
Input.FileName, toString(std::move(Err)));
@@ -1380,7 +1380,7 @@ class CodeCompleteFlow {
const auto &SM = Recorder->CCSema->getSourceManager();
llvm::StringMap<SourceParams> ProxSources;
auto MainFileID =
- Includes.getID(SM.getFileEntryForID(SM.getMainFileID()), SM);
+ Includes.getID(SM.getFileEntryForID(SM.getMainFileID()));
assert(MainFileID);
for (auto &HeaderIDAndDepth : Includes.includeDepth(*MainFileID)) {
auto &Source =
diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp
index ea07e050e9383..5946180f346d9 100644
--- a/clang-tools-extra/clangd/Headers.cpp
+++ b/clang-tools-extra/clangd/Headers.cpp
@@ -67,8 +67,8 @@ class RecordHeaders : public PPCallbacks {
// Treat as if included from the main file.
IncludingFileEntry = SM.getFileEntryForID(MainFID);
}
- auto IncludingID = Out->getOrCreateID(IncludingFileEntry, SM),
- IncludedID = Out->getOrCreateID(File, SM);
+ auto IncludingID = Out->getOrCreateID(IncludingFileEntry),
+ IncludedID = Out->getOrCreateID(File);
Out->IncludeChildren[IncludingID].push_back(IncludedID);
}
}
@@ -150,16 +150,15 @@ llvm::SmallVector<llvm::StringRef, 1> getRankedIncludes(const Symbol &Sym) {
}
std::unique_ptr<PPCallbacks>
-collectIncludeStructureCallback(const SourceManager &SM,
- IncludeStructure *Out) {
- return std::make_unique<RecordHeaders>(SM, Out);
+IncludeStructure::collect(const SourceManager &SM) {
+ MainFileEntry = SM.getFileEntryForID(SM.getMainFileID());
+ return std::make_unique<RecordHeaders>(SM, this);
}
llvm::Optional<IncludeStructure::HeaderID>
-IncludeStructure::getID(const FileEntry *Entry,
- const SourceManager &SM) const {
+IncludeStructure::getID(const FileEntry *Entry) const {
// HeaderID of the main file is always 0;
- if (SM.getMainFileID() == SM.translateFile(Entry)) {
+ if (Entry == MainFileEntry) {
return static_cast<IncludeStructure::HeaderID>(0u);
}
auto It = UIDToIndex.find(Entry->getUniqueID());
@@ -169,12 +168,12 @@ IncludeStructure::getID(const FileEntry *Entry,
}
IncludeStructure::HeaderID
-IncludeStructure::getOrCreateID(const FileEntry *Entry,
- const SourceManager &SM) {
- // Main file's FileID was not known at IncludeStructure creation time.
- if (SM.getMainFileID() == SM.translateFile(Entry)) {
- UIDToIndex[Entry->getUniqueID()] =
- static_cast<IncludeStructure::HeaderID>(0u);
+IncludeStructure::getOrCreateID(const FileEntry *Entry) {
+ // Main file's FileEntry was not known at IncludeStructure creation time.
+ if (Entry == MainFileEntry) {
+ if (RealPathNames.front().empty())
+ RealPathNames.front() = MainFileEntry->tryGetRealPathName().str();
+ return MainFileID;
}
auto R = UIDToIndex.try_emplace(
Entry->getUniqueID(),
diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h
index 4c80c21d514c9..513e21d620880 100644
--- a/clang-tools-extra/clangd/Headers.h
+++ b/clang-tools-extra/clangd/Headers.h
@@ -14,6 +14,7 @@
#include "index/Symbol.h"
#include "support/Logger.h"
#include "support/Path.h"
+#include "clang/Basic/FileEntry.h"
#include "clang/Basic/TokenKinds.h"
#include "clang/Format/Format.h"
#include "clang/Lex/HeaderSearch.h"
@@ -119,15 +120,22 @@ class IncludeStructure {
RealPathNames.emplace_back();
}
+ // Returns a PPCallback that visits all inclusions in the main file and
+ // populates the structure.
+ std::unique_ptr<PPCallbacks> collect(const SourceManager &SM);
+
+ void setMainFileEntry(const FileEntry *Entry) {
+ assert(Entry && Entry->isValid());
+ this->MainFileEntry = Entry;
+ }
+
// HeaderID identifies file in the include graph. It corresponds to a
// FileEntry rather than a FileID, but stays stable across preamble & main
// file builds.
enum class HeaderID : unsigned {};
- llvm::Optional<HeaderID> getID(const FileEntry *Entry,
- const SourceManager &SM) const;
- HeaderID getOrCreateID(const FileEntry *Entry,
- const SourceManager &SM);
+ llvm::Optional<HeaderID> getID(const FileEntry *Entry) const;
+ HeaderID getOrCreateID(const FileEntry *Entry);
StringRef getRealPath(HeaderID ID) const {
assert(static_cast<unsigned>(ID) <= RealPathNames.size());
@@ -141,32 +149,33 @@ class IncludeStructure {
// All transitive includes (absolute paths), with their minimum include depth.
// Root --> 0, #included file --> 1, etc.
// Root is the ID of the header being visited first.
- // Usually it is getID(SM.getFileEntryForID(SM.getMainFileID()), SM).
- llvm::DenseMap<HeaderID, unsigned> includeDepth(HeaderID Root) const;
+ llvm::DenseMap<HeaderID, unsigned>
+ includeDepth(HeaderID Root = MainFileID) const;
// Maps HeaderID to the ids of the files included from it.
llvm::DenseMap<HeaderID, SmallVector<HeaderID>> IncludeChildren;
std::vector<Inclusion> MainFileIncludes;
+ // We reserve HeaderID(0) for the main file and will manually check for that
+ // in getID and getOrCreateID because the UniqueID is not stable when the
+ // content of the main file changes.
+ static const HeaderID MainFileID = HeaderID(0u);
+
private:
+ // MainFileEntry will be used to check if the queried file is the main file
+ // or not.
+ const FileEntry *MainFileEntry = nullptr;
+
std::vector<std::string> RealPathNames; // In HeaderID order.
- // HeaderID maps the FileEntry::UniqueID to the internal representation.
+ // FileEntry::UniqueID is mapped to the internal representation (HeaderID).
// Identifying files in a way that persists from preamble build to subsequent
- // builds is surprisingly hard. FileID is unavailable in
- // InclusionDirective(), and RealPathName and UniqueID are not preserved in
+ // builds is surprisingly hard. FileID is unavailable in InclusionDirective(),
+ // and RealPathName and UniqueID are not preserved in
// the preamble.
- //
- // We reserve 0 to the main file and will manually check for that in getID
- // and getOrCreateID because llvm::sys::fs::UniqueID is not stable when their
- // content of the main file changes.
llvm::DenseMap<llvm::sys::fs::UniqueID, HeaderID> UIDToIndex;
};
-/// Returns a PPCallback that visits all inclusions in the main file.
-std::unique_ptr<PPCallbacks>
-collectIncludeStructureCallback(const SourceManager &SM, IncludeStructure *Out);
-
// Calculates insertion edit for including a new header in a file.
class IncludeInserter {
public:
@@ -228,7 +237,7 @@ class IncludeInserter {
namespace llvm {
-// Support Tokens as DenseMap keys.
+// Support HeaderIDs as DenseMap keys.
template <> struct DenseMapInfo<clang::clangd::IncludeStructure::HeaderID> {
static inline clang::clangd::IncludeStructure::HeaderID getEmptyKey() {
return static_cast<clang::clangd::IncludeStructure::HeaderID>(
@@ -251,30 +260,6 @@ template <> struct DenseMapInfo<clang::clangd::IncludeStructure::HeaderID> {
}
};
-// Support Tokens as DenseMap keys.
-template <> struct DenseMapInfo<llvm::sys::fs::UniqueID> {
- static inline llvm::sys::fs::UniqueID getEmptyKey() {
- auto EmptyKey = DenseMapInfo<std::pair<unsigned, unsigned>>::getEmptyKey();
- return {EmptyKey.first, EmptyKey.second};
- }
-
- static inline llvm::sys::fs::UniqueID getTombstoneKey() {
- auto TombstoneKey =
- DenseMapInfo<std::pair<unsigned, unsigned>>::getTombstoneKey();
- return {TombstoneKey.first, TombstoneKey.second};
- }
-
- static unsigned getHashValue(const llvm::sys::fs::UniqueID &Tag) {
- return hash_value(
- std::pair<unsigned, unsigned>(Tag.getDevice(), Tag.getFile()));
- }
-
- static bool isEqual(const llvm::sys::fs::UniqueID &LHS,
- const llvm::sys::fs::UniqueID &RHS) {
- return LHS == RHS;
- }
-};
-
} // namespace llvm
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_HEADERS_H
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index eab5df1f6b365..21a494aa462fb 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -439,7 +439,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
// Otherwise we would collect the replayed includes again...
// (We can't *just* use the replayed includes, they don't have Resolved path).
Clang->getPreprocessor().addPPCallbacks(
- collectIncludeStructureCallback(Clang->getSourceManager(), &Includes));
+ Includes.collect(Clang->getSourceManager()));
// Copy over the macros in the preamble region of the main file, and combine
// with non-preamble macros below.
MainFileMacros Macros;
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index af7c5db6fedd9..b307f99821b12 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -104,7 +104,7 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
"SourceMgr and LangOpts must be set at this point");
return std::make_unique<PPChainedCallbacks>(
- collectIncludeStructureCallback(*SourceMgr, &Includes),
+ Includes.collect(*SourceMgr),
std::make_unique<PPChainedCallbacks>(
std::make_unique<CollectMainFileMacros>(*SourceMgr, Macros),
collectPragmaMarksCallback(*SourceMgr, Marks)));
@@ -285,7 +285,7 @@ scanPreamble(llvm::StringRef Contents, const tooling::CompileCommand &Cmd) {
const auto &SM = Clang->getSourceManager();
Preprocessor &PP = Clang->getPreprocessor();
IncludeStructure Includes;
- PP.addPPCallbacks(collectIncludeStructureCallback(SM, &Includes));
+ PP.addPPCallbacks(Includes.collect(SM));
ScannedPreamble SP;
SP.Bounds = Bounds;
PP.addPPCallbacks(
diff --git a/clang-tools-extra/clangd/unittests/HeadersTests.cpp b/clang-tools-extra/clangd/unittests/HeadersTests.cpp
index bee3d181cf80d..fabdd26382a17 100644
--- a/clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -72,7 +72,7 @@ class HeadersTest : public ::testing::Test {
auto &SM = Clang->getSourceManager();
auto Entry = SM.getFileManager().getFile(Filename);
EXPECT_TRUE(Entry);
- return Includes.getOrCreateID(*Entry, SM);
+ return Includes.getOrCreateID(*Entry);
}
IncludeStructure collectIncludes() {
@@ -82,7 +82,7 @@ class HeadersTest : public ::testing::Test {
Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
IncludeStructure Includes;
Clang->getPreprocessor().addPPCallbacks(
- collectIncludeStructureCallback(Clang->getSourceManager(), &Includes));
+ Includes.collect(Clang->getSourceManager()));
EXPECT_FALSE(Action.Execute());
Action.EndSourceFile();
return Includes;
@@ -180,9 +180,9 @@ TEST_F(HeadersTest, OnlyCollectInclusionsInMain) {
#include "bar.h"
)cpp";
auto Includes = collectIncludes();
- EXPECT_THAT(Includes.MainFileIncludes,
- UnorderedElementsAre(
- AllOf(Written("\"bar.h\""), Resolved(BarHeader))));
+ EXPECT_THAT(
+ Includes.MainFileIncludes,
+ UnorderedElementsAre(AllOf(Written("\"bar.h\""), Resolved(BarHeader))));
EXPECT_THAT(Includes.includeDepth(getID(MainFile, Includes)),
UnorderedElementsAre(Distance(getID(MainFile, Includes), 0u),
Distance(getID(BarHeader, Includes), 1u),
diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index 67cd18ab01157..211f289eae734 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -516,10 +516,10 @@ TEST(ParsedASTTest, PatchesAdditionalIncludes) {
IncludeStructure Includes = PatchedAST->getIncludeStructure();
auto MainFE = FM.getFile(testPath("foo.cpp"));
ASSERT_TRUE(MainFE);
- auto MainID = Includes.getID(*MainFE, SM);
+ auto MainID = Includes.getID(*MainFE);
auto AuxFE = FM.getFile(testPath("sub/aux.h"));
ASSERT_TRUE(AuxFE);
- auto AuxID = Includes.getID(*AuxFE, SM);
+ auto AuxID = Includes.getID(*AuxFE);
EXPECT_THAT(Includes.IncludeChildren[*MainID], Contains(*AuxID));
}
@@ -560,12 +560,12 @@ TEST(ParsedASTTest, PatchesDeletedIncludes) {
IncludeStructure Includes = ExpectedAST.getIncludeStructure();
auto MainFE = FM.getFile(testPath("foo.cpp"));
ASSERT_TRUE(MainFE);
- auto MainID = Includes.getOrCreateID(*MainFE, SM);
+ auto MainID = Includes.getOrCreateID(*MainFE);
auto &PatchedFM = PatchedAST->getSourceManager().getFileManager();
IncludeStructure PatchedIncludes = PatchedAST->getIncludeStructure();
auto PatchedMainFE = PatchedFM.getFile(testPath("foo.cpp"));
ASSERT_TRUE(PatchedMainFE);
- auto PatchedMainID = PatchedIncludes.getOrCreateID(*PatchedMainFE, SM);
+ auto PatchedMainID = PatchedIncludes.getOrCreateID(*PatchedMainFE);
EXPECT_EQ(Includes.includeDepth(MainID)[MainID],
PatchedIncludes.includeDepth(PatchedMainID)[PatchedMainID]);
}
diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
index 6001533005c97..3d021979d179b 100644
--- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -83,7 +83,7 @@ collectPatchedIncludes(llvm::StringRef ModifiedContents,
}
IncludeStructure Includes;
Clang->getPreprocessor().addPPCallbacks(
- collectIncludeStructureCallback(Clang->getSourceManager(), &Includes));
+ Includes.collect(Clang->getSourceManager()));
if (llvm::Error Err = Action.Execute()) {
ADD_FAILURE() << "failed to execute action: " << std::move(Err);
return {};
diff --git a/llvm/include/llvm/Support/FileSystem/UniqueID.h b/llvm/include/llvm/Support/FileSystem/UniqueID.h
index 229410c8292e5..cb5889cc11c74 100644
--- a/llvm/include/llvm/Support/FileSystem/UniqueID.h
+++ b/llvm/include/llvm/Support/FileSystem/UniqueID.h
@@ -14,7 +14,9 @@
#ifndef LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
#define LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
+#include "llvm/ADT/DenseMap.h"
#include <cstdint>
+#include <utility>
namespace llvm {
namespace sys {
@@ -47,6 +49,31 @@ class UniqueID {
} // end namespace fs
} // end namespace sys
+
+// Support UniqueIDs as DenseMap keys.
+template <> struct DenseMapInfo<llvm::sys::fs::UniqueID> {
+ static inline llvm::sys::fs::UniqueID getEmptyKey() {
+ auto EmptyKey = DenseMapInfo<std::pair<unsigned, unsigned>>::getEmptyKey();
+ return {EmptyKey.first, EmptyKey.second};
+ }
+
+ static inline llvm::sys::fs::UniqueID getTombstoneKey() {
+ auto TombstoneKey =
+ DenseMapInfo<std::pair<unsigned, unsigned>>::getTombstoneKey();
+ return {TombstoneKey.first, TombstoneKey.second};
+ }
+
+ static unsigned getHashValue(const llvm::sys::fs::UniqueID &Tag) {
+ return hash_value(
+ std::pair<unsigned, unsigned>(Tag.getDevice(), Tag.getFile()));
+ }
+
+ static bool isEqual(const llvm::sys::fs::UniqueID &LHS,
+ const llvm::sys::fs::UniqueID &RHS) {
+ return LHS == RHS;
+ }
+};
+
} // end namespace llvm
#endif // LLVM_SUPPORT_FILESYSTEM_UNIQUEID_H
More information about the llvm-commits
mailing list