[Mesa-dev] [PATCH 16/26] python: Explicitly use lists

[Mathieu Bridon]

On Thu, 2018-07-05 at 09:31 -0700, Dylan Baker wrote:
> Quoting Mathieu Bridon (2018-07-05 06:17:47)
> > On Python 2, the builtin functions filter() and zip() would return
> > lists.
> > 
> > On Python 3, they return iterators.
> > 
> > Since we want to use those objects in contexts where we need lists,
> > we
> > need to explicitly turn them into lists.
> > 
> > This makes the code compatible with both Python 2 and Python 3.
> > 
> > Signed-off-by: Mathieu Bridon <bochecha at daitauha.fr>
> > ---
> >  src/compiler/nir/nir_opt_algebraic.py | 2 +-
> >  src/mesa/main/get_hash_generator.py   | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/compiler/nir/nir_opt_algebraic.py
> > b/src/compiler/nir/nir_opt_algebraic.py
> > index 5e07d662b0..7b2ba56990 100644
> > --- a/src/compiler/nir/nir_opt_algebraic.py
> > +++ b/src/compiler/nir/nir_opt_algebraic.py
> > @@ -633,7 +633,7 @@ optimizations = [
> >  
> >  invert = OrderedDict([('feq', 'fne'), ('fne', 'feq'), ('fge',
> > 'flt'), ('flt', 'fge')])
> >  
> > -for left, right in list(itertools.combinations(invert.keys(), 2))
> > + zip(invert.keys(), invert.keys()):
> > +for left, right in list(itertools.combinations(invert.keys(), 2))
> > + list(zip(invert.keys(), invert.keys())):
> 
> Isn't this just a really expenisve re-implementation of:
> itertools.combinations_with_replacement(invert.keys(), 2)

It seems to be, yes. (I was so focused on fixing the "can't concatenate
list with zip" error that I completely missed it)

However, since dict.keys() isn't guaranteed to always be in the same
order, it is theoretically possible that:

>>> zip(invert.keys(), invert.keys())

doesn't always return the same thing.

That is, the following could happen:

>>> d = {'A': …, 'B': …}
>>> zip(d.keys(), d.keys())
['AB', 'BA']

Which would make the whole line not equivalent to
`itertools.combinations_with_replacement()` any more.

In practice it probably doesn't happen though, but that means if the
intention behind the code was what you suspect, then by using
`itertools.combinations_with_replacement()` we'd be fixing an actual
bug, not just making the script compatible with Python 3. :)


-- 
Mathieu