[Lldb-commits] [lldb] [lldb] Add support for updating string during debug process (PR #67782)

[via lldb-commits]

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