[PATCH] D32634: Emulate TrackingVH using WeakVH

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: include/llvm/IR/ValueHandle.h:282-283
 ///
 /// It is an error to do anything with a TrackingVH whose value has been
 /// destroyed, except to destruct it.
 ///
----------------
I think we should allow assigning over the TrackingVH as well, much like we would with a moved-from object.

With this change...


================
Comment at: include/llvm/IR/ValueHandle.h:289
+  WeakVH InnerHandle;
+  bool NullIsAllowed = true;
 
----------------
... we can remove the 'null-is-allowed' aspect of the entire class.

Specifically the CheckValidity call can go awoy from setValPtr and instead is only needed for getValPtr.

I don't think this really reduces the utility of the handle in any way and it makes it much simpler IMO.


================
Comment at: include/llvm/IR/ValueHandle.h:300
+      // NullIsAllowed to differentiate between the two cases.
+      assert(NullIsAllowed && "TrackingVH was nulled out unexpectedly!");
+      return;
----------------
And I think this should go back to checking ValueHandleBase::isValid which handles things like trying to extract the value pointer from a densemap key.


https://reviews.llvm.org/D32634