[Lldb-commits] [PATCH] D67589: Fix crash on SBCommandReturnObject & assignment

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D67589#1672686 <https://reviews.llvm.org/D67589#1672686>, @jankratochvil wrote:

> In D67589#1670966 <https://reviews.llvm.org/D67589#1670966>, @labath wrote:
>
> > I agree that this approach isn't nice, but probably there isn't a nice approach to this at this point. One possibility you could consider is storing a shared_ptr<CommandRO> inside SBCommandRO and encoding the ownership into the deleter of the shared_ptr (regular deleter => owned, noop deleter => unowned).
>
>
> Maybe also possibly cheaper `llvm::PointerIntPair` representing an associated `bool` for the deletion.


We couldn't use PointerIntPair due to the our abi stability promise (http://lldb.llvm.org/resources/sbapi.html). One could try to implement something like that by hand, but I don't think it's worth the trouble.



================
Comment at: lldb/packages/Python/lldbsuite/test/api/command-return-object/main.cpp:18
+    result = subcommand(dbg, "help");
+    result = result;
+    if (!result.Succeeded())
----------------
jankratochvil wrote:
> jankratochvil wrote:
> > labath wrote:
> > > Is that intentional? If so, why?
> > It has no purpose there for this bugfix but when already writing a testcase I wanted to test this generally fragile functionality.
> > 
> If you mean the line:
> ```
> +     if (!result.Succeeded())
> ```
> That is not really needed for this testcase but I think this is how a real world implementation should behave so why not here, just 2 lines of code.
> 
Nope, I meant the "result = result" line. Testing self-assignment is fine, but you may want to call that out explicitly, as otherwise someone may just come along and delete the "obviously useless" code.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67589