Re: SSA Iterators

[Richard Biener <rguenther at suse dot de>]

On January 30, 2020 5:05:09 PM GMT+01:00, Martin Sebor <msebor@gmail.com> wrote:
>On 1/30/20 2:59 AM, Jonathan Wakely wrote:
>> On Thu, 30 Jan 2020, 05:44 Nicholas Krause wrote:
>>>
>>> Greetings,
>>>
>>> I was looking into starting to cleaning up the SSA trees for various
>>> reasons and iterators
>>> seem to be the easiest to do. I searched the list to see if someone
>>> mentioned it before
>>> and I ran across this:
>>> https://gcc.gnu.org/ml/gcc-patches/2019-10/msg02031.html
>>>
>>> If your trying to get a second ++ working then why not override it
>>> prefix as your
>>> just doing it for postfix and those can be separate.
>> 
>> No no no.
>> 
>> No.
>> 
>>> Its not ideal to
>>> have two
>>> different versions for prefix and postfix in terms of overloading
>but it
>>> may be
>>> the simplest solution here.
>> 
>> No, making pre-increment and post-incrememt so different things (i.e.
>> iterate over different ranges) is a horrible idea. That's the kind of
>> thing that gives operator overloading a bad name. Overloaded
>operators
>> should do the thing you expect them to. They should not be used to
>> hide a non-obvious action behind an apparently simple syntax.
>> 
>> 
>> I would suggest avoiding "smart" iterators that contain all the state
>> and know their own end point. Instead create a type that holds all
>the
>> state and has begin/end member functions which return an iterator
>that
>> refers to the state. And have the iterator  dereference to some other
>> object that has the state for the second level of iteration, with its
>> own begin/end members returning the second iterator type. That would
>> end up looking like:
>> 
>>    imm_use_stmt_range r (SSAVAR);
>>    for (imm_use_stmt_iter it = r.begin (); it != r.end (); ++it)
>>      for (imm_use_on_stmt_iter it2 = it->begin (); it2 != it->end ();
>++it2)
>>        ;
>> 
>> At some point when we move to C++11 or later that could become:
>> 
>>    imm_use_stmt_range r (SSAVAR);
>>    for (auto& stmt : r)
>>      for (auto& on_stmt : *stmt)
>>        ;
>
>That's a good goal to aim for with all GCC "iterators," including
>those behind the legacy FOREACH_XXX macros.  I posted a patch for
>one of these in 2018 but it was too late in the development cycle
>and I didn't get back to in in 2019:
>   https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01549.html
>
>If you're working on a general cleanup in this are please consider
>these as well.

We should definitely aim for a standard way, adding yet another one doesn't help. Which is why I was trying to tackle the more difficult one(s) first. Most of the iterators should be trivial to convert to whatever style we settle on. 

Richard. 

>Martin