ilya-biryukov added inline comments.
================
Comment at: clangd/index/SymbolCollector.cpp:73
+ Context = Context->getParent()) {
+ if (llvm::isa<TranslationUnitDecl>(Context) ||
+ llvm::isa<LinkageSpecDecl>(Context))
----------------
ioeric wrote:
> sammccall wrote:
> > I'm not sure this is always correct: at least clang accepts this code:
> >
> > namespace X { extern "C++" { int y; }}
> >
> > and you'll emit "y" instead of "X::y".
> >
> > I think the check you want is
> >
> > if (Context->isTransparentContext() || Context->isInlineNamespace())
> > continue;
> >
> > isTransparentContext will handle the Namespace and Enum cases as you do
> > below, including the enum/enum class distinction.
> >
> > (The code you have below is otherwise correct, I think - but a reader needs
> > to think about more separate cases in order to see that)
> In `namespace X { extern "C++" { int y; }}`, we would still want `y` instead
> of `X::y` since C-style symbol doesn't have scope. `printQualifiedName` also
> does the same thing printing `y`; I've added a test case for `extern C`.
>
> I also realized we've been dropping C symbols in `shouldFilterDecl` and fixed
> it in the same patch.
I think we want `X::y`, not `y`.
Lookup still finds it inside the namespace and does not find it in the global
scope. So for our purposes they are actually inside the namespace and have the
qualified name of this namespace. Here's an example:
```
namespace ns {
extern "C" int foo();
}
void test() {
ns::foo(); // ok
foo(); // error
::foo(); // error
}
```
Note, however, that the tricky bit there is probably merging of the symbols, as
it means symbols with the same USR (they are the same for all `extern "c"`
declarations with the same name, right?) can have different qualified names and
we won't know which one to choose.
```
namespace a {
extern "C" int foo();
}
namespace b {
extern "C" int foo(); // probably same USR, different qname. Also, possibly
different types.
}
```
Repository:
rL LLVM
https://reviews.llvm.org/D42796
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
