Nebiroth updated this revision to Diff 127161.
Nebiroth marked 27 inline comments as done.
Nebiroth added a comment.
inner class IncludeReferenceMap replaced by one map
fillRangeVector() and findPreambleIncludes() code moved into wrapper class
Open definition now returns an empty Range struct (line, character = 0)
Fixed unit tests
Minor code improvements
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D38639
Files:
clangd/ClangdServer.cpp
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/Protocol.h
unittests/clangd/ClangdTests.cpp
Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -7,7 +7,6 @@
//
//===----------------------------------------------------------------------===//
-#include "ClangdLSPServer.h"
#include "ClangdServer.h"
#include "Logger.h"
#include "TestFS.h"
@@ -749,6 +748,75 @@
EXPECT_FALSE(PathResult.hasValue());
}
+TEST_F(ClangdVFSTest, CheckDefinitionIncludes) {
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
+
+ ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+ /*StorePreamblesInMemory=*/true,
+ EmptyLogger::getInstance());
+
+ auto FooCpp = getVirtualTestFilePath("foo.cpp");
+ const auto SourceContents = R"cpp(
+ #include "foo.h"
+ #include "invalid.h"
+ int b = a;
+ // test
+ int foo;
+ #include "foo.h"
+ )cpp";
+ FS.Files[FooCpp] = SourceContents;
+ auto FooH = getVirtualTestFilePath("foo.h");
+ const auto HeaderContents = "int a;";
+
+ FS.Files[FooCpp] = SourceContents;
+ FS.Files[FooH] = HeaderContents;
+
+ Server.addDocument(FooH, HeaderContents);
+ Server.addDocument(FooCpp, SourceContents);
+
+ Position P = Position{1, 11};
+
+ auto ExpectedLocations = Server.findDefinitions(FooCpp, P);
+ ASSERT_TRUE(!!ExpectedLocations);
+ std::vector<Location> Locations = ExpectedLocations->Value;
+ EXPECT_TRUE(!Locations.empty());
+ std::string s("file:///");
+ std::string check = Locations[0].uri.uri;
+ check = check.erase(0, s.size() - 1);
+ check = check.substr(0, check.size());
+ ASSERT_EQ(check, FooH);
+ ASSERT_EQ(Locations[0].range.start.line, 0);
+ ASSERT_EQ(Locations[0].range.start.character, 0);
+ ASSERT_EQ(Locations[0].range.end.line, 0);
+ ASSERT_EQ(Locations[0].range.end.character, 0);
+
+ // Test ctrl-clicking on the #include part on the statement
+ Position P2 = Position{1, 3};
+
+ ExpectedLocations = Server.findDefinitions(FooCpp, P2);
+ ASSERT_TRUE(!!ExpectedLocations);
+ Locations = ExpectedLocations->Value;
+ EXPECT_TRUE(!Locations.empty());
+
+ // Test invalid include
+ Position P3 = Position{2, 11};
+
+ ExpectedLocations = Server.findDefinitions(FooCpp, P3);
+ ASSERT_TRUE(!!ExpectedLocations);
+ Locations = ExpectedLocations->Value;
+ EXPECT_TRUE(Locations.empty());
+
+ // Test include outside of Preamble
+ Position P4 = Position{6, 5};
+
+ ExpectedLocations = Server.findDefinitions(FooCpp, P4);
+ ASSERT_TRUE(!!ExpectedLocations);
+ Locations = ExpectedLocations->Value;
+ EXPECT_TRUE(!Locations.empty());
+}
+
TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) {
class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer {
public:
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -108,6 +108,14 @@
bool fromJSON(const json::Expr &, Range &);
json::Expr toJSON(const Range &);
+class RangeHash {
+public:
+ std::size_t operator()(const Range &R) const {
+ return ((R.start.line & 0x18) << 3) | ((R.start.character & 0x18) << 1) |
+ ((R.end.line & 0x18) >> 1) | ((R.end.character & 0x18) >> 3);
+ }
+};
+
struct Location {
/// The text document's URI.
URI uri;
Index: clangd/ClangdUnit.h
===================================================================
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -22,6 +22,7 @@
#include <future>
#include <memory>
#include <mutex>
+#include <unordered_map>
namespace llvm {
class raw_ostream;
@@ -59,6 +60,8 @@
std::vector<DiagWithFixIts> Diags;
};
+using IncludeReferenceMap = std::unordered_map<Range, Path, RangeHash>;
+
/// Stores and provides access to parsed AST.
class ParsedAST {
public:
@@ -69,7 +72,8 @@
std::shared_ptr<const PreambleData> Preamble,
std::unique_ptr<llvm::MemoryBuffer> Buffer,
std::shared_ptr<PCHContainerOperations> PCHs,
- IntrusiveRefCntPtr<vfs::FileSystem> VFS, clangd::Logger &Logger);
+ IntrusiveRefCntPtr<vfs::FileSystem> VFS, clangd::Logger &Logger,
+ IncludeReferenceMap &IRM);
ParsedAST(ParsedAST &&Other);
ParsedAST &operator=(ParsedAST &&Other);
@@ -89,12 +93,14 @@
const std::vector<DiagWithFixIts> &getDiagnostics() const;
+ IncludeReferenceMap &getIRM() { return IRM; };
+
private:
ParsedAST(std::shared_ptr<const PreambleData> Preamble,
std::unique_ptr<CompilerInstance> Clang,
std::unique_ptr<FrontendAction> Action,
std::vector<const Decl *> TopLevelDecls,
- std::vector<DiagWithFixIts> Diags);
+ std::vector<DiagWithFixIts> Diags, IncludeReferenceMap IRM);
private:
void ensurePreambleDeclsDeserialized();
@@ -114,6 +120,7 @@
std::vector<DiagWithFixIts> Diags;
std::vector<const Decl *> TopLevelDecls;
bool PreambleDeclsDeserialized;
+ IncludeReferenceMap IRM;
};
// Provides thread-safe access to ParsedAST.
@@ -256,14 +263,14 @@
clangd::Logger &Logger;
};
-
/// Get the beginning SourceLocation at a specified \p Pos.
SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos,
const FileEntry *FE);
/// Get definition of symbol at a specified \p Pos.
-std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos,
- clangd::Logger &Logger);
+std::vector<Location>
+findDefinitions(ParsedAST &AST, Position Pos, clangd::Logger &Logger,
+ std::unordered_map<Range, Path, RangeHash> IncludeLocationMap);
/// For testing/debugging purposes. Note that this method deserializes all
/// unserialized Decls, so use with care.
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -29,9 +29,6 @@
#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/Format.h"
-#include <algorithm>
-#include <chrono>
-
using namespace clang::clangd;
using namespace clang;
@@ -73,12 +70,17 @@
std::vector<const Decl *> TopLevelDecls;
};
-class CppFilePreambleCallbacks : public PreambleCallbacks {
+class CppFilePreambleCallbacks : public PreambleCallbacks, public PPCallbacks {
public:
std::vector<serialization::DeclID> takeTopLevelDeclIDs() {
return std::move(TopLevelDeclIDs);
}
+ CppFilePreambleCallbacks(SourceManager *SourceMgr, IncludeReferenceMap &IRM)
+ : SourceMgr(SourceMgr), IRM(IRM) {}
+
+ IncludeReferenceMap &getIRM() { return IRM; };
+
void AfterPCHEmitted(ASTWriter &Writer) override {
TopLevelDeclIDs.reserve(TopLevelDecls.size());
for (Decl *D : TopLevelDecls) {
@@ -97,9 +99,80 @@
}
}
+ void AfterExecute(CompilerInstance &CI) override {
+
+ SourceMgr = &CI.getSourceManager();
+ for (auto InclusionLoc : TempPreambleIncludeLocations)
+ addIncludeLocation(InclusionLoc);
+ }
+
+ void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+ StringRef FileName, bool IsAngled,
+ CharSourceRange FilenameRange, const FileEntry *File,
+ StringRef SearchPath, StringRef RelativePath,
+ const Module *Imported) override {
+ auto SR = FilenameRange.getAsRange();
+ if (SR.isInvalid() || !File || File->tryGetRealPathName().empty())
+ return;
+
+ if (SourceMgr) {
+ addIncludeLocation({SR, File->tryGetRealPathName()});
+ } else {
+ // When we are processing the inclusion directives inside the preamble,
+ // we don't have access to a SourceManager, so we cannot convert
+ // SourceRange to Range. This is done instead in AfterExecute when a
+ // SourceManager becomes available.
+ TempPreambleIncludeLocations.push_back({SR, File->tryGetRealPathName()});
+ }
+ }
+
+ void addIncludeLocation(std::pair<SourceRange, std::string> InclusionLoc) {
+ // Only inclusion directives in the main file make sense. The user cannot
+ // select directives not in the main file.
+ if (SourceMgr->getMainFileID() == SourceMgr->getFileID(InclusionLoc.first.getBegin()) && SourceMgr->isInMainFile(InclusionLoc.first.getBegin()))
+ IRM.insert({getRange(InclusionLoc.first), InclusionLoc.second});
+ }
+
+ Range getRange(SourceRange SR) {
+ Position Begin;
+ Begin.line = SourceMgr->getSpellingLineNumber(SR.getBegin());
+ Begin.character = SourceMgr->getSpellingColumnNumber(SR.getBegin());
+ Position End;
+ End.line = SourceMgr->getSpellingLineNumber(SR.getEnd());
+ End.character = SourceMgr->getSpellingColumnNumber(SR.getEnd());
+ return {Begin, End};
+ }
+
+ std::unique_ptr<PPCallbacks> createPPCallbacks() {
+ class DelegatingPPCallbacks : public PPCallbacks {
+ public:
+ DelegatingPPCallbacks(CppFilePreambleCallbacks &PPCallbacks) : Collector(PPCallbacks) {}
+
+ void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+ StringRef FileName, bool IsAngled,
+ CharSourceRange FilenameRange, const FileEntry *File,
+ StringRef SearchPath, StringRef RelativePath,
+ const Module *Imported) override {
+ Collector.InclusionDirective(HashLoc, IncludeTok, FileName, IsAngled,
+ FilenameRange, File, SearchPath, RelativePath,
+ Imported);
+ }
+
+ private:
+ CppFilePreambleCallbacks &Collector;
+ };
+
+ std::unique_ptr<DelegatingPPCallbacks> DelegatedPPCallbacks =
+ llvm::make_unique<DelegatingPPCallbacks>(*this);
+ return DelegatedPPCallbacks;
+ }
+
private:
std::vector<Decl *> TopLevelDecls;
std::vector<serialization::DeclID> TopLevelDeclIDs;
+ SourceManager *SourceMgr;
+ IncludeReferenceMap &IRM;
+ std::vector<std::pair<SourceRange, std::string>> TempPreambleIncludeLocations;
};
/// Convert from clang diagnostic level to LSP severity.
@@ -171,7 +244,7 @@
std::unique_ptr<llvm::MemoryBuffer> Buffer,
std::shared_ptr<PCHContainerOperations> PCHs,
IntrusiveRefCntPtr<vfs::FileSystem> VFS,
- clangd::Logger &Logger) {
+ clangd::Logger &Logger, IncludeReferenceMap &IRM) {
std::vector<DiagWithFixIts> ASTDiags;
StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags);
@@ -193,16 +266,22 @@
MainInput.getFile());
return llvm::None;
}
+
+ CppFilePreambleCallbacks SerializedDeclsCollector(&Clang->getSourceManager(),
+ IRM);
+
+ Clang->getPreprocessor().addPPCallbacks(SerializedDeclsCollector.createPPCallbacks());
+
if (!Action->Execute())
Logger.log("Execute() failed when building AST for " + MainInput.getFile());
-
// UnitDiagsConsumer is local, we can not store it in CompilerInstance that
// has a longer lifetime.
Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
std::vector<const Decl *> ParsedDecls = Action->takeTopLevelDecls();
return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
- std::move(ParsedDecls), std::move(ASTDiags));
+ std::move(ParsedDecls), std::move(ASTDiags),
+ std::move(std::move(SerializedDeclsCollector.getIRM())));
}
namespace {
@@ -227,12 +306,15 @@
const SourceLocation &SearchedLocation;
const ASTContext &AST;
Preprocessor &PP;
+ std::unordered_map<Range, Path, RangeHash> IncludeLocationMap;
public:
- DeclarationLocationsFinder(raw_ostream &OS,
- const SourceLocation &SearchedLocation,
- ASTContext &AST, Preprocessor &PP)
- : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
+ DeclarationLocationsFinder(
+ raw_ostream &OS, const SourceLocation &SearchedLocation, ASTContext &AST,
+ Preprocessor &PP,
+ std::unordered_map<Range, Path, RangeHash> IncludeLocationMap)
+ : SearchedLocation(SearchedLocation), AST(AST), PP(PP),
+ IncludeLocationMap(IncludeLocationMap) {}
std::vector<Location> takeLocations() {
// Don't keep the same location multiple times.
@@ -262,6 +344,11 @@
SourceMgr.getFileID(SearchedLocation) == FID;
}
+ bool isSameLine(unsigned Line) const {
+ const SourceManager &SourceMgr = AST.getSourceManager();
+ return Line == SourceMgr.getSpellingLineNumber(SearchedLocation);
+ }
+
void addDeclarationLocation(const SourceRange &ValSourceRange) {
const SourceManager &SourceMgr = AST.getSourceManager();
const LangOptions &LangOpts = AST.getLangOpts();
@@ -275,19 +362,28 @@
End.line = SourceMgr.getSpellingLineNumber(LocEnd) - 1;
End.character = SourceMgr.getSpellingColumnNumber(LocEnd) - 1;
Range R = {Begin, End};
+ addLocation(URI::fromFile(
+ SourceMgr.getFilename(SourceMgr.getSpellingLoc(LocStart))),
+ R);
+ }
+
+ void addLocation(URI Uri, Range R) {
Location L;
- if (const FileEntry *F =
- SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart))) {
- StringRef FilePath = F->tryGetRealPathName();
- if (FilePath.empty())
- FilePath = F->getName();
- L.uri = URI::fromFile(FilePath);
- L.range = R;
- DeclarationLocations.push_back(L);
- }
+ L.uri = Uri;
+ L.range = R;
+ DeclarationLocations.push_back(L);
}
void finish() override {
+
+ for (auto &IncludeLoc : IncludeLocationMap) {
+ Range R = IncludeLoc.first;
+ if (isSameLine(R.start.line)) {
+ addLocation(URI::fromFile(IncludeLoc.second), Range{Position{0,0}, Position{0,0}});
+ return;
+ }
+ }
+
// Also handle possible macro at the searched location.
Token Result;
if (!Lexer::getRawToken(SearchedLocation, Result, AST.getSourceManager(),
@@ -319,8 +415,9 @@
} // namespace
-std::vector<Location> clangd::findDefinitions(ParsedAST &AST, Position Pos,
- clangd::Logger &Logger) {
+std::vector<Location> clangd::findDefinitions(
+ ParsedAST &AST, Position Pos, clangd::Logger &Logger,
+ std::unordered_map<Range, Path, RangeHash> IncludeLocationMap) {
const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
if (!FE)
@@ -330,7 +427,7 @@
auto DeclLocationsFinder = std::make_shared<DeclarationLocationsFinder>(
llvm::errs(), SourceLocationBeg, AST.getASTContext(),
- AST.getPreprocessor());
+ AST.getPreprocessor(), IncludeLocationMap);
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -405,11 +502,11 @@
std::unique_ptr<CompilerInstance> Clang,
std::unique_ptr<FrontendAction> Action,
std::vector<const Decl *> TopLevelDecls,
- std::vector<DiagWithFixIts> Diags)
+ std::vector<DiagWithFixIts> Diags, IncludeReferenceMap IRM)
: Preamble(std::move(Preamble)), Clang(std::move(Clang)),
Action(std::move(Action)), Diags(std::move(Diags)),
- TopLevelDecls(std::move(TopLevelDecls)),
- PreambleDeclsDeserialized(false) {
+ TopLevelDecls(std::move(TopLevelDecls)), PreambleDeclsDeserialized(false),
+ IRM(std::move(IRM)) {
assert(this->Clang);
assert(this->Action);
}
@@ -565,6 +662,7 @@
}
assert(CI && "Couldn't create CompilerInvocation");
+ IncludeReferenceMap IRM;
std::unique_ptr<llvm::MemoryBuffer> ContentsBuffer =
llvm::MemoryBuffer::getMemBufferCopy(NewContents, That->FileName);
@@ -590,7 +688,8 @@
IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
CompilerInstance::createDiagnostics(
&CI->getDiagnosticOpts(), &PreambleDiagnosticsConsumer, false);
- CppFilePreambleCallbacks SerializedDeclsCollector;
+
+ CppFilePreambleCallbacks SerializedDeclsCollector(nullptr, IRM);
auto BuiltPreamble = PrecompiledPreamble::Build(
*CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs,
/*StoreInMemory=*/That->StorePreamblesInMemory,
@@ -631,9 +730,9 @@
{
trace::Span Tracer("Build");
SPAN_ATTACH(Tracer, "File", That->FileName);
- NewAST =
- ParsedAST::Build(std::move(CI), std::move(NewPreamble),
- std::move(ContentsBuffer), PCHs, VFS, That->Logger);
+ NewAST = ParsedAST::Build(
+ std::move(CI), std::move(NewPreamble), std::move(ContentsBuffer),
+ PCHs, VFS, That->Logger, IRM);
}
if (NewAST) {
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -22,6 +22,7 @@
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
#include <future>
+#include <unordered_map>
using namespace clang;
using namespace clang::clangd;
@@ -442,10 +443,16 @@
llvm::errc::invalid_argument);
std::vector<Location> Result;
- Resources->getAST().get()->runUnderLock([Pos, &Result, this](ParsedAST *AST) {
+ std::shared_future<std::shared_ptr<const PreambleData>> Preamble =
+ Resources->getPreamble();
+
+ Resources->getAST().get()->runUnderLock([Pos, &Result, Preamble,
+ this](ParsedAST *AST) {
if (!AST)
return;
- Result = clangd::findDefinitions(*AST, Pos, Logger);
+
+ IncludeReferenceMap &IRM = AST->getIRM();
+ Result = clangd::findDefinitions(*AST, Pos, Logger, IRM);
});
return make_tagged(std::move(Result), TaggedFS.Tag);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
