[PATCH] D29006: [PH] Remove all use of AssertingVH from members of analysis results.

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

Hi Chandler,

I'm not sure this is a good idea.

If this is the *only* path forward then I'll be okay going ahead with
it, but I think `AssertingVH` (not unlike most other similar things)
is useful and provides a superset of functionality provided by
sanitizers.

> For example, a debug-build-only
>  WeakVH and an explicit assert that it hasn't gone null when it is *used*
>  so that it can still *exist* even when the value goes away.

But we don't *have* such a thing today, and for that reason this
change is a strict decrease in debug-ability.  If you wanted to add
something like that, and replace existing `AssertingVH` uses with it
then I may feel better about this; however `AssertingVH` is stronger
than what you suggest since `AssertingVH` catches cases where we've
keyed a hash table off `AssertingVH` s.  To be on part with that
functionality, we'd also need to write data structures like `DenseMap`
that can be "poisoned" by a `CallbackVH` key.


https://reviews.llvm.org/D29006