jlebar marked an inline comment as done.
jlebar added inline comments.
================
Comment at: llvm/unittests/ADT/IntrusiveRefCntPtrTest.cpp:14-37
+struct VirtualRefCounted : public llvm::RefCountedBase<VirtualRefCounted> {
+ VirtualRefCounted() { ++NumInstances; }
+ VirtualRefCounted(const VirtualRefCounted &) { ++NumInstances; }
+ virtual ~VirtualRefCounted() { --NumInstances; }
virtual void f() {}
+
+ static int NumInstances;
----------------
dblaikie wrote:
> I'm not sure we even need test coverage for this anymore.
>
> If IntrusiveRefCntPtr calls any dtor (& we should have some coverage of that,
> to be sure) then it'd be basically impossible for it to call the dtor in a
> way that wouldn't be compatible with that dtor dispatching if it was virtual.
> (ie: this test sort of boils down to testing virtual dispatch of virtual
> dtors - a feature of the compiler, not of this library)
>
> But up to you.
sgtm. I got rid of the virtual test and moved the explicit checking for
destructor calls into the non-virtual test below. Will land the patch with
that change.
https://reviews.llvm.org/D28162
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
