[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