werat added a comment.
Thanks for the prompt review!
> This change is wrong for ValueObjectConstResult's. The original point of
> ValueObjectConstResult's was to store the results of expressions so that,
> even if the process is not at the original stop point, you could still check
> the value. It should have the value it had at the time the expression was
> evaluated. Updating the value is exactly what you don't want to do. So I
> think you need to make sure that we can't change their value after they are
> created.
Looking at the current state of things and the available APIs, I don't think
this principle holds anymore. `ValueObjectConstResult` is also used for values
created via `SBTarget::CreateValueFromData`. As a user I don't see any reason
why I shouldn't be allowed to modify the value of an object I created myself
earlier. I would argue the same for the results of the expression evaluation.
Why are some values different from others?
> Note, ExpressionResult variables are different from
> ExpressionPersistentVariables. The former should not be updated, and after
> the process moves on any children that weren't gathered become "unknown". But
> ExpressionPersistentVariables should be able to be assigned to, and when the
> reference target object, it should update them live. .
Please, correct me if I'm wrong. In my example `ExpressionResult` is a value
returned by `EvaluateExpression` (i.e. `b`) and `ExpressionPersistentVariable`
is variable created by the expression evaluator (i.e. `$b_0`), right? As far as
I can tell, they're both `ValueObjectConstResult` and both have the problem
outlined in this patch:
frame0.EvaluateExpression("auto $b_0 = b1")
b = frame0.FindValue("$b_0", lldb.eValueTypeConstResult)
...
# Same problem, updating the value of `b` doesn't invalidate children.
> The other thing to check about this patch is whether it defeats detecting
> changed values in child elements.
This patch invalidates children only in `ValueObject::SetNeedsUpdate()` which
is called only from `SetData/SetValueFromCString` as far as I understand. If
you have a `ValueObjectVariable` which tracks some variable that can be
modified by the process, then it will continue to work fine.
`TestValueVarUpdate.py` passes successfully, but I didn't look to0 closely yet
whether it actually tests the scenario you described.
---
Looking at this from the user perspective, I would prefer to be able to update
any value, regardless of which API it came from. In my use case I rely on this
heavily --
https://werat.dev/blog/blazing-fast-expression-evaluation-for-c-in-lldb/#maintaning-state.
I could potentially live with the results of `EvaluateExpression` being
immutable, but being able to modify values created by `CreateValueFromData` or
persistent values like `$b_0` is necessary for me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105470/new/
https://reviews.llvm.org/D105470
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
