Mark derived destructors as `override`

[antoniofrighetto]

Summary

Derived destructors can be marked as override, in order to prevent possible compilation failures of projects depending on those headers (when compiled with flags -Wall, -Wsuggest-destructor-override, -Winconsistent-missing-destructor-override).

[dblaikie]

llvm/include/llvm/Analysis/ScalarEvolution.h

227

Why is this being made virtual?

llvm/include/llvm/IR/IRBuilder.h

80

Please drop virtual when adding override.

[antoniofrighetto]

llvm/include/llvm/Analysis/ScalarEvolution.h

227

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.

[dblaikie]

llvm/include/llvm/Analysis/ScalarEvolution.h

227

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.

[antoniofrighetto]

@dblaikie: That's true, admittedly, had not noticed the protected dtor (which works as well). Fixed it up, thanks for contextualizing.

llvm/include/llvm/Analysis/ScalarEvolution.h

227

That's true, admittedly, I didn't notice the protected dtor.

[dblaikie]

Looks great, thanks!

llvm/include/llvm/Analysis/ScalarEvolution.h

class SCEVPredicate : public FoldingSetNode {
   /// ScalarEvolution's BumpPtrAllocator holds the data.
   FoldingSetNodeIDRef FastID;
 
 public:
   enum SCEVPredicateKind { P_Union, P_Compare, P_Wrap };
 
 protected:
   SCEVPredicateKind Kind;
-  ~SCEVPredicate() = default;
+  virtual ~SCEVPredicate() = default;
   SCEVPredicate(const SCEVPredicate &) = default;
   SCEVPredicate &operator=(const SCEVPredicate &) = default;
 
 public:
   SCEVPredicate(const FoldingSetNodeIDRef ID, SCEVPredicateKind Kind);
 
   SCEVPredicateKind getKind() const { return Kind; }
 

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

 
 public:
   DWARFContext(std::unique_ptr<const DWARFObject> DObj,
                std::string DWPName = "",
                std::function<void(Error)> RecoverableErrorHandler =
                    WithColor::defaultErrorHandler,
                std::function<void(Error)> WarningHandler =
                    WithColor::defaultWarningHandler);
-  ~DWARFContext();
+  ~DWARFContext() override;
 
   DWARFContext(DWARFContext &) = delete;
   DWARFContext &operator=(DWARFContext &) = delete;
 
   const DWARFObject &getDWARFObj() const { return *DObj; }
 
   static bool classof(const DIContext *DICtx) {
     return DICtx->getKind() == CK_DWARF;

llvm/include/llvm/IR/IRBuilder.h

 };
 
 /// Provides an 'InsertHelper' that calls a user-provided callback after
 /// performing the default insertion.
 class IRBuilderCallbackInserter : public IRBuilderDefaultInserter {
   std::function<void(Instruction *)> Callback;
 
 public:
-  virtual ~IRBuilderCallbackInserter();
+  virtual ~IRBuilderCallbackInserter() override;
 
   IRBuilderCallbackInserter(std::function<void(Instruction *)> Callback)
       : Callback(std::move(Callback)) {}
 
   void InsertHelper(Instruction *I, const Twine &Name,
                     BasicBlock *BB,
                     BasicBlock::iterator InsertPt) const override {
     IRBuilderDefaultInserter::InsertHelper(I, Name, BB, InsertPt);