jimingham wrote:

> Hello, @jimingham , first of all - sorry for the long delay in reply.
> 
> I carefully read through all your messages once again and found, that I 
> totally misunderstood some places, i.e. "Why isn't it enough to have the 
> ValueObjectSynthetic's SetValueFromCString do this" - I think it should be 
> enough, I'll fix it.
> 
> > Secondly, lldb only supports changing scalars because it's hard to give 
> > meaning to "changing an aggregate object".  You have to change it in situ - 
> > which really means changing the values of its contents, i.e. its children - 
> > or you will end up invalidating code that relies on the location of the 
> > aggregate
> 
> I agree with this, but I think there is not so much aggregates for which it 
> makes sense to change their length as for strings (at least in libcxx) - I 
> mean it is natural to update whole string to completely different value, but 
> it is not natural to do so for e.g. vector. In case of strings, one might 
> want to set string longer, than the one he has now for the debug purposes, so 
> this will indeed invalidate code, that relies on the pointers that was 
> obtained through `.data` or `.c_str` methods, but it is the programmer 
> responsibility to care about this. This it the same behaviour as if you 
> change your string in the program - you should update your pointers.

It's not just pointers, of course.  If you've done:

size_t string_len = my_string.size();

then string_len will be wrong.  But I agree, that's more the lookout of whoever 
is messing with their strings while running.

> 
> > However, I think it's confusing here to start with the model that 
> > ValueObjects with SCPs have "underlying strings".
> 
> Sorry, I think that I didn't express my thoughts carefully, by the underlying 
> string I didn't mean, that we have some string in the SCP or ValueObjects, I 
> meant the strings in the code that is under debug.
> 
> > By the way, as a side note, watching the part of your example where you 
> > change the raw string guts, it looks like we don't update summaries of a 
> > ValueObject if one of its children gets changed.  Be good to file bug on 
> > that one so we don't forget.
> 
> I'm not sure that this bug might be reproduced without the string example, I 
> don't know which type have the summary which represent all its children. Is 
> it ok, to file a bug with current strings example or how to do it better?
> 
> > In the case of std::strings, which seems to be your primary motivator here, 
> > I think considering the string value to be the summary makes the most sense
> 
> And in the end, may I kindly ask you to clarify your position about these 
> changes please? Do you suggest to return to the `SetSummaryFromCString` API?

The more I think about it, the more it seems that SetSummaryFromCString is just 
the wrong API to use to change the value of something.  After all, Summaries 
don't actually represent the "value" of the ValueObject, they are just whatever 
bits of that value the Summary Provider decided to extract, plus whatever 
additional text they wanted to provide.  Summaries may not even be about the 
value at all, for instance for checked pointers they might just say the checked 
state.  It's clear that's not the entity you would ask to change that object's 
value.

`std::string` and `char *` seem different because we show the string contents 
in the summary, and people like to pretend that the "value" of a std::string or 
char * is the extracted string contents, though that's not really true.

So I don't think trying to change underlying values makes sense as part of the 
Summary feature.

Changing values of a ValueObject with a Synthetic Child Provider definitely 
makes sense if directed to the children the synthetic child provider shows.  If 
the SCP for some value produces an `int` child named `my_made_up_child`, then 
it would make sense to ask it if it can change the value of whatever underlies 
`my_made_up_child`.  That also has a straight-forward interface, you get a 
SCP's child, and set its value, which the SCP Frontend intercepts, and does 
what is needed to change the value if it can.

If we were presenting the std::string contents as an array of char's then 
changing the individual characters by addressing the members of the array would 
make sense.  Note, that's also the only interface that will work for 
std::string's in full generality because std::strings aren't required to hold 
null-terminated C-strings.  

I'm a little bothered by the notion of changing the whole of a SyntheticValue 
by providing some C-string.  That makes sense for a restricted representation 
of the new values of something that stores string-y objects, but doesn't make 
sense without further syntax for anything else.  It's also not the right 
implementation internally for changing the child provided by an SCP, since it 
that case you'd call SetValueFromCString on the child, and route that to the 
parent.  

But having this in place wouldn't preclude adding machinery to get the SCP to 
effect a change for one of the children they produced.  And I guess we can make 
some attempt to define what this actually means the second time someone uses it.

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

Reply via email to