[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

[Richard Smith - zygoloid via Phabricator via lldb-commits]

rsmith added a comment.

In https://reviews.llvm.org/D39307#907358, @xiaobai wrote:

> It seems like clang shouldn't even be asking us about this in the first place, but I don't fully understand why it's trying to do what it's trying to do.


It looks like there are two issues here: clang is doing seemingly-unnecessary work as part of a redeclaration lookup, and lldb is reacting badly to clang performing those lookups. The former part is possibly a bug, but we sometimes really do want to do a full lookup for redeclaration checking, particularly for `-Wshadow`. In general we reserve the right to look up names even when it might not be obvious why, so a clang API consumer shouldn't be assuming otherwise.

It would seem reasonable to me for lldb to pick the symbol from the current module in the case of an ambiguity, and to otherwise make all symbols visible. I think that's generally what gdb does. That said... dynamically changing how a name resolves might cause some problems, particularly in C++, since Clang caches lookup results from `ExternalASTSource`s in the `DeclContext` lookup tables. If that turns out to be a problem, I think we'll either need to push some part of this disambiguation down into Clang itself or make LLDB invalidate those caches when the current module is switched.


https://reviews.llvm.org/D39307