[Mesa-dev] [PATCH mesa 2/2] mapi: add asserts to enforce ordering and uniqueness

[Ilia Mirkin]

On Sun, Sep 25, 2016 at 12:27 AM, Eric Engestrom <eric at engestrom.ch> wrote:
> On Sat, Sep 24, 2016 at 11:39:39PM -0400, Ilia Mirkin wrote:
>> Also note... the list is already sorted. You just picked a different sort
>> order. One that I'm pretty sure is subject to locale settings. As I recall
>> C and utf8 do things differently in a way relevant to this list but I
>> haven't checked.
>
> Oh OK, I didn't think python's sorting would be locale-dependent.
> How would I make it sort using the same method used for the current sort
> order used for this file?

Hrm, well, you could just check it for LC_ALL=C and LC_ALL=en_US.utf8
and hope that it "just works" for the rest. I did check a few corner
cases (like sorting of lower vs upper case, and short vs long strings)
and they do appear to be sorted the same way. This is not what I
remembered - I must have been thinking of some other odd case, perhaps
to do with spaces. Or perhaps Python string comparison does not take
locale into account - that'd be nice.

Yeah, that appears to be the case:

$ LC_ALL=C python -c 'print sorted(["aa", "aA"])'
['aA', 'aa']
$ LC_ALL=en_US.utf8 python -c 'print sorted(["aa", "aA"])'
['aA', 'aa']

But:
$ cat t
aa
aA
$ LC_ALL=en_US.utf8 sort t
aa
aA
$ LC_ALL=C sort t
aA
aa

OK, so the Python2 sort is not locale-dependent. And it does not
appear that they changed this in Python3. Nice!

And actually my comment about sorted returning a generator was wrong
too - it returns a list. That makes sense. Curious to hear whether
Dylan has any comments here.

>
> Note that patch #1 was done using vim's `:sort`, which I do know follows
> the locale sorting order (but I hadn't thought about it until just now).
>
> I guess the best thing is to drop patch #1 and drop the last assert from
> patch #2?  I'll do that tomorrow then.

Hold off on that.

>
> Cheers,
>   Eric
>
>>
>> On Sep 24, 2016 10:27 PM, "Ilia Mirkin" <imirkin at alum.mit.edu> wrote:
>>
>> > On Sat, Sep 24, 2016 at 10:17 PM, Eric Engestrom <eric at engestrom.ch>
>> > wrote:
>> > > Dylan Baker recently added functions to that list and had to try a
>> > couple times
>> > > to avoid duplicates. He said [1] he ended up testing it using:
>> > >   len(functions) == len(set(functions))
>> > > which I thought should always be done.
>> > >
>> > > Add this and a couple other tests using asserts to enforce the ordering
>> > and
>> > > uniqueness of the `functions` list, and the uniqueness and compactness
>> > of the
>> > > `offsets` dictionary.
>> > >
>> > > [1] https://lists.freedesktop.org/archives/mesa-dev/2016-
>> > September/129525.html
>> > >
>> > > Signed-off-by: Eric Engestrom <eric at engestrom.ch>
>> > > ---
>> > >
>> > > If people don't like enforcing the order, I'm happy to drop the previous
>> > patch
>> > > and send a revised version of this patch with this last assert removed.
>> > >
>> > > ---
>> > >  src/mapi/glapi/gen/static_data.py | 6 ++++++
>> > >  1 file changed, 6 insertions(+)
>> > >
>> > > diff --git a/src/mapi/glapi/gen/static_data.py
>> > b/src/mapi/glapi/gen/static_data.py
>> > > index bb11c1d..ef35b24 100644
>> > > --- a/src/mapi/glapi/gen/static_data.py
>> > > +++ b/src/mapi/glapi/gen/static_data.py
>> > > @@ -435,6 +435,9 @@ offsets = {
>> > >      "MultiTexCoord4sv": 407
>> > >  }
>> > >
>> > > +assert len(offsets) == len(set(offsets.keys())),  "The offsets
>> > dictionary contains duplicates"
>> >
>> > set(offsets) should be enough, I think.
>> >
>> > > +assert len(offsets) == max(offsets.values()) + 1, "The offsets
>> > dictionary has gaps"
>> >
>> > offsets.itervalues()
>> >
>> > > +
>> > >  functions = [
>> > >      "Accum",
>> > >      "ActiveShaderProgram",
>> > > @@ -1723,6 +1726,9 @@ functions = [
>> > >      "WindowPos3svARB",
>> > >  ]
>> > >
>> > > +assert len(functions) == len(set(functions)), "The functions list
>> > contains duplicates"
>> > > +assert functions == sorted(functions),        "The functions list is
>> > not sorted"
>> >
>> > I'm surprised this passes. Functions is an array, while sorted() is,
>> > iirc, an iterator (or maybe a generator). Does list.__eq__ have some
>> > sort of special cleverness to deal with that?
>> >