[Lldb-commits] [PATCH] D117139: [lldb] Copy ScriptedInterface object instance to a new StructuredData object

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D117139#3240564 <https://reviews.llvm.org/D117139#3240564>, @mib wrote:

> In D117139#3240436 <https://reviews.llvm.org/D117139#3240436>, @labath wrote:
>
>> I'd like to remind everyone of a not-very-widely-known, but incredibly nifty feature of std::shared_ptr, called "aliasing". It allows one to create a shared_ptr which points to one object, but deletes a completely different object when it goes out of scope. It works like so:
>>
>>   std::shared_ptr<A> a = std::make_shared<A>(...); // or whatever
>>   std::shared_ptr<B> b(a, getB(a)); // will point to a B, but call `delete A` when it goes out of scope
>>
>> The model use case is when the first object is a subobject (field) of the second one (so we'd have `struct A { B b; ... };` and `getB(a)` would be `&a->b`), but that is not a a requirement. B can be a completely arbitrary object -- the only requirement is that B remains live for as long as A is around.
>
> Very neat indeed, but I don't think it will be of any use here unfortunately: IIUC in your example, `b` is the first to go out-of-scope which calls `a` destructor. In my case, `a` when out of scope but it still referenced by `b`.

That is perfectly fine. All of the shared pointers created in this way share their reference counts. It doesn't matter which one goes out of scope first, but when the last one (whichever it is) does, it will call `delete` on the original object (a). (It is then expected that the deletion of a will somehow ensure that `b` gets deleted as well -- that is trivially true if b is a member, but may need additional logic in other cases.)
So, if the following holds:

- `a` is necessary for the proper usage of `b` (e.g. because `b` holds a pointer back to `a`)
- deletion of `a` will (should) result in deletion of `b`

then you can still use this pattern.

>> Now, I don't understand this code well enough to say whether that could/should be used here, but the (few) parts which I did understand led me to think that there are some subobjects (in the broadest sense of the word) being passed around, and so I have a feeling we should at least consider this option -- it seems like it would be better to pass around objects with explicit ownership semantics instead of raw pointers.
>
> I agree with you on theory but here, the `ScriptedThreadInterface` tries to be language-agnostic that's why I pass `StructuredData::GenericSP` around.

GenericSP is fine(ish), but in this patch i see a lot of void*'s floating around (which (for an uninitiated reader) is even worse than a raw pointer, since he doesn't even know what kind of an object is that referring to, let alone the ownership semantics). btw, std::shared_ptr<void> is also a thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117139