[PATCH] D121993: Mark derived destructors as `override`

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm/Analysis/ScalarEvolution.h:227
   SCEVPredicateKind Kind;
-  ~SCEVPredicate() = default;
+  virtual ~SCEVPredicate() = default;
   SCEVPredicate(const SCEVPredicate &) = default;
----------------
antoniofrighetto wrote:
> dblaikie wrote:
> > Why is this being made virtual?
> Might be wrong, but, considering that `SCEVPredicate` is being inherited, the order of destruction object should be carried out correctly, when a derived object goes out of scope (even if no instance of the derived object is reachable through a pointer to the base class) . That actually should be added at every level of the hirearchy.
If the type isn't owned/destroyed dynamically, then it isn't necessary to make the dtor virtual - that's why the dtor's protected, so it can't be destroyed dynamically.

eg, this code is OK:
```
struct base {
  virtual void f();
protected:
  ~base();
};
struct derived final : base {
  void f();
};
void f(base *b) {
  b->f();
}
...
derived d;
f(&d);
```
The destruction is non-polymorphic/non-dynamic even though the usage may be dynamic.

This class is designed to work in this way & Clang's warnings have been implemented to allow this usage & avoiding dynamic dispatch overhead where it isn't needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121993