tom-anders created this revision.
tom-anders added a reviewer: nridge.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
tom-anders requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
As discussed in https://github.com/clangd/clangd/issues/1082:
"Without all-scopes-completion, such nested symbols would indeed be unnecessary
in the
index, because the only way you could get them as a completion result is if
you're
invoking the completion in the nested scope (e.g. Foo::^), in which case result
can be
obtained from the AST."
I tested this on the LLVM codebase and there was no significant increase in
index size
(less than 1MB). I think this makes sense if we look at the additional data
that is stored
in SymbolCollector::addDeclaration when IndexedForCodeCompletion is set:
- Signature: Empty string for enum constants
- SnippetSuffix: Empty
- Documentation: Empty most of the time
- ReturnType and Type: Same string for all enum constants within a enum, so
only two additional strings per indexed enum
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D136925
Files:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/CodeComplete.h
clang-tools-extra/clangd/index/Serialization.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1316,6 +1316,11 @@
Black
};
}
+ class Color3 {
+ enum {
+ Blue
+ };
+ };
)";
runSymbolCollector(Header, /*Main=*/"");
EXPECT_THAT(Symbols,
@@ -1324,7 +1329,9 @@
AllOf(qName("Color"), forCodeCompletion(true)),
AllOf(qName("Green"), forCodeCompletion(true)),
AllOf(qName("Color2"), forCodeCompletion(true)),
- AllOf(qName("Color2::Yellow"), forCodeCompletion(false)),
+ AllOf(qName("Color2::Yellow"), forCodeCompletion(true)),
+ AllOf(qName("Color3"), forCodeCompletion(true)),
+ AllOf(qName("Color3::Blue"), forCodeCompletion(true)),
AllOf(qName("ns"), forCodeCompletion(true)),
AllOf(qName("ns::Black"), forCodeCompletion(true))));
}
Index: clang-tools-extra/clangd/index/Serialization.cpp
===================================================================
--- clang-tools-extra/clangd/index/Serialization.cpp
+++ clang-tools-extra/clangd/index/Serialization.cpp
@@ -455,7 +455,7 @@
// The current versioning scheme is simple - non-current versions are rejected.
// If you make a breaking change, bump this version number to invalidate stored
// data. Later we may want to support some backward compatibility.
-constexpr static uint32_t Version = 17;
+constexpr static uint32_t Version = 18;
llvm::Expected<IndexFileIn> readRIFF(llvm::StringRef Data,
SymbolOrigin Origin) {
Index: clang-tools-extra/clangd/CodeComplete.h
===================================================================
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -291,7 +291,7 @@
// For index-based completion, we only consider:
// * symbols in namespaces or translation unit scopes (e.g. no class
// members, no locals)
-// * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
+// * enum constants (both scoped and unscoped)
// * primary templates (no specializations)
// For the other cases, we let Clang do the completion because it does not
// need any non-local information and it will be much better at following
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -2129,8 +2129,10 @@
if (InTopLevelScope(ND))
return true;
+ // Always index enum constants, even if they're not in the top level scope:
when
+ // --all-scopes-completion is set, we'll want to complete those as well.
if (const auto *EnumDecl = dyn_cast<clang::EnumDecl>(ND.getDeclContext()))
- return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped();
+ return true;
return false;
}
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1316,6 +1316,11 @@
Black
};
}
+ class Color3 {
+ enum {
+ Blue
+ };
+ };
)";
runSymbolCollector(Header, /*Main=*/"");
EXPECT_THAT(Symbols,
@@ -1324,7 +1329,9 @@
AllOf(qName("Color"), forCodeCompletion(true)),
AllOf(qName("Green"), forCodeCompletion(true)),
AllOf(qName("Color2"), forCodeCompletion(true)),
- AllOf(qName("Color2::Yellow"), forCodeCompletion(false)),
+ AllOf(qName("Color2::Yellow"), forCodeCompletion(true)),
+ AllOf(qName("Color3"), forCodeCompletion(true)),
+ AllOf(qName("Color3::Blue"), forCodeCompletion(true)),
AllOf(qName("ns"), forCodeCompletion(true)),
AllOf(qName("ns::Black"), forCodeCompletion(true))));
}
Index: clang-tools-extra/clangd/index/Serialization.cpp
===================================================================
--- clang-tools-extra/clangd/index/Serialization.cpp
+++ clang-tools-extra/clangd/index/Serialization.cpp
@@ -455,7 +455,7 @@
// The current versioning scheme is simple - non-current versions are rejected.
// If you make a breaking change, bump this version number to invalidate stored
// data. Later we may want to support some backward compatibility.
-constexpr static uint32_t Version = 17;
+constexpr static uint32_t Version = 18;
llvm::Expected<IndexFileIn> readRIFF(llvm::StringRef Data,
SymbolOrigin Origin) {
Index: clang-tools-extra/clangd/CodeComplete.h
===================================================================
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -291,7 +291,7 @@
// For index-based completion, we only consider:
// * symbols in namespaces or translation unit scopes (e.g. no class
// members, no locals)
-// * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
+// * enum constants (both scoped and unscoped)
// * primary templates (no specializations)
// For the other cases, we let Clang do the completion because it does not
// need any non-local information and it will be much better at following
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -2129,8 +2129,10 @@
if (InTopLevelScope(ND))
return true;
+ // Always index enum constants, even if they're not in the top level scope: when
+ // --all-scopes-completion is set, we'll want to complete those as well.
if (const auto *EnumDecl = dyn_cast<clang::EnumDecl>(ND.getDeclContext()))
- return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped();
+ return true;
return false;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
