kbobyrev created this revision.
kbobyrev added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
This is a cheap "fix" of the problem described in
https://github.com/clangd/clangd/issues/39. Currently, for this code:
c++
namespace ns {
enum Color { Green };
enum class Vehicle { Car };
}
The following is true:
- `Vehicle::Car` scope is `ns::Vehicle::` - it can't be accessed without
spelling full enum class name
- `Color::Green` scope is `ns::` because it can be accessed as `ns::Car`
However, this causes index FuzzyFind to show empty results when querying
`ns::Color::` - `Color::Green` will only show up for `ns::`.
This patch changes the behavior and makes `SymbolCollector` treat plain `enum`
the same way it does handle `enum class`. Ideally, the index would point to the
same symbol for both `ns::Green` and `ns::Color::Green` but that increases
coupling since the enum information has to be propagated to the index builder
which is logically quite far from the `SymbolCollector`.
Fixes: https://github.com/clangd/clangd/issues/39
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D113974
Files:
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/FindSymbolsTests.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
@@ -1232,7 +1232,7 @@
UnorderedElementsAre(
AllOf(QName("Red"), ForCodeCompletion(true)),
AllOf(QName("Color"), ForCodeCompletion(true)),
- AllOf(QName("Green"), ForCodeCompletion(true)),
+ AllOf(QName("Color::Green"), ForCodeCompletion(true)),
AllOf(QName("Color2"), ForCodeCompletion(true)),
AllOf(QName("Color2::Yellow"), ForCodeCompletion(false)),
AllOf(QName("ns"), ForCodeCompletion(true)),
Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -250,14 +250,15 @@
)cpp";
EXPECT_THAT(getSymbols(TU, "Red"), ElementsAre(QName("Red")));
EXPECT_THAT(getSymbols(TU, "::Red"), ElementsAre(QName("Red")));
- EXPECT_THAT(getSymbols(TU, "Green"), ElementsAre(QName("Green")));
- EXPECT_THAT(getSymbols(TU, "Green"), ElementsAre(QName("Green")));
+ EXPECT_THAT(getSymbols(TU, "Color::Green"),
+ ElementsAre(QName("Color::Green")));
EXPECT_THAT(getSymbols(TU, "Color2::Yellow"),
ElementsAre(QName("Color2::Yellow")));
EXPECT_THAT(getSymbols(TU, "Yellow"), ElementsAre(QName("Color2::Yellow")));
EXPECT_THAT(getSymbols(TU, "ns::Black"), ElementsAre(QName("ns::Black")));
- EXPECT_THAT(getSymbols(TU, "ns::Blue"), ElementsAre(QName("ns::Blue")));
+ EXPECT_THAT(getSymbols(TU, "ns::Color3::Blue"),
+ ElementsAre(QName("ns::Color3::Blue")));
EXPECT_THAT(getSymbols(TU, "ns::Color4::White"),
ElementsAre(QName("ns::Color4::White")));
}
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -828,6 +828,23 @@
// FIXME: this returns foo:bar: for objective-C methods, we prefer only foo:
// for consistency with CodeCompletionString and a clean name/signature
split.
std::tie(S.Scope, S.Name) = splitQualifiedName(QName);
+ std::string Scope = S.Scope.str();
+ // Put additional qualifiers for enum constants: technically, they can be
+ // accessed without using the enum name prefix (unless declared as enum
+ // class) but it is more convenient to enhance the scope with the enum type
+ // name.
+ // FIXME: It would be better to just have the SymbolIndex look up the same
+ // enum constant both with and without enum type name but at the time the
+ // index is built the information about ND is already lost.
+ if (const auto *EnumConstant = llvm::dyn_cast<EnumConstantDecl>(&ND)) {
+ if (const auto *II = EnumConstant->getType().getBaseTypeIdentifier()) {
+ std::string EnumScope = II->getName().str() + "::";
+ // For "enum class" the qualified name already has EnumScope.
+ if (!S.Scope.endswith(EnumScope))
+ Scope += EnumScope;
+ }
+ }
+ S.Scope = Scope;
std::string TemplateSpecializationArgs = printTemplateSpecializationArgs(ND);
S.TemplateSpecializationArgs = TemplateSpecializationArgs;
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1232,7 +1232,7 @@
UnorderedElementsAre(
AllOf(QName("Red"), ForCodeCompletion(true)),
AllOf(QName("Color"), ForCodeCompletion(true)),
- AllOf(QName("Green"), ForCodeCompletion(true)),
+ AllOf(QName("Color::Green"), ForCodeCompletion(true)),
AllOf(QName("Color2"), ForCodeCompletion(true)),
AllOf(QName("Color2::Yellow"), ForCodeCompletion(false)),
AllOf(QName("ns"), ForCodeCompletion(true)),
Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -250,14 +250,15 @@
)cpp";
EXPECT_THAT(getSymbols(TU, "Red"), ElementsAre(QName("Red")));
EXPECT_THAT(getSymbols(TU, "::Red"), ElementsAre(QName("Red")));
- EXPECT_THAT(getSymbols(TU, "Green"), ElementsAre(QName("Green")));
- EXPECT_THAT(getSymbols(TU, "Green"), ElementsAre(QName("Green")));
+ EXPECT_THAT(getSymbols(TU, "Color::Green"),
+ ElementsAre(QName("Color::Green")));
EXPECT_THAT(getSymbols(TU, "Color2::Yellow"),
ElementsAre(QName("Color2::Yellow")));
EXPECT_THAT(getSymbols(TU, "Yellow"), ElementsAre(QName("Color2::Yellow")));
EXPECT_THAT(getSymbols(TU, "ns::Black"), ElementsAre(QName("ns::Black")));
- EXPECT_THAT(getSymbols(TU, "ns::Blue"), ElementsAre(QName("ns::Blue")));
+ EXPECT_THAT(getSymbols(TU, "ns::Color3::Blue"),
+ ElementsAre(QName("ns::Color3::Blue")));
EXPECT_THAT(getSymbols(TU, "ns::Color4::White"),
ElementsAre(QName("ns::Color4::White")));
}
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -828,6 +828,23 @@
// FIXME: this returns foo:bar: for objective-C methods, we prefer only foo:
// for consistency with CodeCompletionString and a clean name/signature split.
std::tie(S.Scope, S.Name) = splitQualifiedName(QName);
+ std::string Scope = S.Scope.str();
+ // Put additional qualifiers for enum constants: technically, they can be
+ // accessed without using the enum name prefix (unless declared as enum
+ // class) but it is more convenient to enhance the scope with the enum type
+ // name.
+ // FIXME: It would be better to just have the SymbolIndex look up the same
+ // enum constant both with and without enum type name but at the time the
+ // index is built the information about ND is already lost.
+ if (const auto *EnumConstant = llvm::dyn_cast<EnumConstantDecl>(&ND)) {
+ if (const auto *II = EnumConstant->getType().getBaseTypeIdentifier()) {
+ std::string EnumScope = II->getName().str() + "::";
+ // For "enum class" the qualified name already has EnumScope.
+ if (!S.Scope.endswith(EnumScope))
+ Scope += EnumScope;
+ }
+ }
+ S.Scope = Scope;
std::string TemplateSpecializationArgs = printTemplateSpecializationArgs(ND);
S.TemplateSpecializationArgs = TemplateSpecializationArgs;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
