Nebiroth updated this revision to Diff 127900.
Nebiroth marked 11 inline comments as done.
Nebiroth added a comment.
Minor code cleanup
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D38639
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/Protocol.h
clangd/XRefs.cpp
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/XRefs.cpp
===================================================================
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -18,6 +18,7 @@
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;
@@ -37,14 +38,26 @@
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(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() {
// Don't keep the same Macro info multiple times.
std::sort(MacroInfos.begin(), MacroInfos.end());
auto Last = std::unique(MacroInfos.begin(), MacroInfos.end());
MacroInfos.erase(Last, MacroInfos.end());
return std::move(MacroInfos);
}
+ const SourceLocation &getSearchedLocation() { return SearchedLocation; };
+
bool
handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
ArrayRef<index::SymbolRelation> Relations, FileID FID,
@@ -62,6 +75,25 @@
SourceMgr.getFileID(SearchedLocation) == FID;
}
+ 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 {
// Also handle possible macro at the searched location.
Token Result;
@@ -142,6 +174,23 @@
indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
DeclMacrosFinder, IndexOpts);
+ /// Process targets for paths inside #include directive.
+ std::vector<Location> IncludeTargets;
+ for (auto &IncludeLoc : AST.getIRM()) {
+ Range R = IncludeLoc.first;
+ const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
+ unsigned CharNumber = SourceMgr.getSpellingColumnNumber(SourceLocationBeg);
+
+ if ((unsigned)R.start.line ==
+ SourceMgr.getSpellingLineNumber(SourceLocationBeg) &&
+ ((unsigned)R.start.character <= CharNumber &&
+ CharNumber <= (unsigned)R.end.character)) {
+ IncludeTargets.push_back(Location{URI::fromFile(IncludeLoc.second),
+ Range{Position{0, 0}, Position{0, 0}}});
+ return IncludeTargets;
+ }
+ }
+
std::vector<const Decl *> Decls = DeclMacrosFinder->takeDecls();
std::vector<const MacroInfo *> MacroInfos =
DeclMacrosFinder->takeMacroInfos();
@@ -160,7 +209,10 @@
Result.push_back(*L);
}
- return Result;
+ if (Result.empty())
+ return DeclMacrosFinder->takeLocations();
+ else
+ return Result;
}
namespace {
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -112,6 +112,16 @@
};
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);
+ }
+};
+bool fromJSON(const json::Expr &, Range &);
+json::Expr toJSON(const Range &);
llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Range &);
struct Location {
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,6 +59,8 @@
std::vector<DiagWithFixIts> Diags;
};
+using IncludeReferenceMap = std::unordered_map<Range, Path, RangeHash>;
+
/// Stores and provides access to parsed AST.
class ParsedAST {
public:
@@ -68,7 +71,7 @@
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);
ParsedAST(ParsedAST &&Other);
ParsedAST &operator=(ParsedAST &&Other);
@@ -88,12 +91,14 @@
const std::vector<DiagWithFixIts> &getDiagnostics() const;
+ const IncludeReferenceMap &getIRM() const { 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 +118,7 @@
std::vector<DiagWithFixIts> Diags;
std::vector<const Decl *> TopLevelDecls;
bool PreambleDeclsDeserialized;
+ IncludeReferenceMap IRM;
};
// Provides thread-safe access to ParsedAST.
@@ -266,6 +272,13 @@
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);
+
/// 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,52 @@
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.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)
+ : SourceMgr(nullptr), IRM(IRM) {}
+
void AfterPCHEmitted(ASTWriter &Writer) override {
TopLevelDeclIDs.reserve(TopLevelDecls.size());
for (Decl *D : TopLevelDecls) {
@@ -95,9 +133,21 @@
}
}
+ void BeforeExecute(CompilerInstance &CI) override {
+ SourceMgr = &CI.getSourceManager();
+ }
+
+ std::unique_ptr<PPCallbacks> createPPCallbacks() {
+ assert(SourceMgr && "SourceMgr must be set at this point");
+ 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.
@@ -224,13 +274,12 @@
AST.getASTContext().getTranslationUnitDecl()->dump(OS, true);
}
-llvm::Optional<ParsedAST>
-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) {
+llvm::Optional<ParsedAST> 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, IncludeReferenceMap IRM) {
std::vector<DiagWithFixIts> ASTDiags;
StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags);
@@ -252,16 +301,19 @@
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 {
@@ -272,8 +324,6 @@
Mgr.translateFileLineCol(FE, Pos.line + 1, Pos.character + 1);
return Mgr.getMacroArgExpandedLocation(InputLoc);
}
-
-
} // namespace
void ParsedAST::ensurePreambleDeclsDeserialized() {
@@ -339,11 +389,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);
}
@@ -501,6 +551,7 @@
}
assert(CI && "Couldn't create CompilerInvocation");
+ IncludeReferenceMap IRM;
std::unique_ptr<llvm::MemoryBuffer> ContentsBuffer =
llvm::MemoryBuffer::getMemBufferCopy(NewContents, That->FileName);
@@ -529,7 +580,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,
@@ -576,9 +629,8 @@
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);
+ std::move(ContentsBuffer), PCHs, VFS, IRM);
}
-
if (NewAST) {
Diagnostics.insert(Diagnostics.end(), NewAST->getDiagnostics().begin(),
NewAST->getDiagnostics().end());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
