[PATCH] D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums

[Sam McCall via Phabricator via cfe-commits]

sammccall 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.


I don't understand how this would still work (at least when using the index). The index call would have Scopes set to the enclosing scopes {""}, which is equivalent to a search for ::A. Maybe I'm missing something.

In https://reviews.llvm.org/D47223#1112118, @ioeric wrote:

> 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).


Yup. There are a few other instances of this:

1. decls aliased with using decls - we model these as duplicate symbols
2. decls aliased with using directives - we ask clang to resolve the namespaces in scope when searching
3. inline namespaces - we mostly get away with ignoring them

AFAICS none of these are ideal for enums. (2 fails because the list of namespaces would get too long).
This is messy :-(


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47223