[PATCH] D103206: [ADT] Refactor enumerate unit tests

Scott Linder via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 4 09:29:02 PDT 2021


scott.linder added inline comments.


================
Comment at: llvm/unittests/ADT/STLExtrasTest.cpp:157
+  }
+  virtual ~Counted() { ++D; }
+};
----------------
dblaikie wrote:
> Probably doesn't need to be virtual, if no one ever destroys it polymorphically? But I guess we have warnings enabled that make that problematic?
> 
> Could we use composition instead, then? (Range contains a Counted, rather than derives from?)
Good point, I think I just reflexively made it `virtual`. I would prefer to use inheritence as it cuts down on the boilerplate in the derived classes to just `using Counted::Counted`.

It doesn't seem like we have any warnings enabled that complain here, so I just made the destructor non-virtual


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103206/new/

https://reviews.llvm.org/D103206



More information about the llvm-commits mailing list