[PATCH] D29061: [PM] Introduce a PoisoningVH as a (more expensive) alternative to AssertingVH that delays any reported error until the handle is *used*.

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: include/llvm/IR/ValueHandle.h:410
+
+#ifndef NDEBUG
+  /// A flag tracking whether this value has been poisoned.
----------------
chandlerc wrote:
> sanjoy wrote:
> > I know you'll hate me for this, but this needs to be conditional on `LLVM_ENABLE_ABI_BREAKING_CHECKS` also.
> No more or less than AssertingVH?
> 
> I assumed that the design idea of AssertingVH was that *users* must leverage LLVM_ENABLE_ABI_BREAKING_CHECKS if they expose the AssertingVH. Maybe that's wrong or maybe that's a bad model, but I'd rather the two handles follow the same model (and be fixed or changed at the same time).
> 
> Similarly, if today's users of AssertingVH are failing to use this macro, they were before my changes, and I'd rather not have to boil that ocean.
SGTM!

As I said on IRC, I was under the impression that `AssertingVH` was using `LLVM_ENABLE_ABI_BREAKING_CHECKS`.  Given that it does not, this looks fine.


https://reviews.llvm.org/D29061