[Lldb-commits] [PATCH] D98370: [lldb] Fix SBValue::Persist() for constant values

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D98370#2704597 <https://reviews.llvm.org/D98370#2704597>, @werat wrote:

> In D98370#2686515 <https://reviews.llvm.org/D98370#2686515>, @jingham wrote:
>
>> If persisting already persistent variables is indeed the only failure case, then I wonder if it wouldn't be more straightforward to just see if the ValueObject is already a persistent variable and have Persist just return the incoming variable.
>
> Persisting already persistent variables is not the only valid use case, one might also want to persist variables created via `SBTarget::CreateValueFromData()`.

Sure.  But but when I was poking around at it a little bit, it seems like the other use cases already work, and the only one that was failing was the case where you call persist on a persistent variable.  If that is really true, then maybe we should fix the failing case directly.

> I guess it is currently not clear from the API, but I would expect `SBValue::Persist()` to produces a new value every time:
>
>   auto v1 = value.Persist();
>   auto v2 = value.Persist();
>   
>   assert(v1.GetName() != v1.GetName())  // modulo comparing char*...

Not sure why?  The API is a request:  "I made a variable somehow, and I would like you to make it persist so I can use its value later on even if the underlying data has changed."  Why do you care whether you get a copy of an already persistent or just a shared value?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98370/new/

https://reviews.llvm.org/D98370