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

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
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) {}
----------------
george.burgess.iv wrote:
> 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?
I think the problem is that we sometimes assume MemLocs are precise, and sometimes assume they're conservative.  :)

I'm fine with you landing with either default.  I defer to your judgement.  



https://reviews.llvm.org/D44748