[llvm-branch-commits] [clang-tools-extra] 2e25be0 - [clangd] Add main file macros into the main-file index.
Author: Aleksandr Platonov
Date: 2021-01-14T15:10:17+03:00
New Revision: 2e25be0b6134e9544f7cee7bb7b31a921ca37cc0
URL:
https://github.com/llvm/llvm-project/commit/2e25be0b6134e9544f7cee7bb7b31a921ca37cc0
DIFF:
https://github.com/llvm/llvm-project/commit/2e25be0b6134e9544f7cee7bb7b31a921ca37cc0.diff
LOG: [clangd] Add main file macros into the main-file index.
This patch is a try to fix `WorkspaceSymbols.Macros` test after D93796.
If a macro definition is in the preamble section, then it appears to be in the
preamble (static) index and not in the main-file (dynamic) index.
Thus, a such macro could not be found at a symbol search according to the logic
that we skip symbols from the static index if the location of these symbols is
inside the dynamic index files.
To fix this behavior this patch adds main file macros into the main-file
(dynamic) index.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D94477
Added:
Modified:
clang-tools-extra/clangd/CollectMacros.cpp
clang-tools-extra/clangd/CollectMacros.h
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/CollectMacros.cpp
b/clang-tools-extra/clangd/CollectMacros.cpp
index bd6165ef10ba..0e89b35d9d56 100644
--- a/clang-tools-extra/clangd/CollectMacros.cpp
+++ b/clang-tools-extra/clangd/CollectMacros.cpp
@@ -13,8 +13,8 @@
namespace clang {
namespace clangd {
-void CollectMainFileMacros::add(const Token &MacroNameTok,
-const MacroInfo *MI) {
+void CollectMainFileMacros::add(const Token &MacroNameTok, const MacroInfo *MI,
+bool IsDefinition) {
if (!InMainFile)
return;
auto Loc = MacroNameTok.getLocation();
@@ -26,9 +26,9 @@ void CollectMainFileMacros::add(const Token &MacroNameTok,
auto Range = halfOpenToRange(
SM, CharSourceRange::getCharRange(Loc, MacroNameTok.getEndLoc()));
if (auto SID = getSymbolID(Name, MI, SM))
-Out.MacroRefs[SID].push_back(Range);
+Out.MacroRefs[SID].push_back({Range, IsDefinition});
else
-Out.UnknownMacros.push_back(Range);
+Out.UnknownMacros.push_back({Range, IsDefinition});
}
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/CollectMacros.h
b/clang-tools-extra/clangd/CollectMacros.h
index eecea0455be2..3240111e5a33 100644
--- a/clang-tools-extra/clangd/CollectMacros.h
+++ b/clang-tools-extra/clangd/CollectMacros.h
@@ -21,15 +21,20 @@
namespace clang {
namespace clangd {
-struct MainFileMacros {
- llvm::StringSet<> Names;
+struct MacroOccurrence {
// Instead of storing SourceLocation, we have to store the token range
because
// SourceManager from preamble is not available when we build the AST.
- llvm::DenseMap> MacroRefs;
+ Range Rng;
+ bool IsDefinition;
+};
+
+struct MainFileMacros {
+ llvm::StringSet<> Names;
+ llvm::DenseMap> MacroRefs;
// Somtimes it is not possible to compute the SymbolID for the Macro, e.g. a
// reference to an undefined macro. Store them separately, e.g. for semantic
// highlighting.
- std::vector UnknownMacros;
+ std::vector UnknownMacros;
// Ranges skipped by the preprocessor due to being inactive.
std::vector SkippedRanges;
};
@@ -49,7 +54,7 @@ class CollectMainFileMacros : public PPCallbacks {
}
void MacroDefined(const Token &MacroName, const MacroDirective *MD) override
{
-add(MacroName, MD->getMacroInfo());
+add(MacroName, MD->getMacroInfo(), /*IsDefinition=*/true);
}
void MacroExpands(const Token &MacroName, const MacroDefinition &MD,
@@ -87,7 +92,8 @@ class CollectMainFileMacros : public PPCallbacks {
}
private:
- void add(const Token &MacroNameTok, const MacroInfo *MI);
+ void add(const Token &MacroNameTok, const MacroInfo *MI,
+ bool IsDefinition = false);
const SourceManager &SM;
bool InMainFile = true;
MainFileMacros &Out;
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index be0a5437cde6..5e70baf73310 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -397,10 +397,10 @@ std::vector
getSemanticHighlightings(ParsedAST &AST) {
// Add highlightings for macro references.
for (const auto &SIDToRefs : AST.getMacros().MacroRefs) {
for (const auto &M : SIDToRefs.second)
- Builder.addToken({HighlightingKind::Macro, M});
+ Builder.addToken({HighlightingKind::Macro, M.Rng});
}
for (const auto &M : AST.getMacros().UnknownMacros)
-Builder.addToken({Highlig
[llvm-branch-commits] [clang-tools-extra] 7388c34 - [clangd][SwapIndex] ensure that the old index is alive while we are using it via the function returned by `SwapIndex::indexedFiles()` call
Author: Aleksandr Platonov
Date: 2021-01-22T16:26:39+03:00
New Revision: 7388c34685954862e5f1fa4734f42f7087e697a2
URL:
https://github.com/llvm/llvm-project/commit/7388c34685954862e5f1fa4734f42f7087e697a2
DIFF:
https://github.com/llvm/llvm-project/commit/7388c34685954862e5f1fa4734f42f7087e697a2.diff
LOG: [clangd][SwapIndex] ensure that the old index is alive while we are using
it via the function returned by `SwapIndex::indexedFiles()` call
Without this patch the old index could be freed, but there still could be tries
to access it via the function returned by `SwapIndex::indexedFiles()` call.
This leads to hard to reproduce clangd crashes at code completion.
This patch keeps the old index alive until the function returned by
`SwapIndex::indexedFiles()` call is alive.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D95206
Added:
Modified:
clang-tools-extra/clangd/index/Index.cpp
clang-tools-extra/clangd/index/Merge.cpp
Removed:
diff --git a/clang-tools-extra/clangd/index/Index.cpp
b/clang-tools-extra/clangd/index/Index.cpp
index 5da06f36ffe4..1b085140b4ff 100644
--- a/clang-tools-extra/clangd/index/Index.cpp
+++ b/clang-tools-extra/clangd/index/Index.cpp
@@ -78,7 +78,13 @@ void SwapIndex::relations(
llvm::unique_function
SwapIndex::indexedFiles() const {
- return snapshot()->indexedFiles();
+ // The index snapshot should outlive this method return value.
+ auto SnapShot = snapshot();
+ auto IndexedFiles = SnapShot->indexedFiles();
+ return [KeepAlive{std::move(SnapShot)},
+ IndexContainsFile{std::move(IndexedFiles)}](llvm::StringRef File) {
+return IndexContainsFile(File);
+ };
}
size_t SwapIndex::estimateMemoryUsage() const {
diff --git a/clang-tools-extra/clangd/index/Merge.cpp
b/clang-tools-extra/clangd/index/Merge.cpp
index 0dd0d9e01518..6f369ed2edcf 100644
--- a/clang-tools-extra/clangd/index/Merge.cpp
+++ b/clang-tools-extra/clangd/index/Merge.cpp
@@ -44,21 +44,23 @@ bool MergedIndex::fuzzyFind(
SymbolSlab Dyn = std::move(DynB).build();
llvm::DenseSet SeenDynamicSymbols;
- auto DynamicContainsFile = Dynamic->indexedFiles();
- More |= Static->fuzzyFind(Req, [&](const Symbol &S) {
-// We expect the definition to see the canonical declaration, so it seems
-// to be enough to check only the definition if it exists.
-if (DynamicContainsFile(S.Definition ? S.Definition.FileURI
- : S.CanonicalDeclaration.FileURI))
- return;
-auto DynS = Dyn.find(S.ID);
-++StaticCount;
-if (DynS == Dyn.end())
- return Callback(S);
-++MergedCount;
-SeenDynamicSymbols.insert(S.ID);
-Callback(mergeSymbol(*DynS, S));
- });
+ {
+auto DynamicContainsFile = Dynamic->indexedFiles();
+More |= Static->fuzzyFind(Req, [&](const Symbol &S) {
+ // We expect the definition to see the canonical declaration, so it seems
+ // to be enough to check only the definition if it exists.
+ if (DynamicContainsFile(S.Definition ? S.Definition.FileURI
+ : S.CanonicalDeclaration.FileURI))
+return;
+ auto DynS = Dyn.find(S.ID);
+ ++StaticCount;
+ if (DynS == Dyn.end())
+return Callback(S);
+ ++MergedCount;
+ SeenDynamicSymbols.insert(S.ID);
+ Callback(mergeSymbol(*DynS, S));
+});
+ }
SPAN_ATTACH(Tracer, "dynamic", DynamicCount);
SPAN_ATTACH(Tracer, "static", StaticCount);
SPAN_ATTACH(Tracer, "merged", MergedCount);
@@ -77,20 +79,22 @@ void MergedIndex::lookup(
Dynamic->lookup(Req, [&](const Symbol &S) { B.insert(S); });
auto RemainingIDs = Req.IDs;
- auto DynamicContainsFile = Dynamic->indexedFiles();
- Static->lookup(Req, [&](const Symbol &S) {
-// We expect the definition to see the canonical declaration, so it seems
-// to be enough to check only the definition if it exists.
-if (DynamicContainsFile(S.Definition ? S.Definition.FileURI
- : S.CanonicalDeclaration.FileURI))
- return;
-const Symbol *Sym = B.find(S.ID);
-RemainingIDs.erase(S.ID);
-if (!Sym)
- Callback(S);
-else
- Callback(mergeSymbol(*Sym, S));
- });
+ {
+auto DynamicContainsFile = Dynamic->indexedFiles();
+Static->lookup(Req, [&](const Symbol &S) {
+ // We expect the definition to see the canonical declaration, so it seems
+ // to be enough to check only the definition if it exists.
+ if (DynamicContainsFile(S.Definition ? S.Definition.FileURI
+ : S.CanonicalDeclaration.FileURI))
+return;
+ const Symbol *Sym = B.find(S.ID);
+ RemainingIDs.erase(S.ID);
+ if (!Sym)
+Callback(S);
+ else
+Callback(mergeSymbol(*Sym, S));
+});
+ }
for (const auto &ID : RemainingIDs)
if (const Symbol *Sym = B.fi
[llvm-branch-commits] [clang-tools-extra] 979228f - [clangd][fuzzyFind] Do not show stale symbols in the result.
Author: Aleksandr Platonov
Date: 2021-01-06T11:17:12+03:00
New Revision: 979228f120f4aa1265648b1c06f65a84bcca1ed6
URL:
https://github.com/llvm/llvm-project/commit/979228f120f4aa1265648b1c06f65a84bcca1ed6
DIFF:
https://github.com/llvm/llvm-project/commit/979228f120f4aa1265648b1c06f65a84bcca1ed6.diff
LOG: [clangd][fuzzyFind] Do not show stale symbols in the result.
This is follow up to D93393.
Without this patch `MergedIndex::fuzzyFind()` returns stale symbols from the
static index even if these symbols were removed.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D93796
Added:
Modified:
clang-tools-extra/clangd/index/Merge.cpp
clang-tools-extra/clangd/index/Merge.h
clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
clang-tools-extra/clangd/unittests/IndexTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/index/Merge.cpp
b/clang-tools-extra/clangd/index/Merge.cpp
index f66f47499624..29174ff0d354 100644
--- a/clang-tools-extra/clangd/index/Merge.cpp
+++ b/clang-tools-extra/clangd/index/Merge.cpp
@@ -22,11 +22,6 @@
namespace clang {
namespace clangd {
-// FIXME: Deleted symbols in dirty files are still returned (from Static).
-//To identify these eliminate these, we should:
-// - find the generating file from each Symbol which is Static-only
-// - ask Dynamic if it has that file (needs new SymbolIndex method)
-// - if so, drop the Symbol.
bool MergedIndex::fuzzyFind(
const FuzzyFindRequest &Req,
llvm::function_ref Callback) const {
@@ -49,7 +44,13 @@ bool MergedIndex::fuzzyFind(
SymbolSlab Dyn = std::move(DynB).build();
llvm::DenseSet SeenDynamicSymbols;
+ auto DynamicContainsFile = Dynamic->indexedFiles();
More |= Static->fuzzyFind(Req, [&](const Symbol &S) {
+// We expect the definition to see the canonical declaration, so it seems
+// to be enough to check only the definition if it exists.
+if (DynamicContainsFile(S.Definition ? S.Definition.FileURI
+ : S.CanonicalDeclaration.FileURI))
+ return;
auto DynS = Dyn.find(S.ID);
++StaticCount;
if (DynS == Dyn.end())
diff --git a/clang-tools-extra/clangd/index/Merge.h
b/clang-tools-extra/clangd/index/Merge.h
index 0cdff38f0678..f8696b460c90 100644
--- a/clang-tools-extra/clangd/index/Merge.h
+++ b/clang-tools-extra/clangd/index/Merge.h
@@ -23,10 +23,6 @@ Symbol mergeSymbol(const Symbol &L, const Symbol &R);
// - the Dynamic index covers few files, but is relatively up-to-date.
// - the Static index covers a bigger set of files, but is relatively stale.
// The returned index attempts to combine results, and avoid duplicates.
-//
-// FIXME: We don't have a mechanism in Index to track deleted symbols and
-// refs in dirty files, so the merged index may return stale symbols
-// and refs from Static index.
class MergedIndex : public SymbolIndex {
const SymbolIndex *Dynamic, *Static;
diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
index 43658284937e..bdd3ddca1a61 100644
--- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -52,7 +52,17 @@ std::vector getSymbols(TestTU &TU,
llvm::StringRef Query,
return *SymbolInfos;
}
-TEST(WorkspaceSymbols, Macros) {
+// FIXME: We update two indexes during main file processing:
+//- preamble index (static)
+//- main-file index (dynamic)
+//The macro in this test appears to be in the preamble index and not
+//in the main-file index. According to our logic of indexes merging, we
+//do not take this macro from the static (preamble) index, because it
+//location within the file from the dynamic (main-file) index.
+//
+//Possible solution is to exclude main-file symbols from the preamble
+//index, after that we can enable this test again.
+TEST(WorkspaceSymbols, DISABLED_Macros) {
TestTU TU;
TU.Code = R"cpp(
#define MACRO X
diff --git a/clang-tools-extra/clangd/unittests/IndexTests.cpp
b/clang-tools-extra/clangd/unittests/IndexTests.cpp
index ce4845e3e144..33b0414275ca 100644
--- a/clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -335,6 +335,39 @@ TEST(MergeIndexTest, FuzzyFind) {
UnorderedElementsAre("ns::A", "ns::B", "ns::C"));
}
+TEST(MergeIndexTest, FuzzyFindRemovedSymbol) {
+ FileIndex DynamicIndex, StaticIndex;
+ MergedIndex Merge(&DynamicIndex, &StaticIndex);
+
+ const char *HeaderCode = "class Foo;";
+ auto HeaderSymbols = TestTU::withHeaderCode(HeaderCode).headerSymbols();
+ auto Foo = findSymbol(HeaderSymbols, "Foo");
+
+ // Build static index for test.cc with Foo symbol
+ TestTU Test;
+ Test.HeaderCode =
[llvm-branch-commits] [clang-tools-extra] e35f922 - [clangd] Ignore the static index refs from the dynamic index files.
Author: Aleksandr Platonov
Date: 2020-12-18T15:36:30+03:00
New Revision: e35f9229dcb264be4a0a1ecf5cca2493f2c48878
URL:
https://github.com/llvm/llvm-project/commit/e35f9229dcb264be4a0a1ecf5cca2493f2c48878
DIFF:
https://github.com/llvm/llvm-project/commit/e35f9229dcb264be4a0a1ecf5cca2493f2c48878.diff
LOG: [clangd] Ignore the static index refs from the dynamic index files.
This patch fixes the following problem:
- open a file with references to the symbol `Foo`
- remove all references to `Foo` (from the dynamic index).
- `MergedIndex::refs()` result will contain positions of removed references
(from the static index).
The idea of this patch is to keep a set of files which were used during index
build inside the index.
Thus at processing the static index references we can check if the file of
processing reference is a part of the dynamic index or not.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D93393
Added:
Modified:
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/Index.cpp
clang-tools-extra/clangd/index/Index.h
clang-tools-extra/clangd/index/MemIndex.cpp
clang-tools-extra/clangd/index/MemIndex.h
clang-tools-extra/clangd/index/Merge.cpp
clang-tools-extra/clangd/index/Merge.h
clang-tools-extra/clangd/index/ProjectAware.cpp
clang-tools-extra/clangd/index/dex/Dex.cpp
clang-tools-extra/clangd/index/dex/Dex.h
clang-tools-extra/clangd/index/remote/Client.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/DexTests.cpp
clang-tools-extra/clangd/unittests/IndexTests.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
clang-tools-extra/clangd/unittests/TestFS.cpp
Removed:
diff --git a/clang-tools-extra/clangd/index/FileIndex.cpp
b/clang-tools-extra/clangd/index/FileIndex.cpp
index 1ccfb4485638..143e76863777 100644
--- a/clang-tools-extra/clangd/index/FileIndex.cpp
+++ b/clang-tools-extra/clangd/index/FileIndex.cpp
@@ -266,11 +266,14 @@ FileSymbols::buildIndex(IndexType Type, DuplicateHandling
DuplicateHandle,
std::vector> SymbolSlabs;
std::vector> RefSlabs;
std::vector> RelationSlabs;
+ llvm::StringSet<> Files;
std::vector MainFileRefs;
{
std::lock_guard Lock(Mutex);
-for (const auto &FileAndSymbols : SymbolsSnapshot)
+for (const auto &FileAndSymbols : SymbolsSnapshot) {
SymbolSlabs.push_back(FileAndSymbols.second);
+ Files.insert(FileAndSymbols.first());
+}
for (const auto &FileAndRefs : RefsSnapshot) {
RefSlabs.push_back(FileAndRefs.second.Slab);
if (FileAndRefs.second.CountReferences)
@@ -372,14 +375,14 @@ FileSymbols::buildIndex(IndexType Type, DuplicateHandling
DuplicateHandle,
case IndexType::Light:
return std::make_unique(
llvm::make_pointee_range(AllSymbols), std::move(AllRefs),
-std::move(AllRelations),
+std::move(AllRelations), std::move(Files),
std::make_tuple(std::move(SymbolSlabs), std::move(RefSlabs),
std::move(RefsStorage), std::move(SymsStorage)),
StorageSize);
case IndexType::Heavy:
return std::make_unique(
llvm::make_pointee_range(AllSymbols), std::move(AllRefs),
-std::move(AllRelations),
+std::move(AllRelations), std::move(Files),
std::make_tuple(std::move(SymbolSlabs), std::move(RefSlabs),
std::move(RefsStorage), std::move(SymsStorage)),
StorageSize);
diff --git a/clang-tools-extra/clangd/index/Index.cpp
b/clang-tools-extra/clangd/index/Index.cpp
index b309053972eb..5da06f36ffe4 100644
--- a/clang-tools-extra/clangd/index/Index.cpp
+++ b/clang-tools-extra/clangd/index/Index.cpp
@@ -76,6 +76,11 @@ void SwapIndex::relations(
return snapshot()->relations(R, CB);
}
+llvm::unique_function
+SwapIndex::indexedFiles() const {
+ return snapshot()->indexedFiles();
+}
+
size_t SwapIndex::estimateMemoryUsage() const {
return snapshot()->estimateMemoryUsage();
}
diff --git a/clang-tools-extra/clangd/index/Index.h
b/clang-tools-extra/clangd/index/Index.h
index f0959e71d50f..c961aa9d8bd9 100644
--- a/clang-tools-extra/clangd/index/Index.h
+++ b/clang-tools-extra/clangd/index/Index.h
@@ -14,6 +14,7 @@
#include "Symbol.h"
#include "SymbolID.h"
#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/FunctionExtras.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/JSON.h"
@@ -121,6 +122,11 @@ class SymbolIndex {
llvm::function_ref
Callback) const = 0;
+ /// Returns function which checks if the specified file was used to build
this
+ /// index or not. The function must only be called while the index is alive.
+ virtual llvm::unique_function
+ indexedFiles() const = 0;
+
/// Returns estimated size of index (in bytes).
virtual size_t estimateM
[llvm-branch-commits] [clang-tools-extra] 2522fa0 - [clangd] Do not take stale definition from the static index.
Author: Aleksandr Platonov
Date: 2020-12-23T18:21:38+03:00
New Revision: 2522fa053b62520ae48b4b27117ca003a2c878ab
URL:
https://github.com/llvm/llvm-project/commit/2522fa053b62520ae48b4b27117ca003a2c878ab
DIFF:
https://github.com/llvm/llvm-project/commit/2522fa053b62520ae48b4b27117ca003a2c878ab.diff
LOG: [clangd] Do not take stale definition from the static index.
This is follow up to D93393.
Without this patch clangd takes the symbol definition from the static index if
this definition was removed from the dynamic index.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D93683
Added:
Modified:
clang-tools-extra/clangd/index/Merge.cpp
clang-tools-extra/clangd/unittests/IndexTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/index/Merge.cpp
b/clang-tools-extra/clangd/index/Merge.cpp
index 97babacf2b38..f66f47499624 100644
--- a/clang-tools-extra/clangd/index/Merge.cpp
+++ b/clang-tools-extra/clangd/index/Merge.cpp
@@ -76,7 +76,13 @@ void MergedIndex::lookup(
Dynamic->lookup(Req, [&](const Symbol &S) { B.insert(S); });
auto RemainingIDs = Req.IDs;
+ auto DynamicContainsFile = Dynamic->indexedFiles();
Static->lookup(Req, [&](const Symbol &S) {
+// We expect the definition to see the canonical declaration, so it seems
+// to be enough to check only the definition if it exists.
+if (DynamicContainsFile(S.Definition ? S.Definition.FileURI
+ : S.CanonicalDeclaration.FileURI))
+ return;
const Symbol *Sym = B.find(S.ID);
RemainingIDs.erase(S.ID);
if (!Sym)
diff --git a/clang-tools-extra/clangd/unittests/IndexTests.cpp
b/clang-tools-extra/clangd/unittests/IndexTests.cpp
index 8efc637d1250..ce4845e3e144 100644
--- a/clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -290,6 +290,40 @@ TEST(MergeIndexTest, Lookup) {
EXPECT_THAT(lookup(M, {}), UnorderedElementsAre());
}
+TEST(MergeIndexTest, LookupRemovedDefinition) {
+ FileIndex DynamicIndex, StaticIndex;
+ MergedIndex Merge(&DynamicIndex, &StaticIndex);
+
+ const char *HeaderCode = "class Foo;";
+ auto HeaderSymbols = TestTU::withHeaderCode(HeaderCode).headerSymbols();
+ auto Foo = findSymbol(HeaderSymbols, "Foo");
+
+ // Build static index for test.cc with Foo definition
+ TestTU Test;
+ Test.HeaderCode = HeaderCode;
+ Test.Code = "class Foo {};";
+ Test.Filename = "test.cc";
+ auto AST = Test.build();
+ StaticIndex.updateMain(testPath(Test.Filename), AST);
+
+ // Remove Foo definition from test.cc, i.e. build dynamic index for test.cc
+ // without Foo definition.
+ Test.Code = "class Foo;";
+ AST = Test.build();
+ DynamicIndex.updateMain(testPath(Test.Filename), AST);
+
+ // Merged index should not return the symbol definition if this definition
+ // location is inside a file from the dynamic index.
+ LookupRequest LookupReq;
+ LookupReq.IDs = {Foo.ID};
+ unsigned SymbolCounter = 0;
+ Merge.lookup(LookupReq, [&](const Symbol &Sym) {
+++SymbolCounter;
+EXPECT_FALSE(Sym.Definition);
+ });
+ EXPECT_EQ(SymbolCounter, 1u);
+}
+
TEST(MergeIndexTest, FuzzyFind) {
auto I = MemIndex::build(generateSymbols({"ns::A", "ns::B"}), RefSlab(),
RelationSlab()),
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang-tools-extra] d8ba6b4 - [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines
Author: Aleksandr Platonov
Date: 2020-09-29T19:54:55+03:00
New Revision: d8ba6b4ab3eceb6bbcdf4371d4ffaab9d1a5cebe
URL:
https://github.com/llvm/llvm-project/commit/d8ba6b4ab3eceb6bbcdf4371d4ffaab9d1a5cebe
DIFF:
https://github.com/llvm/llvm-project/commit/d8ba6b4ab3eceb6bbcdf4371d4ffaab9d1a5cebe.diff
LOG: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines
As @kadircet mentions in D84912#2184144, `findNearbyIdentifier()` traverses the
whole file if there is no identifier for the word.
This patch ensures give up after 2^N lines in any case.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D87891
Added:
Modified:
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/XRefs.cpp
b/clang-tools-extra/clangd/XRefs.cpp
index 9e8791c2a765..9532e1418cca 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -562,19 +562,34 @@ const syntax::Token *findNearbyIdentifier(const
SpelledWord &Word,
auto Cost = [&](SourceLocation Loc) -> unsigned {
assert(SM.getFileID(Loc) == File && "spelled token in wrong file?");
unsigned Line = SM.getSpellingLineNumber(Loc);
-if (Line > WordLine)
- return 1 + llvm::Log2_64(Line - WordLine);
-if (Line < WordLine)
- return 2 + llvm::Log2_64(WordLine - Line);
-return 0;
+return Line >= WordLine ? Line - WordLine : 2 * (WordLine - Line);
};
const syntax::Token *BestTok = nullptr;
- // Search bounds are based on word length: 2^N lines forward.
- unsigned BestCost = Word.Text.size() + 1;
+ unsigned BestCost = -1;
+ // Search bounds are based on word length:
+ // - forward: 2^N lines
+ // - backward: 2^(N-1) lines.
+ unsigned MaxDistance =
+ 1U << std::min(Word.Text.size(),
+ std::numeric_limits::digits - 1);
+ // Line number for SM.translateLineCol() should be one-based, also
+ // SM.translateLineCol() can handle line number greater than
+ // number of lines in the file.
+ // - LineMin = max(1, WordLine + 1 - 2^(N-1))
+ // - LineMax = WordLine + 1 + 2^N
+ unsigned LineMin =
+ WordLine + 1 <= MaxDistance / 2 ? 1 : WordLine + 1 - MaxDistance / 2;
+ unsigned LineMax = WordLine + 1 + MaxDistance;
+ SourceLocation LocMin = SM.translateLineCol(File, LineMin, 1);
+ assert(LocMin.isValid());
+ SourceLocation LocMax = SM.translateLineCol(File, LineMax, 1);
+ assert(LocMax.isValid());
// Updates BestTok and BestCost if Tok is a good candidate.
// May return true if the cost is too high for this token.
auto Consider = [&](const syntax::Token &Tok) {
+if (Tok.location() < LocMin || Tok.location() > LocMax)
+ return true; // we are too far from the word, break the outer loop.
if (!(Tok.kind() == tok::identifier && Tok.text(SM) == Word.Text))
return false;
// No point guessing the same location we started with.
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index a48bb9c8c182..40637b5fa758 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1428,6 +1428,11 @@ TEST(LocateSymbol, NearbyIdentifier) {
// h^i
+
+
+
+
+ int x = hi;
)cpp",
R"cpp(
// prefer nearest occurrence even if several matched tokens
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang-tools-extra] 1ca174b - [clangd][query-driver] Extract target
Author: Aleksandr Platonov
Date: 2020-11-26T15:08:26+03:00
New Revision: 1ca174b6420a49bcd3331d6f86e237b627163597
URL:
https://github.com/llvm/llvm-project/commit/1ca174b6420a49bcd3331d6f86e237b627163597
DIFF:
https://github.com/llvm/llvm-project/commit/1ca174b6420a49bcd3331d6f86e237b627163597.diff
LOG: [clangd][query-driver] Extract target
In some cases system includes extractions is not enough, we also need target
specific defines.
The problems appears when clang default target is not the same as toolchain's
one (GCC cross-compiler, MinGW on Windows).
After this patch `query-driver` also extracts target and adds
`--target=` compile option.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D92012
Added:
Modified:
clang-tools-extra/clangd/QueryDriverDatabase.cpp
clang-tools-extra/clangd/test/system-include-extractor.test
Removed:
diff --git a/clang-tools-extra/clangd/QueryDriverDatabase.cpp
b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
index 9d731f5563cf..d59263e73994 100644
--- a/clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -33,6 +33,9 @@
#include "support/Logger.h"
#include "support/Path.h"
#include "support/Trace.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
#include "clang/Driver/Types.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/DenseMap.h"
@@ -56,55 +59,102 @@ namespace clang {
namespace clangd {
namespace {
-std::vector parseDriverOutput(llvm::StringRef Output) {
+struct DriverInfo {
std::vector SystemIncludes;
+ std::string Target;
+};
+
+bool isValidTarget(llvm::StringRef Triple) {
+ std::shared_ptr TargetOpts(new TargetOptions);
+ TargetOpts->Triple = Triple.str();
+ DiagnosticsEngine Diags(new DiagnosticIDs, new DiagnosticOptions,
+ new IgnoringDiagConsumer);
+ IntrusiveRefCntPtr Target =
+ TargetInfo::CreateTargetInfo(Diags, TargetOpts);
+ return bool(Target);
+}
+
+llvm::Optional parseDriverOutput(llvm::StringRef Output) {
+ DriverInfo Info;
const char SIS[] = "#include <...> search starts here:";
const char SIE[] = "End of search list.";
+ const char TS[] = "Target: ";
llvm::SmallVector Lines;
Output.split(Lines, '\n', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
- auto StartIt = llvm::find_if(
- Lines, [SIS](llvm::StringRef Line) { return Line.trim() == SIS; });
- if (StartIt == Lines.end()) {
+ enum {
+Initial,// Initial state: searching for target or includes
list.
+IncludesExtracting, // Includes extracting.
+Done// Includes and target extraction done.
+ } State = Initial;
+ bool SeenIncludes = false;
+ bool SeenTarget = false;
+ for (auto *It = Lines.begin(); State != Done && It != Lines.end(); ++It) {
+auto Line = *It;
+switch (State) {
+case Initial:
+ if (!SeenIncludes && Line.trim() == SIS) {
+SeenIncludes = true;
+State = IncludesExtracting;
+ } else if (!SeenTarget && Line.trim().startswith(TS)) {
+SeenTarget = true;
+llvm::StringRef TargetLine = Line.trim();
+TargetLine.consume_front(TS);
+// Only detect targets that clang understands
+if (!isValidTarget(TargetLine)) {
+ elog("System include extraction: invalid target \"{0}\", ignoring",
+ TargetLine);
+} else {
+ Info.Target = TargetLine.str();
+ vlog("System include extraction: target extracted: \"{0}\"",
+ TargetLine);
+}
+ }
+ break;
+case IncludesExtracting:
+ if (Line.trim() == SIE) {
+State = SeenTarget ? Done : Initial;
+ } else {
+Info.SystemIncludes.push_back(Line.trim().str());
+vlog("System include extraction: adding {0}", Line);
+ }
+ break;
+default:
+ llvm_unreachable("Impossible state of the driver output parser");
+ break;
+}
+ }
+ if (!SeenIncludes) {
elog("System include extraction: start marker not found: {0}", Output);
-return {};
+return llvm::None;
}
- ++StartIt;
- const auto EndIt =
- llvm::find_if(llvm::make_range(StartIt, Lines.end()),
-[SIE](llvm::StringRef Line) { return Line.trim() == SIE;
});
- if (EndIt == Lines.end()) {
+ if (State == IncludesExtracting) {
elog("System include extraction: end marker missing: {0}", Output);
-return {};
+return llvm::None;
}
-
- for (llvm::StringRef Line : llvm::make_range(StartIt, EndIt)) {
-SystemIncludes.push_back(Line.trim().str());
-vlog("System include extraction: adding {0}", Line);
- }
- return SystemIncludes;
+ return std::move(Info);
}
-std::vector
-extractSystemIncludes(PathRef Driver, llvm::StringRef Lang,
- llvm::ArrayRef Command
