> On Jul 10, 2021, at 6:18 AM, Andy Yankovsky via Phabricator 
> <revi...@reviews.llvm.org> wrote:
> 
> werat added a comment.
> 
> Thanks for the explanation! But at this point I feel I'm a bit confused about 
> how it all _supposed_ to work in the current design :)
> 
> If I understand correctly, there are four "types" of values from the user 
> (API) perspective:
> 
> 1. `ExpressionResult` -- value returned by `SBFrame::EvaluateExpression()`
> 2. `ExpressionPersistentVariable` -- value created by the expression via 
> `auto $name = ...` syntax. Can be obtained by `SBFrame::FindValue("$name", 
> lldb::eValueTypeConstResult)`.
> 3. "Const value" -- value created by `SBTarget::CreateValueFromData()` or 
> `SBTarget::CreateValueFromAddress`
> 4. "Variable reference" -- value returned by `SBFrame::FindVariable()`
> 
> For which of these value the following test is supposed to work?
> 
>  struct Foo { int x; };
>  Foo* f1 = { .x = 1}
>  Foo* f2 = { .x = 2}  # pseudo-C for simplicity
> 
>  f1_ref = ...  # Get a value that holds the value of `f1` using one of the 
> four methods described above
>  print(f1_ref.GetChild(0))  # '1'
>  f1_ref.SetValueFromCString(frame.FindVariable('f2').value)
>  print(f1_ref.GetChild(0))  # '2'
> 
> My experiments show that it works for "variable references" and "const 
> values" created by `CreateValueFromAddress` (but _not_ `CreateValueFromData`).
> If I understand your comment correctly, you're saying it should work only for 
> `ExpressionPersistentVariable` values (#2). Is that right?

No, that is not what I meant to say.  

Of the 4 cases above (there's also Register SBValues BTW) I was only arguing 
that #1 should not be settable.  After all, you might have done:

expr 5 + 5
(int) $0 = 10

I'm not sure what $0 = 11 would mean.  But beyond that, they are useful for 
freezing values for later consultation, so you want to make sure that once the 
process has changed state
they won't try to update themselves, accidentally or on purpose.

ValueObjectVariable objects should definitely be settable.  That's how most 
GUI's would implement choosing a child from a Variables display and changing 
its value, so setting them is
actually a pretty common operation.

Persistent variables also need to be settable, since they are often used as 
scratch counters and so forth, and wouldn't be of nearly as much use if they 
weren't.

CreateValueFromAddress values relate to things in memory (and actually track 
changes in the memory underlying them when the process changes state).  Since 
they are the way
that you can do casts to get a structured access to raw memory at the SB API 
layer, it seems reasonable that they should also be settable.

And objects made from CreateValueFromData are mostly used for artificial tasks 
like making backing ValueObjects for pieces of complex data structures like 
dictionaries, maps, etc
then having them be settable is also appropriate.

The other concern I have is that all these types of values that are used for 
"local variable display" like the Variable, FromAddress & FromData ones need to 
support IsChanged.  
That's why it struck me as wrong that a general sounding method like 
SetNeedsUpdating was throwing away the already made child values.  To do a good 
job of presenting "is changed" 
results you need to know both what children had been fetched (in a GUI this 
corresponds to what values the user has seen, so those are the ones you're 
supposed to check) 
and their old values.  So discarding the children except in cases where you 
know you are going to reset them completely is likely to get in the way of that.

I do agree the current implementation has gotten a little confused, but I think 
what we want to have happen is fairly clear, as evinced by the fact that we 
actually agreed on
the correct behaviors...

Jim


> 
> I don't have the full picture about the internal implementation and all the 
> use cases, but as a user I would expect it to work for at least #2, #3 and 
> #4. Afaik there's no API to fully distinguish between these kinds of values, 
> so I find it confusing why `SBValue::SetData()` would be allowed for some 
> values and not allowed for others. If I can create a value using 
> `CreateValueFromData` and then there's a method `SetValueFromCString`, then I 
> don't see why it should not be allowed (apart from implementation 
> complexity/consistency reasons).
> 
> What do you think? How should we proceed with this?
> 
> 
> 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

Reply via email to