Nebiroth updated this revision to Diff 118046.
Nebiroth added a comment.
Fixed accidental removal of CheckSourceHeaderSwitch test
https://reviews.llvm.org/D38639
Files:
clangd/ClangdServer.cpp
clangd/ClangdUnit.cpp
clangd/ClangdUnit.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 "clang/Basic/VirtualFileSystem.h"
@@ -978,6 +977,57 @@
EXPECT_FALSE(PathResult.hasValue());
}
+TEST_F(ClangdVFSTest, CheckDefinitionIncludes) {
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
+
+ ClangdServer Server(CDB, DiagConsumer, FS, 0,
+ /*SnippetCompletions=*/false, EmptyLogger::getInstance());
+
+ auto FooCpp = getVirtualTestFilePath("foo.cpp");
+ const auto SourceContents = R"cpp(
+ #include "foo.h"
+ #include "invalid.h"
+ int b = a;
+ )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};
+
+ std::vector<Location> Locations = (Server.findDefinitions(FooCpp, P)).Value;
+ EXPECT_TRUE(!Locations.empty());
+ std::string s("file:///");
+ std::string check = URI::unparse(Locations[0].uri);
+ check = check.erase(0, s.size());
+ check = check.substr(0, check.size() - 1);
+ 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 P3 = Position{1, 3};
+
+ Locations = (Server.findDefinitions(FooCpp, P3)).Value;
+ EXPECT_TRUE(!Locations.empty());
+
+ // Test invalid include
+ Position P2 = Position{2, 11};
+
+ Locations = (Server.findDefinitions(FooCpp, P2)).Value;
+ EXPECT_TRUE(Locations.empty());
+}
+
TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) {
class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer {
public:
Index: clangd/ClangdUnit.h
===================================================================
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -127,11 +127,13 @@
struct PreambleData {
PreambleData(PrecompiledPreamble Preamble,
std::vector<serialization::DeclID> TopLevelDeclIDs,
- std::vector<DiagWithFixIts> Diags);
+ std::vector<DiagWithFixIts> Diags,
+ std::map<SourceLocation, std::string> IncludeMap);
PrecompiledPreamble Preamble;
std::vector<serialization::DeclID> TopLevelDeclIDs;
std::vector<DiagWithFixIts> Diags;
+ std::map<SourceLocation, std::string> IncludeMap;
};
/// Manages resources, required by clangd. Allows to rebuild file with new
@@ -261,7 +263,8 @@
/// Get definition of symbol at a specified \p Pos.
std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos,
- clangd::Logger &Logger);
+ clangd::Logger &Logger,
+ std::map<SourceLocation, std::string>);
/// 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
@@ -78,6 +78,10 @@
return std::move(TopLevelDeclIDs);
}
+ std::map<SourceLocation, std::string> takeIncludeMap() {
+ return std::move(IncludeMap);
+ }
+
void AfterPCHEmitted(ASTWriter &Writer) override {
TopLevelDeclIDs.reserve(TopLevelDecls.size());
for (Decl *D : TopLevelDecls) {
@@ -96,9 +100,55 @@
}
}
+ void AfterExecute(CompilerInstance &CI) override {
+ const SourceManager &SM = CI.getSourceManager();
+ unsigned n = SM.local_sloc_entry_size();
+ SmallVector<SourceLocation, 10> InclusionStack;
+ std::map<SourceLocation, std::string>::iterator it = IncludeMap.begin();
+
+ for (unsigned i = 0; i < n; ++i) {
+ bool Invalid = false;
+ const SrcMgr::SLocEntry &SL = SM.getLocalSLocEntry(i, &Invalid);
+ if (!SL.isFile() || Invalid)
+ continue;
+ const SrcMgr::FileInfo &FI = SL.getFile();
+ SourceLocation L = FI.getIncludeLoc();
+ InclusionStack.clear();
+
+ SourceLocation LocationToInsert;
+
+ while (L.isValid()) {
+ PresumedLoc PLoc = SM.getPresumedLoc(L);
+ InclusionStack.push_back(L);
+ L = PLoc.isValid() ? PLoc.getIncludeLoc() : SourceLocation();
+ }
+ if (InclusionStack.size() == 0) {
+ // Skip main file
+ continue;
+ }
+
+ if (InclusionStack.size() > 1) {
+ // Don't care about includes of includes
+ continue;
+ }
+
+ StringRef FilePath =
+ FI.getContentCache()->OrigEntry->tryGetRealPathName();
+ if (FilePath.empty()) {
+ // FIXME: Does tryGetRealPathName return empty if and only if the path
+ // to the include doesn't exist? If that's the case we should skip this
+ // include.
+ FilePath = FI.getContentCache()->OrigEntry->getName();
+ }
+ IncludeMap.insert(std::pair<SourceLocation, std::string>(
+ InclusionStack.front(), FilePath.str()));
+ }
+ }
+
private:
std::vector<Decl *> TopLevelDecls;
std::vector<serialization::DeclID> TopLevelDeclIDs;
+ std::map<SourceLocation, std::string> IncludeMap;
};
/// Convert from clang diagnostic level to LSP severity.
@@ -709,12 +759,15 @@
const SourceLocation &SearchedLocation;
const ASTContext &AST;
Preprocessor &PP;
+ std::map<SourceLocation, std::string> IncludeMap;
public:
DeclarationLocationsFinder(raw_ostream &OS,
const SourceLocation &SearchedLocation,
- ASTContext &AST, Preprocessor &PP)
- : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
+ ASTContext &AST, Preprocessor &PP,
+ std::map<SourceLocation, std::string> IncludeMap)
+ : SearchedLocation(SearchedLocation), AST(AST), PP(PP),
+ IncludeMap(IncludeMap) {}
std::vector<Location> takeLocations() {
// Don't keep the same location multiple times.
@@ -744,7 +797,13 @@
SourceMgr.getFileID(SearchedLocation) == FID;
}
- void addDeclarationLocation(const SourceRange &ValSourceRange) {
+ bool isSameLine(unsigned line) const {
+ const SourceManager &SourceMgr = AST.getSourceManager();
+ return line == SourceMgr.getSpellingLineNumber(SearchedLocation);
+ }
+
+ void addDeclarationLocation(const SourceRange &ValSourceRange,
+ bool test = false) {
const SourceManager &SourceMgr = AST.getSourceManager();
const LangOptions &LangOpts = AST.getLangOpts();
SourceLocation LocStart = ValSourceRange.getBegin();
@@ -757,14 +816,30 @@
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;
- L.uri = URI::fromFile(
- SourceMgr.getFilename(SourceMgr.getSpellingLoc(LocStart)));
+ L.uri = uri;
L.range = R;
DeclarationLocations.push_back(L);
}
void finish() override {
+
+ for (auto it = IncludeMap.begin(); it != IncludeMap.end(); ++it) {
+ SourceLocation L = it->first;
+ std::string &Path = it->second;
+ Range r = Range();
+ unsigned line = AST.getSourceManager().getSpellingLineNumber(L);
+ if (isSameLine(line))
+ addLocation(URI::fromFile(Path), Range());
+ }
+
// Also handle possible macro at the searched location.
Token Result;
if (!Lexer::getRawToken(SearchedLocation, Result, AST.getSourceManager(),
@@ -834,8 +909,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::map<SourceLocation, std::string> IncludeMap) {
const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
if (!FE)
@@ -845,7 +921,7 @@
auto DeclLocationsFinder = std::make_shared<DeclarationLocationsFinder>(
llvm::errs(), SourceLocationBeg, AST.getASTContext(),
- AST.getPreprocessor());
+ AST.getPreprocessor(), IncludeMap);
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -929,9 +1005,11 @@
PreambleData::PreambleData(PrecompiledPreamble Preamble,
std::vector<serialization::DeclID> TopLevelDeclIDs,
- std::vector<DiagWithFixIts> Diags)
+ std::vector<DiagWithFixIts> Diags,
+ std::map<SourceLocation, std::string> IncludeMap)
: Preamble(std::move(Preamble)),
- TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)) {}
+ TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)),
+ IncludeMap(std::move(IncludeMap)) {}
std::shared_ptr<CppFile>
CppFile::Create(PathRef FileName, tooling::CompileCommand Command,
@@ -1093,7 +1171,8 @@
return std::make_shared<PreambleData>(
std::move(*BuiltPreamble),
SerializedDeclsCollector.takeTopLevelDeclIDs(),
- std::move(PreambleDiags));
+ std::move(PreambleDiags),
+ SerializedDeclsCollector.takeIncludeMap());
} else {
return nullptr;
}
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -287,11 +287,18 @@
assert(Resources && "Calling findDefinitions on non-added file");
std::vector<Location> Result;
- Resources->getAST().get()->runUnderLock([Pos, &Result, this](ParsedAST *AST) {
- if (!AST)
- return;
- Result = clangd::findDefinitions(*AST, Pos, Logger);
- });
+ std::map<SourceLocation, std::string> IncludeMap;
+ std::shared_ptr<const PreambleData> StalePreamble =
+ Resources->getPossiblyStalePreamble();
+ if (StalePreamble)
+ IncludeMap = StalePreamble->IncludeMap;
+ Resources->getAST().get()->runUnderLock(
+ [Pos, &Result, IncludeMap, this](ParsedAST *AST) {
+ if (!AST)
+ return;
+
+ Result = clangd::findDefinitions(*AST, Pos, Logger, IncludeMap);
+ });
return make_tagged(std::move(Result), TaggedFS.Tag);
}
@@ -344,7 +351,7 @@
return NewPath.str().str(); // First str() to convert from SmallString to
// StringRef, second to convert from StringRef
// to std::string
-
+
// Also check NewExt in upper-case, just in case.
llvm::sys::path::replace_extension(NewPath, NewExt.upper());
if (FS->exists(NewPath))
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
