[PATCH] D137955: [AST] Make AliasSetTracker work on BatchAA

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

I have no problem with the idea of the patch, but I think this needs to be split into two patches.

The first simply removes support for ASTs over mutating IR, and makes the API changes required.  (Note the suggestion about asserting value handl inline.)  This should land on it's own so that if we missed a remaining mutation point, we get a chance to smoke out the failure without reasoning about potential confusing AA results at the same time.

The second actually uses batch-aa.



================
Comment at: llvm/lib/Analysis/AliasSetTracker.cpp:626
 void AliasSetTracker::ASTCallbackVH::deleted() {
-  assert(AST && "ASTCallbackVH called with a null AliasSetTracker!");
-  AST->deleteValue(getValPtr());
-  // this now dangles!
+  llvm_unreachable("AliasSetTracker must work on immutable IR");
 }
----------------
This bit really doesn't look right.  

I suspect that the ASTCallbackVH should now be a AssertingVH.  At a minimum, this needs to be a well written assert or fatal error, not an unreachable.  


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137955/new/

https://reviews.llvm.org/D137955