[Lldb-commits] [PATCH] D67589: Fix crash on SBCommandReturnObject & assignment
Jan Kratochvil via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 26 13:46:14 PDT 2019
jankratochvil updated this revision to Diff 222021.
jankratochvil edited the summary of this revision.
jankratochvil added a comment.
Changed @clayborg's `lldb_private::CommandReturnObjectImpl` to `lldb_private::SBCommandReturnObjectImpl` - both variants are used in LLDB `SB*.cpp` and the `SB*` looks more logical to me.
Used @clayborg's `lldb_private::SBCommandReturnObjectImpl::m_owned` with conditional in dtor but personally I would use two inherited classes + virtual dtor instead.
In D67589#1676511 <https://reviews.llvm.org/D67589#1676511>, @labath wrote:
> I think I would like that better than the swap trick. Since you're now inside a pimpl, you can replace the two members with a llvm::PointerIntPair, if you are so inclined.
Not done. I proposed `llvm::PointerIntPair` originally to match your goal of keeping single pointer in `SBCommandReturnObject`. But when there is already a `new` instance of `lldb_private::CommandReturnObjectImpl` then `llvm::PointerIntPair` therein would be more a burden both in the source code and for CPU.
| `m_opaque_up` type | ABI compatible | CPU overhead | formal http://lldb.llvm.org/resources/sbapi.html compliance |
| `shared_ptr` with two deleters | no (size 16) | high ('lock' instructions) | yes (a bug in the sbapi.html doc?) |
| `llvm::PointerIntPair` | yes (size 8) | low | no (sort of) |
| `unique_ptr<SBCommandReturnObjectImpl>` | yes (size 8) | high (extra object allocation) | yes |
But I am fine with this patch implementing `SBCommandReturnObjectImpl` as this is not any performance critical part of code.
CHANGES SINCE LAST ACTION
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 17688 bytes
Desc: not available
More information about the lldb-commits