On 7/24/19 12:33 PM, Richard Biener wrote: > On July 24, 2019 8:18:57 PM GMT+02:00, Jeff Law <l...@redhat.com> > wrote: >> On 7/24/19 11:00 AM, Richard Biener wrote: [ Big snip, ignore >> missing reply attributions... ] >> >>>> it. But I'd claim that if callers are required not to change >>>> these ranges, then the callers are fundamentally broken. I'm >>>> not sure what the "sanitization" is really buying you here. >>>> Can you point to something specific? >>>> >>>>> >>>>> But you lose the sanitizing that nobody can change it and the >>>>> changed info leaks to other SSA vars. >>>>> >>>>> As said, fix all callers to deal with NULL. >>>>> >>>>> But I argue the current code is exactly optimal and safe. >>>> ANd I'd argue that it's just plain broken and that the >>>> sanitization you're referring to points to something broken >>>> elsewhere, higher up in the callers. >>> >>> Another option is to make get_value_range return by value and >>> the only way to change the lattice to call an appropriate set >>> function. I think we already do the latter in all cases (but we >>> use get_value_range in the setter) and returning by reference is >>> just eliding the copy. >> OK, so what I think you're getting at (and please correct me if >> I'm wrong) is that once the lattice values are set, you don't want >> something changing the recorded ranges underneath? >> >> ISTM the way to enforce that is to embed the concept in the class >> and enforce it by not allowing direct manipulation of range by the >> clients. So a client that wants this behavior somehow tells the >> class that ranges are "set in stone" and from that point the >> setters don't allow changing the underlying ranges. > > Yes. You'll see that nearly all callers do > > Value_range vr = *get_value_range (name); > > Modify > > Update_value_range (name, &vr) ; > > And returning by reference was mostly an optimization. We _did_ have > callers Changing the range in place and the const varying catched > those. Well, that's the kind of thing we want to avoid at the API level. One way was to simply prohibit changes with a by-value return and forcing changes through a setter. Another would be returning everything as a const, which is what I think your patch from today did. In both cases you have to use the appropriate APIs to make changes. I prefer the former because someone could cast away the const property, but if the former isn't really feasible, then always returning a const object is a reasonable compromise I guess.
jeff
