slinder1 wrote:

> I still don't necessarily agree that this is a required step for something 
> usable as it changes nothing about DIExpression to use. If you want to add on 
> such facilities later we can talk about how to design those in.

I think this _does_ change DIExpression, though. As-is the change impacts (in 
no particular order):

* Future work on `entry_value`
* Future work on optimizing `DIExpression`s
* The actual code around `DIExpression`, to the point of adding a whole 
feature-conflict-resolution aspect to `isValid`. Anyone working on any of this 
code in the future will need to consider control-flow.
* The soundness of code which already edits `DIExpression`s

I think @Snowy1803 gave a perfect example for that last point:

> What happens if a frontend creates a DIExpression with branching and without 
> an arglist, and then salvageDebugInfo, through an optimization, rewrites the 
> argument to use an arglist when a binary instruction is deleted? Would it 
> just crash? I think that if an expression can't be lowered due to an 
> implementation detail, the debug value should just be killed. Hopefully 
> support for DIArgList can be added later.

I think in this case we will either `assert` or the verifier will strip _all_ 
debug-info for the module. Neither seems like what we want in a case I expect 
will immediately crop up in any real use of the feature.

IMO it is a stretch to claim adding this has no impact on `DIExpression`, when 
the only way to achieve that is to so underdefine the feature as to allow the 
compiler to effectively just drop any expression using it at any point.




https://github.com/llvm/llvm-project/pull/210850
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to