Yeah this is just a bug in clang's pprinter. I'll fix it. If you give multiple C++ names to the same linker symbol using extern C, I'm pretty sure you're in UB land.
On Fri, Feb 2, 2018, 12:04 Ilya Biryukov via Phabricator < revi...@reviews.llvm.org> wrote: > 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