[PATCH] D28162: [ADT] Delete RefCountedBaseVPTR.

[Justin Lebar via Phabricator via cfe-commits]

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