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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
