[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)

[Michael Buch via lldb-commits]

Michael137 wrote:

> I'm not sure I understand because this will break the logic that is currently in place. Many nuances in post-processing wouldn't be possible to implement with a simple short-circuit. If a symbol is found in the hinted module, it might be internal. `IRExecutionUnit::FindInSymbols` processes the results by preferring external symbols over internal ones. If I short-circuit the search after the first match in the hinted module `IRExecutionUnit::FindInSymbols` just wouldn't see potentially more preferable results. On the other hand `SymbolContext::FindBestGlobalDataSymbol` prefers symbols of any type from the hinted module over the other symbols. But it still prefers external symbols over internal ones. So the priority order looks like this: `hint external > hint internal > other external > other internal`. Moreover, `SymbolContext::FindBestGlobalDataSymbol` verifies that there are no multiple matches found, but it's tricky as well. It checks if there's a result in the module - it does verification only matches from the module, otherwise it verifies all the results. Perhaps I can't explain all the nuances quite accurately, you should look at the post-processing code itself in these functions. But the most important thing is that we don't have a full understanding of when we can stop inside FindFunction/FindSymbolsWithNameAndType.

 Sorry, just getting back to this. Got lost in my review queue.

Ok thanks for pointing this out. I didn't realize `Resolve` was tracking the internal load address and only succeeding when it found an external symbol. That's quite a subtle API. I would agree with Greg's expectation of how symbol resolution should work when evaluating expressions. Starting with the most local match, search outward until we find something appropriate to call.

What happens if we stop preferring the external symbols over internal ones in `IRExecutionUnit::FindInSymbols`? What tests break?

https://github.com/llvm/llvm-project/pull/102835