[Python-checkins] bpo-32929: Dataclasses: Change the tri-state hash parameter to the boolean unsafe_hash. (GH-5891) (GH-5902)

Eric V. Smith webhook-mailer at python.org
Mon Feb 26 04:43:45 EST 2018


https://github.com/python/cpython/commit/4cffe2f66b581fa7538f6de884d54a5c7364d8e0
commit: 4cffe2f66b581fa7538f6de884d54a5c7364d8e0
branch: 3.7
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: Eric V. Smith <ericvsmith at users.noreply.github.com>
date: 2018-02-26T04:43:35-05:00
summary:

bpo-32929: Dataclasses: Change the tri-state hash parameter to the boolean unsafe_hash. (GH-5891) (GH-5902)

unsafe_hash=False is now the default. It is the same behavior as the old hash=None parameter. unsafe_hash=True will try to add __hash__. If it already exists, TypeError is raised.
(cherry picked from commit dbf9cff48a4ad0fd58e1c623ce1f36c3dd3d5f38)

Co-authored-by: Eric V. Smith <ericvsmith at users.noreply.github.com>

files:
A Misc/NEWS.d/next/Library/2018-02-25-13-47-48.bpo-32929.X2gTDH.rst
M Lib/dataclasses.py
M Lib/test/test_dataclasses.py

diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py
index fb279cd30517..db92b10960d5 100644
--- a/Lib/dataclasses.py
+++ b/Lib/dataclasses.py
@@ -20,11 +20,11 @@
 
 # Conditions for adding methods.  The boxes indicate what action the
 #  dataclass decorator takes.  For all of these tables, when I talk
-#  about init=, repr=, eq=, order=, hash=, or frozen=, I'm referring
-#  to the arguments to the @dataclass decorator.  When checking if a
-#  dunder method already exists, I mean check for an entry in the
-#  class's __dict__.  I never check to see if an attribute is defined
-#  in a base class.
+#  about init=, repr=, eq=, order=, unsafe_hash=, or frozen=, I'm
+#  referring to the arguments to the @dataclass decorator.  When
+#  checking if a dunder method already exists, I mean check for an
+#  entry in the class's __dict__.  I never check to see if an
+#  attribute is defined in a base class.
 
 # Key:
 # +=========+=========================================+
@@ -34,11 +34,6 @@
 # +---------+-----------------------------------------+
 # | add     | Generated method is added.              |
 # +---------+-----------------------------------------+
-# | add*    | Generated method is added only if the   |
-# |         |  existing attribute is None and if the  |
-# |         |  user supplied a __eq__ method in the   |
-# |         |  class definition.                      |
-# +---------+-----------------------------------------+
 # | raise   | TypeError is raised.                    |
 # +---------+-----------------------------------------+
 # | None    | Attribute is set to None.               |
@@ -115,43 +110,36 @@
 
 # __hash__
 
-#      +------------------- hash= parameter
-#      |       +----------- eq= parameter
-#      |       |       +--- frozen= parameter
-#      |       |       |
-#      v       v       v    |        |        |
-#                           |   no   |  yes   |  <--- class has __hash__ in __dict__?
-# +=========+=======+=======+========+========+
-# | 1 None  | False | False |        |        | No __eq__, use the base class __hash__
-# +---------+-------+-------+--------+--------+
-# | 2 None  | False | True  |        |        | No __eq__, use the base class __hash__
-# +---------+-------+-------+--------+--------+
-# | 3 None  | True  | False | None   |        | <-- the default, not hashable
-# +---------+-------+-------+--------+--------+
-# | 4 None  | True  | True  | add    | add*   | Frozen, so hashable
-# +---------+-------+-------+--------+--------+
-# | 5 False | False | False |        |        |
-# +---------+-------+-------+--------+--------+
-# | 6 False | False | True  |        |        |
-# +---------+-------+-------+--------+--------+
-# | 7 False | True  | False |        |        |
-# +---------+-------+-------+--------+--------+
-# | 8 False | True  | True  |        |        |
-# +---------+-------+-------+--------+--------+
-# | 9 True  | False | False | add    | add*   | Has no __eq__, but hashable
-# +---------+-------+-------+--------+--------+
-# |10 True  | False | True  | add    | add*   | Has no __eq__, but hashable
-# +---------+-------+-------+--------+--------+
-# |11 True  | True  | False | add    | add*   | Not frozen, but hashable
-# +---------+-------+-------+--------+--------+
-# |12 True  | True  | True  | add    | add*   | Frozen, so hashable
-# +=========+=======+=======+========+========+
+#    +------------------- unsafe_hash= parameter
+#    |       +----------- eq= parameter
+#    |       |       +--- frozen= parameter
+#    |       |       |
+#    v       v       v    |        |        |
+#                         |   no   |  yes   |  <--- class has explicitly defined __hash__
+# +=======+=======+=======+========+========+
+# | False | False | False |        |        | No __eq__, use the base class __hash__
+# +-------+-------+-------+--------+--------+
+# | False | False | True  |        |        | No __eq__, use the base class __hash__
+# +-------+-------+-------+--------+--------+
+# | False | True  | False | None   |        | <-- the default, not hashable
+# +-------+-------+-------+--------+--------+
+# | False | True  | True  | add    |        | Frozen, so hashable, allows override
+# +-------+-------+-------+--------+--------+
+# | True  | False | False | add    | raise  | Has no __eq__, but hashable
+# +-------+-------+-------+--------+--------+
+# | True  | False | True  | add    | raise  | Has no __eq__, but hashable
+# +-------+-------+-------+--------+--------+
+# | True  | True  | False | add    | raise  | Not frozen, but hashable
+# +-------+-------+-------+--------+--------+
+# | True  | True  | True  | add    | raise  | Frozen, so hashable
+# +=======+=======+=======+========+========+
 # For boxes that are blank, __hash__ is untouched and therefore
 #  inherited from the base class.  If the base is object, then
 #  id-based hashing is used.
 # Note that a class may have already __hash__=None if it specified an
 #  __eq__ method in the class body (not one that was created by
 #  @dataclass).
+# See _hash_action (below) for a coded version of this table.
 
 
 # Raised when an attempt is made to modify a frozen class.
@@ -557,7 +545,45 @@ def _set_new_attribute(cls, name, value):
     return False
 
 
-def _process_class(cls, repr, eq, order, hash, init, frozen):
+# Decide if/how we're going to create a hash function.  Key is
+#  (unsafe_hash, eq, frozen, does-hash-exist).  Value is the action to
+#  take.
+# Actions:
+#  '':          Do nothing.
+#  'none':      Set __hash__ to None.
+#  'add':       Always add a generated __hash__function.
+#  'exception': Raise an exception.
+#
+#                +-------------------------------------- unsafe_hash?
+#                |      +------------------------------- eq?
+#                |      |      +------------------------ frozen?
+#                |      |      |      +----------------  has-explicit-hash?
+#                |      |      |      |
+#                |      |      |      |        +-------  action
+#                |      |      |      |        |
+#                v      v      v      v        v
+_hash_action = {(False, False, False, False): (''),
+                (False, False, False, True ): (''),
+                (False, False, True,  False): (''),
+                (False, False, True,  True ): (''),
+                (False, True,  False, False): ('none'),
+                (False, True,  False, True ): (''),
+                (False, True,  True,  False): ('add'),
+                (False, True,  True,  True ): (''),
+                (True,  False, False, False): ('add'),
+                (True,  False, False, True ): ('exception'),
+                (True,  False, True,  False): ('add'),
+                (True,  False, True,  True ): ('exception'),
+                (True,  True,  False, False): ('add'),
+                (True,  True,  False, True ): ('exception'),
+                (True,  True,  True,  False): ('add'),
+                (True,  True,  True,  True ): ('exception'),
+                }
+# See https://bugs.python.org/issue32929#msg312829 for an if-statement
+#  version of this table.
+
+
+def _process_class(cls, repr, eq, order, unsafe_hash, init, frozen):
     # Now that dicts retain insertion order, there's no reason to use
     #  an ordered dict.  I am leveraging that ordering here, because
     #  derived class fields overwrite base class fields, but the order
@@ -605,8 +631,14 @@ def _process_class(cls, repr, eq, order, hash, init, frozen):
     #  be inherited down.
     is_frozen = frozen or cls.__setattr__ is _frozen_setattr
 
-    # Was this class defined with an __eq__?  Used in __hash__ logic.
-    auto_hash_test= '__eq__' in cls.__dict__ and getattr(cls.__dict__, '__hash__', MISSING) is None
+    # Was this class defined with an explicit __hash__?  Note that if
+    #  __eq__ is defined in this class, then python will automatically
+    #  set __hash__ to None.  This is a heuristic, as it's possible
+    #  that such a __hash__ == None was not auto-generated, but it
+    #  close enough.
+    class_hash = cls.__dict__.get('__hash__', MISSING)
+    has_explicit_hash = not (class_hash is MISSING or
+                             (class_hash is None and '__eq__' in cls.__dict__))
 
     # If we're generating ordering methods, we must be generating
     #  the eq methods.
@@ -661,7 +693,7 @@ def _process_class(cls, repr, eq, order, hash, init, frozen):
             if _set_new_attribute(cls, name,
                                   _cmp_fn(name, op, self_tuple, other_tuple)):
                 raise TypeError(f'Cannot overwrite attribute {name} '
-                                f'in {cls.__name__}. Consider using '
+                                f'in class {cls.__name__}. Consider using '
                                 'functools.total_ordering')
 
     if is_frozen:
@@ -669,40 +701,30 @@ def _process_class(cls, repr, eq, order, hash, init, frozen):
                          ('__delattr__', _frozen_delattr)]:
             if _set_new_attribute(cls, name, fn):
                 raise TypeError(f'Cannot overwrite attribute {name} '
-                                f'in {cls.__name__}')
+                                f'in class {cls.__name__}')
 
     # Decide if/how we're going to create a hash function.
-    # TODO: Move this table to module scope, so it's not recreated
-    #  all the time.
-    generate_hash = {(None,  False, False): ('',     ''),
-                     (None,  False, True):  ('',     ''),
-                     (None,  True,  False): ('none', ''),
-                     (None,  True,  True):  ('fn',   'fn-x'),
-                     (False, False, False): ('',     ''),
-                     (False, False, True):  ('',     ''),
-                     (False, True,  False): ('',     ''),
-                     (False, True,  True):  ('',     ''),
-                     (True,  False, False): ('fn',   'fn-x'),
-                     (True,  False, True):  ('fn',   'fn-x'),
-                     (True,  True,  False): ('fn',   'fn-x'),
-                     (True,  True,  True):  ('fn',   'fn-x'),
-                     }[None if hash is None else bool(hash),   # Force bool() if not None.
-                       bool(eq),
-                       bool(frozen)]['__hash__' in cls.__dict__]
+    hash_action = _hash_action[bool(unsafe_hash),
+                               bool(eq),
+                               bool(frozen),
+                               has_explicit_hash]
+
     # No need to call _set_new_attribute here, since we already know if
     #  we're overwriting a __hash__ or not.
-    if generate_hash == '':
+    if hash_action == '':
         # Do nothing.
         pass
-    elif generate_hash == 'none':
+    elif hash_action == 'none':
         cls.__hash__ = None
-    elif generate_hash in ('fn', 'fn-x'):
-        if generate_hash == 'fn' or auto_hash_test:
-            flds = [f for f in field_list
-                    if (f.compare if f.hash is None else f.hash)]
-            cls.__hash__ = _hash_fn(flds)
+    elif hash_action == 'add':
+        flds = [f for f in field_list if (f.compare if f.hash is None else f.hash)]
+        cls.__hash__ = _hash_fn(flds)
+    elif hash_action == 'exception':
+        # Raise an exception.
+        raise TypeError(f'Cannot overwrite attribute __hash__ '
+                        f'in class {cls.__name__}')
     else:
-        assert False, f"can't get here: {generate_hash}"
+        assert False, f"can't get here: {hash_action}"
 
     if not getattr(cls, '__doc__'):
         # Create a class doc-string.
@@ -716,7 +738,7 @@ def _process_class(cls, repr, eq, order, hash, init, frozen):
 #  underscore. The presence of _cls is used to detect if this
 #  decorator is being called with parameters or not.
 def dataclass(_cls=None, *, init=True, repr=True, eq=True, order=False,
-              hash=None, frozen=False):
+              unsafe_hash=None, frozen=False):
     """Returns the same class as was passed in, with dunder methods
     added based on the fields defined in the class.
 
@@ -724,13 +746,13 @@ def dataclass(_cls=None, *, init=True, repr=True, eq=True, order=False,
 
     If init is true, an __init__() method is added to the class. If
     repr is true, a __repr__() method is added. If order is true, rich
-    comparison dunder methods are added. If hash is true, a __hash__()
-    method function is added. If frozen is true, fields may not be
-    assigned to after instance creation.
+    comparison dunder methods are added. If unsafe_hash is true, a
+    __hash__() method function is added. If frozen is true, fields may
+    not be assigned to after instance creation.
     """
 
     def wrap(cls):
-        return _process_class(cls, repr, eq, order, hash, init, frozen)
+        return _process_class(cls, repr, eq, order, unsafe_hash, init, frozen)
 
     # See if we're being called as @dataclass or @dataclass().
     if _cls is None:
@@ -793,6 +815,7 @@ class C:
         raise TypeError("asdict() should be called on dataclass instances")
     return _asdict_inner(obj, dict_factory)
 
+
 def _asdict_inner(obj, dict_factory):
     if _is_dataclass_instance(obj):
         result = []
@@ -832,6 +855,7 @@ class C:
         raise TypeError("astuple() should be called on dataclass instances")
     return _astuple_inner(obj, tuple_factory)
 
+
 def _astuple_inner(obj, tuple_factory):
     if _is_dataclass_instance(obj):
         result = []
@@ -849,7 +873,8 @@ def _astuple_inner(obj, tuple_factory):
 
 
 def make_dataclass(cls_name, fields, *, bases=(), namespace=None, init=True,
-                   repr=True, eq=True, order=False, hash=None, frozen=False):
+                   repr=True, eq=True, order=False, unsafe_hash=None,
+                   frozen=False):
     """Return a new dynamically created dataclass.
 
     The dataclass name will be 'cls_name'.  'fields' is an iterable
@@ -869,7 +894,7 @@ class C(Base):
 
     For the bases and namespace parameters, see the builtin type() function.
 
-    The parameters init, repr, eq, order, hash, and frozen are passed to
+    The parameters init, repr, eq, order, unsafe_hash, and frozen are passed to
     dataclass().
     """
 
@@ -894,7 +919,8 @@ class C(Base):
     namespace['__annotations__'] = anns
     cls = type(cls_name, bases, namespace)
     return dataclass(cls, init=init, repr=repr, eq=eq, order=order,
-                     hash=hash, frozen=frozen)
+                     unsafe_hash=unsafe_hash, frozen=frozen)
+
 
 def replace(obj, **changes):
     """Return a new object replacing specified fields with new values.
diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py
index c24af7fc79c4..582cb3459f5d 100755
--- a/Lib/test/test_dataclasses.py
+++ b/Lib/test/test_dataclasses.py
@@ -83,32 +83,59 @@ class B:
             class C(B):
                 x: int = 0
 
-    def test_overwriting_hash(self):
+    def test_overwrite_hash(self):
+        # Test that declaring this class isn't an error.  It should
+        #  use the user-provided __hash__.
         @dataclass(frozen=True)
         class C:
             x: int
             def __hash__(self):
-                pass
+                return 301
+        self.assertEqual(hash(C(100)), 301)
 
-        @dataclass(frozen=True,hash=False)
+        # Test that declaring this class isn't an error.  It should
+        #  use the generated __hash__.
+        @dataclass(frozen=True)
         class C:
             x: int
-            def __hash__(self):
-                return 600
-        self.assertEqual(hash(C(0)), 600)
+            def __eq__(self, other):
+                return False
+        self.assertEqual(hash(C(100)), hash((100,)))
 
-        @dataclass(frozen=True)
+        # But this one should generate an exception, because with
+        #  unsafe_hash=True, it's an error to have a __hash__ defined.
+        with self.assertRaisesRegex(TypeError,
+                                    'Cannot overwrite attribute __hash__'):
+            @dataclass(unsafe_hash=True)
+            class C:
+                def __hash__(self):
+                    pass
+
+        # Creating this class should not generate an exception,
+        #  because even though __hash__ exists before @dataclass is
+        #  called, (due to __eq__ being defined), since it's None
+        #  that's okay.
+        @dataclass(unsafe_hash=True)
         class C:
             x: int
-            def __hash__(self):
+            def __eq__(self):
                 pass
+        # The generated hash function works as we'd expect.
+        self.assertEqual(hash(C(10)), hash((10,)))
+
+        # Creating this class should generate an exception, because
+        # __hash__ exists and is not None, which it would be if it had
+        # been auto-generated do due __eq__ being defined.
+        with self.assertRaisesRegex(TypeError,
+                                    'Cannot overwrite attribute __hash__'):
+            @dataclass(unsafe_hash=True)
+            class C:
+                x: int
+                def __eq__(self):
+                    pass
+                def __hash__(self):
+                    pass
 
-        @dataclass(frozen=True, hash=False)
-        class C:
-            x: int
-            def __hash__(self):
-                return 600
-        self.assertEqual(hash(C(0)), 600)
 
     def test_overwrite_fields_in_derived_class(self):
         # Note that x from C1 replaces x in Base, but the order remains
@@ -294,19 +321,6 @@ class C(B):
                                             "not supported between instances of 'B' and 'C'"):
                     fn(B(0), C(0))
 
-    def test_0_field_hash(self):
-        @dataclass(hash=True)
-        class C:
-            pass
-        self.assertEqual(hash(C()), hash(()))
-
-    def test_1_field_hash(self):
-        @dataclass(hash=True)
-        class C:
-            x: int
-        self.assertEqual(hash(C(4)), hash((4,)))
-        self.assertEqual(hash(C(42)), hash((42,)))
-
     def test_eq_order(self):
         # Test combining eq and order.
         for (eq,    order, result   ) in [
@@ -407,25 +421,25 @@ def test_hash_field_rules(self):
         # Test all 6 cases of:
         #  hash=True/False/None
         #  compare=True/False
-        for (hash_val, compare, result  ) in [
+        for (hash_,    compare, result  ) in [
             (True,     False,   'field' ),
             (True,     True,    'field' ),
             (False,    False,   'absent'),
             (False,    True,    'absent'),
             (None,     False,   'absent'),
             (None,     True,    'field' ),
-        ]:
-            with self.subTest(hash_val=hash_val, compare=compare):
-                @dataclass(hash=True)
+            ]:
+            with self.subTest(hash=hash_, compare=compare):
+                @dataclass(unsafe_hash=True)
                 class C:
-                    x: int = field(compare=compare, hash=hash_val, default=5)
+                    x: int = field(compare=compare, hash=hash_, default=5)
 
                 if result == 'field':
                     # __hash__ contains the field.
-                    self.assertEqual(C(5).__hash__(), hash((5,)))
+                    self.assertEqual(hash(C(5)), hash((5,)))
                 elif result == 'absent':
                     # The field is not present in the hash.
-                    self.assertEqual(C(5).__hash__(), hash(()))
+                    self.assertEqual(hash(C(5)), hash(()))
                 else:
                     assert False, f'unknown result {result!r}'
 
@@ -737,7 +751,7 @@ class C:
         validate_class(C)
 
         # Now repeat with __hash__.
-        @dataclass(frozen=True, hash=True)
+        @dataclass(frozen=True, unsafe_hash=True)
         class C:
             i: int
             j: str
@@ -1107,7 +1121,7 @@ class C:
         self.assertEqual(C().x, [])
 
         # hash
-        @dataclass(hash=True)
+        @dataclass(unsafe_hash=True)
         class C:
             x: list = field(default_factory=list, hash=False)
         self.assertEqual(astuple(C()), ([],))
@@ -2242,28 +2256,13 @@ def __ge__(self):
                     pass
 
 class TestHash(unittest.TestCase):
-    def test_hash(self):
-        @dataclass(hash=True)
+    def test_unsafe_hash(self):
+        @dataclass(unsafe_hash=True)
         class C:
             x: int
             y: str
         self.assertEqual(hash(C(1, 'foo')), hash((1, 'foo')))
 
-    def test_hash_false(self):
-        @dataclass(hash=False)
-        class C:
-            x: int
-            y: str
-        self.assertNotEqual(hash(C(1, 'foo')), hash((1, 'foo')))
-
-    def test_hash_none(self):
-        @dataclass(hash=None)
-        class C:
-            x: int
-        with self.assertRaisesRegex(TypeError,
-                                    "unhashable type: 'C'"):
-            hash(C(1))
-
     def test_hash_rules(self):
         def non_bool(value):
             # Map to something else that's True, but not a bool.
@@ -2273,89 +2272,73 @@ def non_bool(value):
                 return (3,)
             return 0
 
-        def test(case, hash, eq, frozen, with_hash, result):
-            with self.subTest(case=case, hash=hash, eq=eq, frozen=frozen):
-                if with_hash:
-                    @dataclass(hash=hash, eq=eq, frozen=frozen)
-                    class C:
-                        def __hash__(self):
-                            return 0
-                else:
-                    @dataclass(hash=hash, eq=eq, frozen=frozen)
-                    class C:
-                        pass
+        def test(case, unsafe_hash, eq, frozen, with_hash, result):
+            with self.subTest(case=case, unsafe_hash=unsafe_hash, eq=eq,
+                              frozen=frozen):
+                if result != 'exception':
+                    if with_hash:
+                        @dataclass(unsafe_hash=unsafe_hash, eq=eq, frozen=frozen)
+                        class C:
+                            def __hash__(self):
+                                return 0
+                    else:
+                        @dataclass(unsafe_hash=unsafe_hash, eq=eq, frozen=frozen)
+                        class C:
+                            pass
 
                 # See if the result matches what's expected.
-                if result in ('fn', 'fn-x'):
+                if result == 'fn':
                     # __hash__ contains the function we generated.
                     self.assertIn('__hash__', C.__dict__)
                     self.assertIsNotNone(C.__dict__['__hash__'])
 
-                    if result == 'fn-x':
-                        # This is the "auto-hash test" case.  We
-                        #  should overwrite __hash__ iff there's an
-                        #  __eq__ and if __hash__=None.
-
-                        # There are two ways of getting __hash__=None:
-                        #  explicitely, and by defining __eq__.  If
-                        #  __eq__ is defined, python will add __hash__
-                        #  when the class is created.
-                        @dataclass(hash=hash, eq=eq, frozen=frozen)
-                        class C:
-                            def __eq__(self, other): pass
-                            __hash__ = None
-
-                        # Hash should be overwritten (non-None).
-                        self.assertIsNotNone(C.__dict__['__hash__'])
-
-                        # Same test as above, but we don't provide
-                        #  __hash__, it will implicitely set to None.
-                        @dataclass(hash=hash, eq=eq, frozen=frozen)
-                        class C:
-                            def __eq__(self, other): pass
-
-                        # Hash should be overwritten (non-None).
-                        self.assertIsNotNone(C.__dict__['__hash__'])
-
                 elif result == '':
                     # __hash__ is not present in our class.
                     if not with_hash:
                         self.assertNotIn('__hash__', C.__dict__)
+
                 elif result == 'none':
                     # __hash__ is set to None.
                     self.assertIn('__hash__', C.__dict__)
                     self.assertIsNone(C.__dict__['__hash__'])
+
+                elif result == 'exception':
+                    # Creating the class should cause an exception.
+                    #  This only happens with with_hash==True.
+                    assert(with_hash)
+                    with self.assertRaisesRegex(TypeError, 'Cannot overwrite attribute __hash__'):
+                        @dataclass(unsafe_hash=unsafe_hash, eq=eq, frozen=frozen)
+                        class C:
+                            def __hash__(self):
+                                return 0
+
                 else:
                     assert False, f'unknown result {result!r}'
 
-        # There are 12 cases of:
-        #  hash=True/False/None
+        # There are 8 cases of:
+        #  unsafe_hash=True/False
         #  eq=True/False
         #  frozen=True/False
         # And for each of these, a different result if
         #  __hash__ is defined or not.
-        for case, (hash,  eq,    frozen, result_no, result_yes) in enumerate([
-                  (None,  False, False,  '',        ''),
-                  (None,  False, True,   '',        ''),
-                  (None,  True,  False,  'none',    ''),
-                  (None,  True,  True,   'fn',      'fn-x'),
-                  (False, False, False,  '',        ''),
-                  (False, False, True,   '',        ''),
-                  (False, True,  False,  '',        ''),
-                  (False, True,  True,   '',        ''),
-                  (True,  False, False,  'fn',      'fn-x'),
-                  (True,  False, True,   'fn',      'fn-x'),
-                  (True,  True,  False,  'fn',      'fn-x'),
-                  (True,  True,  True,   'fn',      'fn-x'),
-        ], 1):
-            test(case, hash, eq, frozen, False, result_no)
-            test(case, hash, eq, frozen, True,  result_yes)
+        for case, (unsafe_hash,  eq,    frozen, res_no_defined_hash, res_defined_hash) in enumerate([
+                  (False,        False, False,  '',                  ''),
+                  (False,        False, True,   '',                  ''),
+                  (False,        True,  False,  'none',              ''),
+                  (False,        True,  True,   'fn',                ''),
+                  (True,         False, False,  'fn',                'exception'),
+                  (True,         False, True,   'fn',                'exception'),
+                  (True,         True,  False,  'fn',                'exception'),
+                  (True,         True,  True,   'fn',                'exception'),
+                  ], 1):
+            test(case, unsafe_hash, eq, frozen, False, res_no_defined_hash)
+            test(case, unsafe_hash, eq, frozen, True,  res_defined_hash)
 
             # Test non-bool truth values, too.  This is just to
             #  make sure the data-driven table in the decorator
             #  handles non-bool values.
-            test(case, non_bool(hash), non_bool(eq), non_bool(frozen), False, result_no)
-            test(case, non_bool(hash), non_bool(eq), non_bool(frozen), True,  result_yes)
+            test(case, non_bool(unsafe_hash), non_bool(eq), non_bool(frozen), False, res_no_defined_hash)
+            test(case, non_bool(unsafe_hash), non_bool(eq), non_bool(frozen), True,  res_defined_hash)
 
 
     def test_eq_only(self):
@@ -2373,8 +2356,8 @@ def __eq__(self, other):
         self.assertNotEqual(C(1), C(4))
 
         # And make sure things work in this case if we specify
-        #  hash=True.
-        @dataclass(hash=True)
+        #  unsafe_hash=True.
+        @dataclass(unsafe_hash=True)
         class C:
             i: int
             def __eq__(self, other):
@@ -2384,7 +2367,7 @@ def __eq__(self, other):
 
         # And check that the classes __eq__ is being used, despite
         #  specifying eq=True.
-        @dataclass(hash=True, eq=True)
+        @dataclass(unsafe_hash=True, eq=True)
         class C:
             i: int
             def __eq__(self, other):
@@ -2393,10 +2376,35 @@ def __eq__(self, other):
         self.assertNotEqual(C(1), C(1))
         self.assertEqual(hash(C(1)), hash(C(1.0)))
 
+    def test_0_field_hash(self):
+        @dataclass(frozen=True)
+        class C:
+            pass
+        self.assertEqual(hash(C()), hash(()))
+
+        @dataclass(unsafe_hash=True)
+        class C:
+            pass
+        self.assertEqual(hash(C()), hash(()))
+
+    def test_1_field_hash(self):
+        @dataclass(frozen=True)
+        class C:
+            x: int
+        self.assertEqual(hash(C(4)), hash((4,)))
+        self.assertEqual(hash(C(42)), hash((42,)))
+
+        @dataclass(unsafe_hash=True)
+        class C:
+            x: int
+        self.assertEqual(hash(C(4)), hash((4,)))
+        self.assertEqual(hash(C(42)), hash((42,)))
+
     def test_hash_no_args(self):
         # Test dataclasses with no hash= argument.  This exists to
-        # make sure that when hash is changed, the default hashability
-        # keeps working.
+        # make sure that if the @dataclass parameter name is changed
+        # or the non-default hashing behavior changes, the default
+        # hashability keeps working the same way.
 
         class Base:
             def __hash__(self):
diff --git a/Misc/NEWS.d/next/Library/2018-02-25-13-47-48.bpo-32929.X2gTDH.rst b/Misc/NEWS.d/next/Library/2018-02-25-13-47-48.bpo-32929.X2gTDH.rst
new file mode 100644
index 000000000000..b8a470cbf280
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-02-25-13-47-48.bpo-32929.X2gTDH.rst
@@ -0,0 +1,6 @@
+Remove the tri-state parameter "hash", and add the boolean "unsafe_hash". If
+unsafe_hash is True, add a __hash__ function, but if a __hash__ exists,
+raise TypeError.  If unsafe_hash is False, add a __hash__ based on the
+values of eq= and frozen=.  The unsafe_hash=False behavior is the same as
+the old hash=None behavior.  unsafe_hash=False is the default, just as
+hash=None used to be.



More information about the Python-checkins mailing list