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

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added a comment.

Woo! Thank you very much for the review (and for bearing with me in your repeated requests to minimize this patch :) ).

Since there's quite a few pieces to this, I'll probably just land it all slowly over a few days, pending our last discussion here being settled.



================
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) {}
----------------
reames wrote:
> 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.  
I'm not sure I understand. All MemoryLocations are implicitly precise today, so the most functionality-preserving thing we can do here is to treat all of them as precise by default.

Treating them all as imprecise would certainly fix any existing, yet-to-be-reported bugs where we're passing 'imprecise' MemoryLocations around. The hope is to find those places as a part of the migration to explicitly use `::precise`/`::upperBound`, though.

Does that make sense? Or am I missing something?


https://reviews.llvm.org/D44748