[PATCH] D44748: Track whether the size of a MemoryLocation is precise

[Philip Reames via Phabricator via llvm-commits]

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

LGTM w/one optional, but strongly recommended change.



================
Comment at: include/llvm/Analysis/MemoryLocation.h:88
+  // this assumes the provided value is precise.
   constexpr LocationSize(uint64_t Raw)
       : Value(Raw > MaxValue ? Unknown : Raw) {}
----------------
This implicit switch of all callers makes me nervous.  I'm willing to trust that you've audited all the callers, but my advice (which I would follow if doing this myself) would be to submit this patch with the constructor defaulting to imprecise locations.  Then, in a separate patch, flip the default.  Specifically, the motivation is so that if you've missed something in your audit, most of the infrastructure can stay in tree and not need to be reverted while you debug.  


https://reviews.llvm.org/D44748