[Python-Dev] RE: test_sort.py failure

[Tim Peters]

[Armin Rigo]
> Unfortunately it is not true that ob_item can never be set to NULL with normal
> list operations.  list_ass_slice() and list_inplace_repeat() can both do that.
> The former is not so rare as it is used e.g. for 'del lst[:]'.

Thank you for digging into this!  I like all the changes you made.

> The invariants of the ob_xxx list members should be clearly specified
> somewhere

None were, but as part of my stab at fixing this I documented what I
believe to be minimally sane listobject invariants, as comments in the
declaration of the listobject struct.

> and we should stick to them.

You think <wink>?  I confess I made no effort to verify that the
invariants actually hold.  My view was that the invariants were so
mild and so necessary that any violation discovered should (and would)
be treated as a bug.

>  E.g. it is unclear to me why we allow list_ass_slice() to reset ob_item to NULL
> and ob_size to 0 without resetting ob_allocated to 0

Since that violates one of the now-documented invariants, you can
guess my position on that.

> -- I see why it doesn't crash in a subsequent list_resize(), but it looks messy.

There's no adequate reason to endure convolutions like this to save a
theoretical nanosecond in a rare case -- if indeed that ever was
someone's intent.  I expect it's more likely an oversight, and largely
due to the previous lack of documenting intended invariants.

> Alternatively, the quickest fix for the broken listsort() would have been to
> keep the empty_ob_item hack but just check that ob_allocated is still zero in
> addition to ob_size.

Too ugly; I don't think either of us *liked* the empty_ob_item hack,
and if I was going to piss away more time on this I was determined to
get rid of it <wink>.

> However the current version is quite clean and can be
> made bulletproof by ensuring that ob_item is never reset to NULL.

It was indeed cleaner, and I'm all in favor of your later change to
get rid of the ad hoc "ob_item != NULL implies ob_item will never be
NULL" invariant (which I agree was far more expedient than
reasonable).