Skip to content

Regression with __hash__ definition and rich comparison operators #45890

Closed
Closed
Regression with __hash__ definition and rich comparison operators#45890
Assignees
gvanrossum
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)type-crashA hard crash of the interpreter, possibly with a core dump

Description

@therve
therve
mannequin
opened on Dec 3, 2007
BPO 1549
Nosy @gvanrossum
Files
  • 1549_1.diff
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/gvanrossum'
closed_at = <Date 2007-12-19.22:51:37.125>
created_at = <Date 2007-12-03.13:51:31.627>
labels = ['interpreter-core', 'type-crash']
title = 'Regression with __hash__ definition and rich comparison operators'
updated_at = <Date 2007-12-20.09:31:22.706>
user = 'https://bugs.python.org/therve'

    bugs.python.org fields:

    activity = <Date 2007-12-20.09:31:22.706>
actor = 'therve'
assignee = 'gvanrossum'
closed = True
closed_date = <Date 2007-12-19.22:51:37.125>
closer = 'gvanrossum'
components = ['Interpreter Core']
creation = <Date 2007-12-03.13:51:31.627>
creator = 'therve'
dependencies = []
files = ['8871']
hgrepos = []
issue_num = 1549
keywords = ['patch']
message_count = 15.0
messages = ['58124', '58125', '58126', '58140', '58141', '58148', '58153', '58154', '58156', '58157', '58182', '58803', '58817', '58836', '58866']
nosy_count = 3.0
nosy_names = ['gvanrossum', 'exarkun', 'therve']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue1549'
versions = ['Python 2.6']

    Activity

    1. therve commented on Dec 3, 2007

      @therve
      thervemannequin
      on Dec 3, 2007
      MannequinAuthor

      A new behavior was introduced in r59106 in python trunk, which look
      suspicious to me. Basically, every time a class defines a comparison
      operator, it's __hash__ method is defined to None. The simple following
      example shows it:

      >>> class A(object):
...     def __init__(self, b):
...             self.b = b
...     def __cmp__(self, other):
...             if not isinstance(other, A):
...                     return -1
...             return cmp(self.b, other.b)
...
>>> hash(A(2))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'NoneType' object is not callable

      The problematic change is here:
      http://svn.python.org/view/python/trunk/Objects/typeobject.c?rev=59106&r1=58032&r2=59106

      And mainly the overrides_hash call in inherit_slots.

      FWIW, I've encounter the problem because zope.interface is now usable on
      trunk.

    2. therve
      added
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      type-crashA hard crash of the interpreter, possibly with a core dump
      on Dec 3, 2007

    3. therve commented on Dec 3, 2007

      @therve
      thervemannequin
      on Dec 3, 2007
      MannequinAuthor

      Of course, I meant unusable.

    4. therve commented on Dec 3, 2007

      @therve
      thervemannequin
      on Dec 3, 2007
      MannequinAuthor

      Also, to be more clear, that happens when you define any of the
      functions in the name_op array (lt, __le___, __gt__, __ge__, ...).
      It's not just a problem about the object being non hashable.

    5. tiran
      assigned
      gvanrossum
      on Dec 3, 2007

    6. gvanrossum commented on Dec 3, 2007

      @gvanrossum
      gvanrossum
      on Dec 3, 2007
      Member

      This was done intentionally -- if you define a comparison without
      defining a hash, the default hash will not match your comparison, and
      your objects will misbehave when used as dictionary keys.

      Can you give more details about the Zope issue? I agree that needs to
      be resolved before we release 2.6.

    7. exarkun commented on Dec 3, 2007

      @exarkun
      exarkunmannequin
      on Dec 3, 2007
      Mannequin

      zope.interface.interface.InterfaceClass implements __lt__ and __gt__.
      It doesn't override any of the other comparison methods, and it relies
      on inheriting an identity-based __hash__ from its parent.

      Disabling __hash__ when __cmp__ or __eq__ is defined makes sense, but it
      does not seem like it needs to be disabled if only __lt__ or __gt__ is
      defined.

    8. gvanrossum commented on Dec 3, 2007

      @gvanrossum
      gvanrossum
      on Dec 3, 2007
      Member

      You have a point. Can you suggest a patch that fixes this?

      Question though -- what semantics are used for __lt__ and __gt__ that
      they don't require overriding __eq__?

    9. exarkun commented on Dec 3, 2007

      @exarkun
      exarkunmannequin
      on Dec 3, 2007
      Mannequin

      It implements ordering based on the interface's fully-qualified Python
      name (ie, (module, __name__)). This is compatible with the default
      __eq__ implementation.

    10. gvanrossum commented on Dec 3, 2007

      @gvanrossum
      gvanrossum
      on Dec 3, 2007
      Member

      Fair enough.

      Still, this would go much faster if someone other than myself could be
      coerced into researching how to fix this (at least the Zope use -- I
      haven't heard back from the OP).

    11. therve commented on Dec 3, 2007

      @therve
      thervemannequin
      on Dec 3, 2007
      MannequinAuthor

      There are different ways to fix this. Reverting r59016 is one of them,
      but I guess it's not an acceptable solution :).

      From what I understand of the changes, removing the operations (lt,
      __gt__, __le__, __ge__) from the name_op array would fix this. But I
      have to understand why they have been put in there in the first place.

      I'll try tomorrow, you assign this to me if you want.

    12. gvanrossum commented on Dec 3, 2007

      @gvanrossum
      gvanrossum
      on Dec 3, 2007
      Member

      Well, the name_op array is used for other purposes. Perhaps the main
      issue is that the presence of any of these causes the "rich comparison"
      functionality to be overridden. But please do look into this if you can!

    13. therve commented on Dec 4, 2007

      @therve
      thervemannequin
      on Dec 4, 2007
      MannequinAuthor

      I gave it a try. The following patch corrects 2 problems:

      • classes with only __lt__ and __gt__ defined are hashable
      • classes defining __eq__ get a meaningful exception.

      I've restricted the hash_name_op to the bare minimum, but no tests
      failed, so I supposed it was enough :).

      Please review.

    14. therve commented on Dec 19, 2007

      @therve
      thervemannequin
      on Dec 19, 2007
      MannequinAuthor

      This is probably not the best moment (holidays etc), but please take
      this issue seriously. This problem completely block the community
      builders at pybots.org, and make any tests on current python trunk
      impossible. This also means that if in the meantime another regression
      appear, it will be even more difficult to solve it afterwards.

      Also, zope is not the only one affected, storm seems to suffer from the
      problem too (not exacly the same, though). As twisted is also unusable
      without zope.interface, that makes several project that can't follow
      python developpement at all (zope.interface error happens at import
      time, for the record).

      If this problem isn't solved in a reasonable time, it really questions
      the future of the community builders and their purpose.

      Thanks.

    15. gvanrossum commented on Dec 19, 2007

      @gvanrossum
      gvanrossum
      on Dec 19, 2007
      Member

      To be honest, I have no idea what pybots is, and how it's affected.
      Maybe you can get someone else interested in reviewing the code? I'm
      focusing mostly on Py3k.

    16. gvanrossum commented on Dec 19, 2007

      @gvanrossum
      gvanrossum
      on Dec 19, 2007
      Member

      Committed revision 59576.
      I found some time.
      Happy pybotting!

    17. gvanrossum
      closed this as completedon Dec 19, 2007

    18. therve commented on Dec 20, 2007

      @therve
      thervemannequin
      on Dec 20, 2007
      MannequinAuthor

      Thanks a lot!

    19. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    interpreter-core(Objects, Python, Grammar, and Parser dirs)type-crashA hard crash of the interpreter, possibly with a core dump

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions