Add a new WeakVH value handle; NFC

[sanjoy]

Summary

WeakVH nulls itself out if the value it was tracking gets deleted, but
it does not track RAUW.

[davide]

include/llvm/IR/ValueHandle.h

37–38 ↗

(On Diff #95882)

sorting?

129–130 ↗

(On Diff #95882)

Maybe a nullable value handle is enough?

[sanjoy]

include/llvm/IR/ValueHandle.h

129–130 ↗

(On Diff #95882)

I want to be clear about *when* the value is nulled out.

[davide]

include/llvm/IR/ValueHandle.h

129–130 ↗

(On Diff #95882)

No objections.

llvm/trunk/include/llvm/IR/ValueHandle.h

 /// below for details.
 class ValueHandleBase {
   friend class Value;
 protected:
   /// \brief This indicates what sub class the handle actually is.
   ///
   /// This is to avoid having a vtable for the light-weight handle pointers. The
   /// fully general Callback version does have a vtable.
-  enum HandleBaseKind { Assert, Callback, Tracking, WeakTracking };
+  enum HandleBaseKind { Assert, Callback, Tracking, Weak, WeakTracking };
 
   ValueHandleBase(const ValueHandleBase &RHS)
       : ValueHandleBase(RHS.PrevPair.getInt(), RHS) {}
 
   ValueHandleBase(HandleBaseKind Kind, const ValueHandleBase &RHS)
       : PrevPair(nullptr, Kind), Next(nullptr), V(RHS.V) {
     if (isValid(V))
       AddToExistingUseList(RHS.getPrevPtr());
   }
 
 private:
-  PointerIntPair<ValueHandleBase**, 2, HandleBaseKind> PrevPair;
+  PointerIntPair<ValueHandleBase**, 3, HandleBaseKind> PrevPair;
   ValueHandleBase *Next;
 
   Value* V;
 
 public:
   explicit ValueHandleBase(HandleBaseKind Kind)
     : PrevPair(nullptr, Kind), Next(nullptr), V(nullptr) {}
   ValueHandleBase(HandleBaseKind Kind, Value *V)
private:
 
   /// \brief Add this ValueHandle to the use list after Node.
   void AddToExistingUseListAfter(ValueHandleBase *Node);
 
   /// \brief Add this ValueHandle to the use list for V.
   void AddToUseList();
 };
 
+/// \brief A nullable Value handle that is nullable.
+///
+/// This is a value handle that points to a value, and nulls itself
+/// out if that value is deleted.
+class WeakVH : public ValueHandleBase {
+public:
+  WeakVH() : ValueHandleBase(Weak) {}
+  WeakVH(Value *P) : ValueHandleBase(Weak, P) {}
+  WeakVH(const WeakVH &RHS)
+      : ValueHandleBase(Weak, RHS) {}
+
+  WeakVH &operator=(const WeakVH &RHS) = default;
+
+  Value *operator=(Value *RHS) {
+    return ValueHandleBase::operator=(RHS);
+  }
+  Value *operator=(const ValueHandleBase &RHS) {
+    return ValueHandleBase::operator=(RHS);
+  }
+
+  operator Value*() const {
+    return getValPtr();
+  }
+};
+
+// Specialize simplify_type to allow WeakVH to participate in
+// dyn_cast, isa, etc.
+template <> struct simplify_type<WeakVH> {
+  typedef Value *SimpleType;
+  static SimpleType getSimplifiedValue(WeakVH &WVH) { return WVH; }
+};
+template <> struct simplify_type<const WeakVH> {
+  typedef Value *SimpleType;
+  static SimpleType getSimplifiedValue(const WeakVH &WVH) { return WVH; }
+};
+
 /// \brief Value handle that is nullable, but tries to track the Value.
 ///
 /// This is a value handle that tries hard to point to a Value, even across
 /// RAUW operations, but will null itself out if the value is destroyed.  this
 /// is useful for advisory sorts of information, but should not be used as the
 /// key of a map (since the map would have to rearrange itself when the pointer
 /// changes).
 class WeakTrackingVH : public ValueHandleBase {

llvm/trunk/lib/IR/Value.cpp

for (ValueHandleBase Iterator(Assert, *Entry); Entry; Entry = Iterator.Next) {
     switch (Entry->getKind()) {
     case Assert:
       break;
     case Tracking:
       // Mark that this value has been deleted by setting it to an invalid Value
       // pointer.
       Entry->operator=(DenseMapInfo<Value *>::getTombstoneKey());
       break;
+    case Weak:
     case WeakTracking:
-      // WeakTracking just goes to null, which will unlink it from the list.
+      // WeakTracking and Weak just go to null, which unlinks them
+      // from the list.
       Entry->operator=(nullptr);
       break;
     case Callback:
       // Forward to the subclass's implementation.
       static_cast<CallbackVH*>(Entry)->deleted();
       break;
     }
   }
void ValueHandleBase::ValueIsRAUWd(Value *Old, Value *New) {
   // just have to give ValueHandleBase some kind.
   for (ValueHandleBase Iterator(Assert, *Entry); Entry; Entry = Iterator.Next) {
     Iterator.RemoveFromUseList();
     Iterator.AddToExistingUseListAfter(Entry);
     assert(Entry->Next == &Iterator && "Loop invariant broken.");
 
     switch (Entry->getKind()) {
     case Assert:
-      // Asserting handle does not follow RAUW implicitly.
+    case Weak:
+      // Asserting and Weak handles do not follow RAUW implicitly.
       break;
     case Tracking:
       // Tracking goes to new value like a WeakTrackingVH. Note that this may
       // make it something incompatible with its templated type. We don't want
       // to have a virtual (or inline) interface to handle this though, so
       // instead we make the TrackingVH accessors guarantee that a client never
       // sees this value.
 

llvm/trunk/unittests/IR/ValueHandleTest.cpp

ValueHandle()
         BitcastV(new BitCastInst(ConstantV, Type::getInt32Ty(Context))) {}
 };
 
 class ConcreteCallbackVH final : public CallbackVH {
 public:
   ConcreteCallbackVH(Value *V) : CallbackVH(V) {}
 };
 
+TEST_F(ValueHandle, WeakVH_BasicOperation) {
+  WeakVH WVH(BitcastV.get());
+  EXPECT_EQ(BitcastV.get(), WVH);
+  WVH = ConstantV;
+  EXPECT_EQ(ConstantV, WVH);
+
+  // Make sure I can call a method on the underlying Value.  It
+  // doesn't matter which method.
+  EXPECT_EQ(Type::getInt32Ty(Context), WVH->getType());
+  EXPECT_EQ(Type::getInt32Ty(Context), (*WVH).getType());
+
+  WVH = BitcastV.get();
+  BitcastV->replaceAllUsesWith(ConstantV);
+  EXPECT_EQ(WVH, BitcastV.get());
+  BitcastV.reset();
+  EXPECT_EQ(WVH, nullptr);
+}
+
 TEST_F(ValueHandle, WeakTrackingVH_BasicOperation) {
   WeakTrackingVH WVH(BitcastV.get());
   EXPECT_EQ(BitcastV.get(), WVH);
   WVH = ConstantV;
   EXPECT_EQ(ConstantV, WVH);
 
   // Make sure I can call a method on the underlying Value.  It
   // doesn't matter which method.