ioeric added a comment. In https://reviews.llvm.org/D47223#1110571, @malaperle wrote:
> In https://reviews.llvm.org/D47223#1109247, @ilya-biryukov wrote: > > > I'm not sure if we have tests for that, but I remember that we kept the > > enumerators in the outer scope so that completion could find them.. > > Am I right that this patch will change the behavior and we won't get > > enumerators in the following example: > > > > /// foo.h > > enum Foo { > > A, B, C > > }; > > > > /// foo.cpp > > #include "foo.h" > > > > int a = ^ // <-- A, B, C should be in completion list here. > > > > > Not quite but still potentially problematic. With the patch, A, B, C would be > found but not ::A, ::B, ::C. > > > It's one of those cases where code completion and workspace symbol search > > seem to want different results :-( > > I suggest to add an extra string field for containing unscoped enum name, > > maybe into symbol details? And add a parameter to `Index::fuzzyFind` on > > whether we need to match enum scopes or not. > > +@ioeric, +@sammccall, WDYT? > > I'll wait to see what others think before changing it. But I feel it's a bit > odd that completion and workspace symbols would be inconsistent. I'd rather > have it that A, ::A, and Foo::A work for both completion and workspace. Maybe > it would complicate things too much... Having "wrong" names in workspaceSymbol (e.g. ::A instead of ::Foo::A) and missing results in code completions (e.g. ::A missing in ::^) seem equally bad. I think the fundamental problem here is that C++ symbols (esp. enum constants) can have different names (e.g. ns::Foo::A and ns::A). We want `ns::Foo::A` for workspaceSymbols, but we also want `ns::A` for index-based code completions (sema completion would give us `ns::Foo::A` right? ). One possible solution would be adding a short name in `Symbol` (e.g. `ns::A` is a short name for `ns::Foo::A`), and index/fuzzyFind can match on both names. ================ Comment at: clangd/index/SymbolCollector.cpp:365 + + using namespace clang::ast_matchers; + // For enumerators in unscoped enums that have names, even if they are not ---------------- drive-by nit: please pull this to a helper function. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47223 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
