Re: [PATCH 07/18] loop-iv.c: make cond_list a vec

[Trevor Saunders <tbsaunde at tbsaunde dot org>]

On Mon, Apr 25, 2016 at 04:25:27PM +0200, Bernd Schmidt wrote:
> On 04/25/2016 04:21 PM, Bernd Schmidt wrote:
> >On 04/25/2016 03:30 PM, Trevor Saunders wrote:
> >>On Mon, Apr 25, 2016 at 02:28:51PM +0200, Bernd Schmidt wrote:
> >>>On 04/20/2016 08:22 AM, tbsaunde+gcc@tbsaunde.org wrote:
> >>>>From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> >>>
> >>>>+          unsigned int len = cond_list.length ();
> >>>>+          for (unsigned int i = len - 1; i < len; i--)
> >>>
> >>>This is a really icky way to write a loop, the i < len condition
> >>>makes it
> >>>look like a forward one. We have FOR_EACH_VEC_ELT{,_REVERSE}, any
> >>>reason not
> >>>to use these?
> >>
> >>I'll agree that depending on unsigned wrapping is a tad wierd, but
> >>personally I think FOR_EACH_VEC_* are pretty icky, and just forget to
> >>think about them before writing a loop.
> >
> >They're standard inside gcc though, and readability-wise much
> >preferrable to the above IMO.
> >
> >I noticed this pattern in a lot of these patches; at this point I think
> >the best thing to do would be for you to go through all of them, address
> >review comments across the whole set, and then start a new thread with
> >v2 patches of all of them so we can retire this thread.
> >
> Oh, and also - I would prefer for each nontrivial such loop some sort of
> analysis about what order elements were inserted/processed in before this
> change, and what order afterwards. This could be a source of subtle errors
> with this patch series. When removing elements in a loop, we also need to
> pay attention to whether that's still safe.

I didn't write it down, but I did think about then when writing these
patches.  Most if not all places push stuff onto the front of a list and
then iterate the list from front to back.  Which is the same order as
pushing things on the back of a vector and then iterating the vector
back to front.

Trev

> 
> 
> Bernd
>