labath wrote:

> This type of patch also slightly worries me in that it could make it harder 
> to unify the LLDB dwarf data structures with the LLVM ones

That is something I considered, but I think this might actually help with that. 
At this point the LLDB and llvm dwarf classes are so different, that the only 
way I see to unify them is to do it piecemeal, and this kind of decoupling 
helps with that. So, if you wanted to replace LLDB DWARFUnit with llvm's, you 
could do that without touching DWARFExpression. It's true lldb's DWARFUnit now 
inherits from some DWARFExpression thingy, but that's easy to replace with 
composition. I alluded to that in the earlier comments, but in the end I didn't 
insist on it. Maybe I should have. If you think it's better to do that now, I'd 
be certainly fine with that.

This doesn't help with replacing LLDB's DWARFExpression with the llvm version, 
but I don't think it makes it worse either. I see a couple of possible ways 
forwards there:
- introduce a similar abstraction to llvm DWARFExpression. I think that would 
make sense since dwarf expressions are used in contexts (eh_frame) where there 
is no DWARFUnit to speak of.
- replace (non-unwinding) uses of DWARF Expression with a higher level 
abstraction. It's use in Function and Variable classes is breaking 
abstractions, and the PDB plugin has to jump through hoops to create DWARF 
expressions for these objects. That would leave DWARFExpression only used for 
unwinding in the core code, where it would be easier to replace this with 
llvm::DWARFExpression (and at that point, it may not matter that it depends on 
llvm::DWARFUnit, since this class -- unlike the lldb counterpart -- does not 
depend on the whole world)

https://github.com/llvm/llvm-project/pull/131645
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to