From: Jeff Law <law@redhat.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: Andrew MacLeod <amacleod@redhat.com>,
Aldy Hernandez <aldyh@redhat.com>,
gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [range-ops] patch 01/04: types for VR_UNDEFINED and VR_VARYING
Date: Fri, 26 Jul 2019 04:34:00 -0000 [thread overview]
Message-ID: <d4cff840-d8f4-8468-d57a-c0e85f98655c@redhat.com> (raw)
In-Reply-To: <F71C6413-6DB8-44B3-A356-B0EA507888D8@gmail.com>
On 7/24/19 12:33 PM, Richard Biener wrote:
> On July 24, 2019 8:18:57 PM GMT+02:00, Jeff Law <law@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
next prev parent reply other threads:[~2019-07-26 3:41 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-01 8:52 Aldy Hernandez
2019-07-02 20:17 ` Jeff Law
2019-07-03 9:46 ` Aldy Hernandez
2019-07-03 22:40 ` Jeff Law
2019-07-03 8:28 ` Richard Biener
2019-07-03 9:19 ` Aldy Hernandez
2019-07-03 11:08 ` Richard Biener
2019-07-03 12:23 ` Aldy Hernandez
2019-07-03 12:30 ` Aldy Hernandez
2019-07-04 10:34 ` Richard Biener
2019-07-09 7:49 ` Aldy Hernandez
2019-07-09 9:57 ` Richard Biener
2019-07-16 19:56 ` Andrew MacLeod
2019-07-22 15:38 ` Aldy Hernandez
2019-07-23 0:19 ` Jeff Law
2019-07-23 9:45 ` Richard Biener
2019-07-24 16:09 ` Jeff Law
2019-07-24 17:06 ` Richard Biener
2019-07-24 18:33 ` Jeff Law
2019-07-24 18:38 ` Richard Biener
2019-07-26 3:41 ` Jeff Law
2019-07-26 14:52 ` Andrew MacLeod
2019-07-30 9:02 ` Richard Biener
2019-07-30 15:16 ` Andrew MacLeod
2019-07-31 8:37 ` Richard Biener
[not found] ` <78846d0a-354e-b73a-6e15-123752038fb2@redhat.com>
2019-08-01 14:11 ` Richard Biener
2019-08-01 16:35 ` Andrew MacLeod
2019-07-26 4:34 ` Jeff Law [this message]
2019-07-24 20:52 ` Andrew MacLeod
2019-07-25 9:13 ` Richard Biener
2019-07-23 9:34 ` Richard Biener
2019-07-23 23:05 ` Andrew MacLeod
2019-07-24 13:28 ` Richard Biener
2019-07-24 18:07 ` Andrew MacLeod
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d4cff840-d8f4-8468-d57a-c0e85f98655c@redhat.com \
--to=law@redhat.com \
--cc=aldyh@redhat.com \
--cc=amacleod@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=richard.guenther@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).