> On Jan 11, 2016, at 2:13 PM, Benjamin Kramer <benny....@gmail.com> wrote:
>
> On Mon, Jan 11, 2016 at 8:08 PM, Owen Anderson <resis...@mac.com
> <mailto:resis...@mac.com>> wrote:
>>
>>> On Jan 11, 2016, at 8:25 AM, David Blaikie <dblai...@gmail.com> wrote:
>>>
>>>
>>>
>>> On Sun, Jan 10, 2016 at 11:42 PM, Owen Anderson via cfe-commits
>>> <cfe-commits@lists.llvm.org> wrote:
>>> resistor created this revision.
>>> resistor added reviewers: chandlerc, bkramer, klimek.
>>> resistor added a subscriber: cfe-commits.
>>> resistor set the repository for this revision to rL LLVM.
>>> Herald added a subscriber: klimek.
>>>
>>> Managing it with IntrusiveRefCntPtr caused the virtual destructor not to be
>>> called properly.
>>>
>>> Regardless of the broader discussion on this patch, I'm confused by why
>>> this ^ would be the case. What is it that IntrusiveRefCntPtr is doing
>>> that's causing problems with destruction? (& I'm all for changing this to
>>> non-intrusive smart pointers over intrusive ones anyway, but I'd still like
>>> to understand the extra motivation here)
>>>
>>
>> ThreadSafeRefCountedBase, which classes must inherit from in order to use
>> IntrusiveRefCntPtr, does not handle virtual destructors properly. For the
>> non-thread safe version, there is RefCountBaseVPTR which solves this
>> problem. An alternative solution would have been to add a corresponding
>> ThreadSafeRefCountedBaseVPTR, but IMO at that point one might as well use
>> shared_ptr since it’s 90% of the way there.
>
> I find this surprising. ThreadSafeRefCountedBase<FileSystem> calls
> "delete static_cast<const FileSystem *>(this)". As FileSystem has a
> virtual dtor, polymorphic deletion should just work. Am I missing
> something?
I’m not an expert on this stuff, but I’ll refer you to the difference between
RefCountedBase and RefCountedBaseVPTR, and point out that
ThreadSafeRefCountedBase is like the former rather than the latter.
—Owen
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
