This is the mail archive of the
mailing list for the GCC project.
Re: [range-ops] patch 01/04: types for VR_UNDEFINED and VR_VARYING
On 7/24/19 12:33 PM, Richard Biener wrote:
> On July 24, 2019 8:18:57 PM GMT+02:00, Jeff Law <firstname.lastname@example.org>
>> 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);
> 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
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.