[IR] Use LLVM_ENABLE_ABI_BREAKING_CHECKS to guard ABI changes.

[varungandhi-apple]

Summary

Incorrect usage of NDEBUG to guard ABI changes can prevent clients
from enabling assertions for their C++ code while having assertions in
LLVM turned off. So we use LLVM_ENABLE_ABI_BREAKING_CHECKS instead, as
described in llvm/docs/ProgrammersManual.rst. Most types already use this
macro, however, there were a couple of stragglers in ValueHandle.h, which
are fixed by this revision.

[dblaikie]

Looks good to me - couple of side comments, possibly more directed at other reviewers to discuss/consider.

llvm/include/llvm/IR/ValueHandle.h

290

Follow-up/preliminary commit could probably remove the <ValueTy> from this line, I think.

447

Would it be OK (follow-up/preliminary commit) to move final out of this block? So the type is always final - or does that break something else?

[dexonsmith]

llvm/include/llvm/IR/ValueHandle.h

447

Interesting; I hadn't considered putting final on a class that doesn't inherit from anything, but I imagine you're right and this could be moved out.

[dexonsmith]

(LGTM too)

[varungandhi-apple]

Going to land this as-is, will make suggested tweaks in a follow-up change.

[dblaikie]

llvm/include/llvm/IR/ValueHandle.h

489

Missed this usage (probably worth building with/without ABI breaking checks in the future to validate these sort of changes) which breaks the non-abi-breaking-checks build. Oh, and some death unit tests no longer worked/needed to be protected by ABI_BREAKING_CHECKS as well as !NDEBUG.

Fixed in: 3503c856819efc01ce210fa56e597ddfb7a4c1a1

llvm/include/llvm/IR/ValueHandle.h

 /// delete occurs.
 ///
 /// Note that an AssertingVH handle does *not* follow values across RAUW
 /// operations.  This means that RAUW's need to explicitly update the
 /// AssertingVH's as it moves.  This is required because in non-assert mode this
 /// class turns into a trivial wrapper around a pointer.
 template <typename ValueTy>
 class AssertingVH
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
     : public ValueHandleBase
 #endif
-  {
   friend struct DenseMapInfo<AssertingVH<ValueTy>>;
 
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   Value *getRawValPtr() const { return ValueHandleBase::getValPtr(); }
   void setRawValPtr(Value *P) { ValueHandleBase::operator=(P); }
 #else
   Value *ThePtr;
   Value *getRawValPtr() const { return ThePtr; }
   void setRawValPtr(Value *P) { ThePtr = P; }
 #endif
   // Convert a ValueTy*, which may be const, to the raw Value*.
   static Value *GetAsValue(Value *V) { return V; }
   static Value *GetAsValue(const Value *V) { return const_cast<Value*>(V); }
 
   ValueTy *getValPtr() const { return static_cast<ValueTy *>(getRawValPtr()); }
   void setValPtr(ValueTy *P) { setRawValPtr(GetAsValue(P)); }
 
 public:
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   AssertingVH() : ValueHandleBase(Assert) {}
   AssertingVH(ValueTy *P) : ValueHandleBase(Assert, GetAsValue(P)) {}
   AssertingVH(const AssertingVH &RHS) : ValueHandleBase(Assert, RHS) {}
 #else
   AssertingVH() : ThePtr(nullptr) {}
   AssertingVH(ValueTy *P) : ThePtr(GetAsValue(P)) {}
   AssertingVH(const AssertingVH<ValueTy> &) = default;
 #endif
 
   operator ValueTy*() const {
     return getValPtr();
   }
 
   ValueTy *operator=(ValueTy *RHS) {
     setValPtr(RHS);

 /// in the map finds a handle from the old value.
 ///
 /// Note that a PoisoningVH handle does *not* follow values across RAUW
 /// operations. This means that RAUW's need to explicitly update the
 /// PoisoningVH's as it moves. This is required because in non-assert mode this
 /// class turns into a trivial wrapper around a pointer.
 template <typename ValueTy>
 class PoisoningVH
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
     final : public CallbackVH
 #endif
 {
   friend struct DenseMapInfo<PoisoningVH<ValueTy>>;
 
   // Convert a ValueTy*, which may be const, to the raw Value*.
   static Value *GetAsValue(Value *V) { return V; }
   static Value *GetAsValue(const Value *V) { return const_cast<Value *>(V); }
 
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   /// A flag tracking whether this value has been poisoned.
   ///
   /// On delete and RAUW, we leave the value pointer alone so that as a raw
   /// pointer it produces the same value (and we fit into the same key of
   /// a hash table, etc), but we poison the handle so that any top-level usage
   /// will fail.
   bool Poisoned = false;
 
   Value *getRawValPtr() const { return ValueHandleBase::getValPtr(); }
   void setRawValPtr(Value *P) { ValueHandleBase::operator=(P); }
 
   /// Handle deletion by poisoning the handle.
   void deleted() override {
     assert(!Poisoned && "Tried to delete an already poisoned handle!");
     Poisoned = true;
     RemoveFromUseList();
   }
 
   /// Handle RAUW by poisoning the handle.
   void allUsesReplacedWith(Value *) override {
     assert(!Poisoned && "Tried to RAUW an already poisoned handle!");
     Poisoned = true;
     RemoveFromUseList();
   }
-#else // NDEBUG
+#else // LLVM_ENABLE_ABI_BREAKING_CHECKS
   Value *ThePtr = nullptr;
 
   Value *getRawValPtr() const { return ThePtr; }
   void setRawValPtr(Value *P) { ThePtr = P; }
 #endif
 
   ValueTy *getValPtr() const {
     assert(!Poisoned && "Accessed a poisoned value handle!");
     return static_cast<ValueTy *>(getRawValPtr());
   }
   void setValPtr(ValueTy *P) { setRawValPtr(GetAsValue(P)); }
 
 public:
   PoisoningVH() = default;
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   PoisoningVH(ValueTy *P) : CallbackVH(GetAsValue(P)) {}
   PoisoningVH(const PoisoningVH &RHS)
       : CallbackVH(RHS), Poisoned(RHS.Poisoned) {}
 
   ~PoisoningVH() {
     if (Poisoned)
       clearValPtr();
   }