Nebiroth updated this revision to Diff 127386.
Nebiroth marked 14 inline comments as done.
Nebiroth added a comment.
Herald added a subscriber: mgrang.
CppFilePreambleCallbacks no longer extends PPCallbacks
CppFilePreambleCallbacks no longer needs SourceManager parameter
Removed temporary vector TempPreambleIncludeLocations
IncludeReferenceMap in ParsedAST no longer passed by reference
Code handling includes outside of preambles is now separated in
IncludeRefsCollector class
Removed addLocation
Changed isSameLine function name and now checks if searched location is in
range instead of verifying the line
Removed changes to findDefinitions interface
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 "Context.h"
#include "TestFS.h"
@@ -751,6 +750,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
@@ -23,6 +23,7 @@
#include <future>
#include <memory>
#include <mutex>
+#include <unordered_map>
namespace llvm {
class raw_ostream;
@@ -58,17 +59,22 @@
std::vector<DiagWithFixIts> Diags;
};
+using IncludeReferenceMap = std::unordered_map<Range, Path, RangeHash>;
+
/// Stores and provides access to parsed AST.
class ParsedAST {
public:
/// Attempts to run Clang and store parsed AST. If \p Preamble is non-null
/// it is reused during parsing.
static llvm::Optional<ParsedAST>
- Build(const Context &Ctx, std::unique_ptr<clang::CompilerInvocation> CI,
- std::shared_ptr<const PreambleData> Preamble,
- std::unique_ptr<llvm::MemoryBuffer> Buffer,
- std::shared_ptr<PCHContainerOperations> PCHs,
- IntrusiveRefCntPtr<vfs::FileSystem> VFS);
+ Build(const Context &Ctx,
+ std::unique_ptr<clang::CompilerInvocation> CI,
+ std::shared_ptr<const PreambleData> Preamble,
+ std::unique_ptr<llvm::MemoryBuffer> Buffer,
+ std::shared_ptr<PCHContainerOperations> PCHs,
+ IntrusiveRefCntPtr<vfs::FileSystem> VFS,
+ IncludeReferenceMap IRM);
+
ParsedAST(ParsedAST &&Other);
ParsedAST &operator=(ParsedAST &&Other);
@@ -88,12 +94,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();
@@ -113,6 +121,7 @@
std::vector<DiagWithFixIts> Diags;
std::vector<const Decl *> TopLevelDecls;
bool PreambleDeclsDeserialized;
+ IncludeReferenceMap IRM;
};
// Provides thread-safe access to ParsedAST.
@@ -258,13 +267,14 @@
SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos,
const FileEntry *FE);
-/// Get definition of symbol at a specified \p Pos.
-std::vector<Location> findDefinitions(const Context &Ctx, ParsedAST &AST,
- Position Pos);
-
std::vector<DocumentHighlight>
findDocumentHighlights(const Context &Ctx, ParsedAST &AST, Position Pos);
+
+
+std::vector<Location>
+findDefinitions(const Context &Ctx, ParsedAST &AST, Position Pos);
+
/// For testing/debugging purposes. Note that this method deserializes all
/// unserialized Decls, so use with care.
void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS);
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -27,8 +27,6 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/Format.h"
-#include <algorithm>
-#include <chrono>
using namespace clang::clangd;
using namespace clang;
@@ -71,12 +69,50 @@
std::vector<const Decl *> TopLevelDecls;
};
+class IncludeRefsCollector : public PPCallbacks {
+public:
+ IncludeRefsCollector(SourceManager &SourceMgr, IncludeReferenceMap &IRM) : SourceMgr(SourceMgr), IRM(IRM) {}
+
+ 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.getMainFileID() == SourceMgr.getFileID(FilenameRange.getAsRange().getBegin()) && SourceMgr.isInMainFile(FilenameRange.getAsRange().getBegin())) {
+ // Only inclusion directives in the main file make sense. The user cannot
+ // select directives not in the main file.
+ IRM.insert({getRange(FilenameRange.getAsRange()), File->tryGetRealPathName()});
+ }
+ }
+
+ 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};
+ }
+
+private:
+ SourceManager &SourceMgr;
+ IncludeReferenceMap &IRM;
+};
+
class CppFilePreambleCallbacks : public PreambleCallbacks {
public:
std::vector<serialization::DeclID> takeTopLevelDeclIDs() {
return std::move(TopLevelDeclIDs);
}
+ CppFilePreambleCallbacks(IncludeReferenceMap &IRM)
+ : IRM(IRM) {}
+
void AfterPCHEmitted(ASTWriter &Writer) override {
TopLevelDeclIDs.reserve(TopLevelDecls.size());
for (Decl *D : TopLevelDecls) {
@@ -95,9 +131,20 @@
}
}
+ void BeforeExecute(CompilerInstance &CI) override {
+ SourceMgr = &CI.getSourceManager();
+ }
+
+ std::unique_ptr<PPCallbacks> createPPCallbacks() {
+ return llvm::make_unique<IncludeRefsCollector>(*SourceMgr, IRM);
+ }
+
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.
@@ -230,7 +277,8 @@
std::shared_ptr<const PreambleData> Preamble,
std::unique_ptr<llvm::MemoryBuffer> Buffer,
std::shared_ptr<PCHContainerOperations> PCHs,
- IntrusiveRefCntPtr<vfs::FileSystem> VFS) {
+ IntrusiveRefCntPtr<vfs::FileSystem> VFS,
+ IncludeReferenceMap IRM) {
std::vector<DiagWithFixIts> ASTDiags;
StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags);
@@ -249,19 +297,22 @@
const FrontendInputFile &MainInput = Clang->getFrontendOpts().Inputs[0];
if (!Action->BeginSourceFile(*Clang, MainInput)) {
log(Ctx, "BeginSourceFile() failed when building AST for " +
- MainInput.getFile());
+ MainInput.getFile());
return llvm::None;
}
+
+ Clang->getPreprocessor().addPPCallbacks(llvm::make_unique<IncludeRefsCollector>(Clang->getSourceManager(), IRM));
+
if (!Action->Execute())
log(Ctx, "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(IRM));
}
namespace {
@@ -284,23 +335,34 @@
class DeclarationAndMacrosFinder : public index::IndexDataConsumer {
std::vector<const Decl *> Decls;
std::vector<const MacroInfo *> MacroInfos;
+ std::vector<Location> DeclarationLocations;
const SourceLocation &SearchedLocation;
const ASTContext &AST;
Preprocessor &PP;
+ IncludeReferenceMap IncludeLocationMap;
public:
DeclarationAndMacrosFinder(raw_ostream &OS,
const SourceLocation &SearchedLocation,
- ASTContext &AST, Preprocessor &PP)
- : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
+ ASTContext &AST, Preprocessor &PP, const IncludeReferenceMap &IncludeLocationMap)
+ : SearchedLocation(SearchedLocation), AST(AST), PP(PP), IncludeLocationMap(IncludeLocationMap) {}
std::vector<const Decl *> takeDecls() {
- // Don't keep the same declaration multiple times.
+ // Don't keep the same declaration multiple times.
+ // This can happen when nodes in the AST are visited twice.
+ std::sort(Decls.begin(), Decls.end());
+ auto Last = std::unique(Decls.begin(), Decls.end());
+ Decls.erase(Last, Decls.end());
+ return std::move(Decls);
+ }
+
+ std::vector<Location> takeLocations() {
+ // Don't keep the same location multiple times.
// This can happen when nodes in the AST are visited twice.
- std::sort(Decls.begin(), Decls.end());
- auto Last = std::unique(Decls.begin(), Decls.end());
- Decls.erase(Last, Decls.end());
- return std::move(Decls);
+ std::sort(DeclarationLocations.begin(), DeclarationLocations.end());
+ auto Last = std::unique(DeclarationLocations.begin(), DeclarationLocations.end());
+ DeclarationLocations.erase(Last, DeclarationLocations.end());
+ return std::move(DeclarationLocations);
}
std::vector<const MacroInfo *> takeMacroInfos() {
@@ -328,7 +390,43 @@
SourceMgr.getFileID(SearchedLocation) == FID;
}
+
+ bool isInRange(Range R) const {
+ const SourceManager &SourceMgr = AST.getSourceManager();
+ unsigned CharNumber = SourceMgr.getSpellingColumnNumber(SearchedLocation);
+ return R.start.line == SourceMgr.getSpellingLineNumber(SearchedLocation) &&
+ (R.start.character >= CharNumber && CharNumber <= R.end.character);
+
+ }
+
+ void addDeclarationLocation(const SourceRange &ValSourceRange) {
+ const SourceManager &SourceMgr = AST.getSourceManager();
+ const LangOptions &LangOpts = AST.getLangOpts();
+ SourceLocation LocStart = ValSourceRange.getBegin();
+ SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(),
+ 0, SourceMgr, LangOpts);
+ Position Begin;
+ Begin.line = SourceMgr.getSpellingLineNumber(LocStart) - 1;
+ Begin.character = SourceMgr.getSpellingColumnNumber(LocStart) - 1;
+ Position End;
+ End.line = SourceMgr.getSpellingLineNumber(LocEnd) - 1;
+ End.character = SourceMgr.getSpellingColumnNumber(LocEnd) - 1;
+ Range R = {Begin, End};
+ DeclarationLocations.push_back(Location{URI::fromFile(
+ SourceMgr.getFilename(SourceMgr.getSpellingLoc(LocStart))), R});
+ }
+
+
void finish() override {
+ for (auto &IncludeLoc : IncludeLocationMap) {
+ Range R = IncludeLoc.first;
+
+ if (isInRange(R)) {
+ DeclarationLocations.push_back(Location{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(),
@@ -466,7 +564,7 @@
auto DeclMacrosFinder = std::make_shared<DeclarationAndMacrosFinder>(
llvm::errs(), SourceLocationBeg, AST.getASTContext(),
- AST.getPreprocessor());
+ AST.getPreprocessor(), AST.getIRM());
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -493,22 +591,27 @@
Result.push_back(*L);
}
- return Result;
+ if (Result.empty())
+ return DeclMacrosFinder->takeLocations();
+ else
+ return Result;
}
std::vector<DocumentHighlight>
clangd::findDocumentHighlights(const Context &Ctx, ParsedAST &AST,
Position Pos) {
+
const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
if (!FE)
return {};
SourceLocation SourceLocationBeg = getBeginningOfIdentifier(AST, Pos, FE);
+ IncludeReferenceMap IRM;
auto DeclMacrosFinder = std::make_shared<DeclarationAndMacrosFinder>(
llvm::errs(), SourceLocationBeg, AST.getASTContext(),
- AST.getPreprocessor());
+ AST.getPreprocessor(), IRM);
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -592,11 +695,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);
}
@@ -750,6 +853,7 @@
}
assert(CI && "Couldn't create CompilerInvocation");
+ IncludeReferenceMap IRM;
std::unique_ptr<llvm::MemoryBuffer> ContentsBuffer =
llvm::MemoryBuffer::getMemBufferCopy(NewContents, That->FileName);
@@ -775,7 +879,9 @@
IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
CompilerInstance::createDiagnostics(
&CI->getDiagnosticOpts(), &PreambleDiagnosticsConsumer, false);
- CppFilePreambleCallbacks SerializedDeclsCollector;
+
+ CppFilePreambleCallbacks SerializedDeclsCollector(IRM);
+
auto BuiltPreamble = PrecompiledPreamble::Build(
*CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs,
/*StoreInMemory=*/That->StorePreamblesInMemory,
@@ -816,10 +922,9 @@
{
trace::Span Tracer(Ctx, "Build");
SPAN_ATTACH(Tracer, "File", That->FileName);
- NewAST = ParsedAST::Build(Ctx, std::move(CI), std::move(NewPreamble),
- std::move(ContentsBuffer), PCHs, VFS);
+ NewAST = ParsedAST::Build(Ctx,
+ std::move(CI), std::move(NewPreamble), std::move(ContentsBuffer), PCHs, VFS, IRM);
}
-
if (NewAST) {
Diagnostics.insert(Diagnostics.end(), NewAST->getDiagnostics().begin(),
NewAST->getDiagnostics().end());
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;
@@ -425,7 +426,8 @@
llvm::errc::invalid_argument);
std::vector<Location> Result;
- Resources->getAST().get()->runUnderLock([Pos, &Result, &Ctx](ParsedAST *AST) {
+ Resources->getAST().get()->runUnderLock([Pos, &Result, &Ctx,
+ this](ParsedAST *AST) {
if (!AST)
return;
Result = clangd::findDefinitions(Ctx, *AST, Pos);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
