[Lldb-commits] [lldb] [LLDB] Add DIL code for handling plain variable names. (PR #120971)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

This is a partial review. I ran out of steam when I reached the Lexer component. I really think that one should be a patch of its own because:
- all of the lookaheads and everything are sufficiently complicated to deserve the proper attention
- the implementation is really amenable to unit testing (all you need is a string as an input)

I also noticed that this implementation casts a much wider web when it comes to searching for global variables (it uses `Target::FindGlobalVariables`, which searches the entire process, whereas the current implementation only uses variables from the [current compile unit](https://github.com/llvm/llvm-project/blob/cbe583b0bd8d46b4e5edda463e19e6a24c0817bc/lldb/source/Target/StackFrame.cpp#L476). I think it would be best to stick to the existing implementation for now (ideally by even reusing the `GetInScopeVariableList` function), and saving the discussion about the pros and cons of changing that to a separate patch.
The reason for that is I can find at least two benefits to the current implementation (it's faster and it's guaranteed to return the same variable you would get by compiling the expression in the compiler). I think it'd be best if that was isolated from the more technical/mundane aspects of parsing an expression.

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