From 01d618c5606a239b03ad1269541eddb6e724775d Mon Sep 17 00:00:00 2001 From: "Eric V. Smith" Date: Sat, 24 Mar 2018 22:10:14 -0400 Subject: [PATCH] bpo-33134: dataclasses: use function dispatch table for hash, instead of a string lookup which then is tested with if tests. (GH-6222) * Change _hash_action to be a function table lookup, instead of a list of strings which is then tested with if statements. --- Lib/dataclasses.py | 76 +++++++++---------- .../2018-03-24-19-34-26.bpo-33134.hbVeIX.rst | 3 + 2 files changed, 39 insertions(+), 40 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-03-24-19-34-26.bpo-33134.hbVeIX.rst diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index 4425408b27..8ccc4c88ae 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -585,14 +585,24 @@ def _set_new_attribute(cls, name, value): return False + # 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. +# take. The common case is to do nothing, so instead of providing a +# function that is a no-op, use None to signify that. + +def _hash_set_none(cls, fields): + return None + +def _hash_add(cls, fields): + flds = [f for f in fields if (f.compare if f.hash is None else f.hash)] + return _hash_fn(flds) + +def _hash_exception(cls, fields): + # Raise an exception. + raise TypeError(f'Cannot overwrite attribute __hash__ ' + f'in class {cls.__name__}') + # # +-------------------------------------- unsafe_hash? # | +------------------------------- eq? @@ -602,22 +612,22 @@ def _set_new_attribute(cls, name, value): # | | | | +------- 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'), +_hash_action = {(False, False, False, False): None, + (False, False, False, True ): None, + (False, False, True, False): None, + (False, False, True, True ): None, + (False, True, False, False): _hash_set_none, + (False, True, False, True ): None, + (False, True, True, False): _hash_add, + (False, True, True, True ): None, + (True, False, False, False): _hash_add, + (True, False, False, True ): _hash_exception, + (True, False, True, False): _hash_add, + (True, False, True, True ): _hash_exception, + (True, True, False, False): _hash_add, + (True, True, False, True ): _hash_exception, + (True, True, True, False): _hash_add, + (True, True, True, True ): _hash_exception, } # See https://bugs.python.org/issue32929#msg312829 for an if-statement # version of this table. @@ -774,7 +784,6 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen): 'functools.total_ordering') if frozen: - # XXX: Which fields are frozen? InitVar? ClassVar? hashed-only? for fn in _frozen_get_del_attr(cls, field_list): if _set_new_attribute(cls, fn.__name__, fn): raise TypeError(f'Cannot overwrite attribute {fn.__name__} ' @@ -785,23 +794,10 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen): 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 hash_action == '': - # Do nothing. - pass - elif hash_action == 'none': - cls.__hash__ = None - 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: {hash_action}" + if hash_action: + # No need to call _set_new_attribute here, since by the time + # we're here the overwriting is unconditional. + cls.__hash__ = hash_action(cls, field_list) if not getattr(cls, '__doc__'): # Create a class doc-string. diff --git a/Misc/NEWS.d/next/Library/2018-03-24-19-34-26.bpo-33134.hbVeIX.rst b/Misc/NEWS.d/next/Library/2018-03-24-19-34-26.bpo-33134.hbVeIX.rst new file mode 100644 index 0000000000..3f4ce84bef --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-03-24-19-34-26.bpo-33134.hbVeIX.rst @@ -0,0 +1,3 @@ +When computing dataclass's __hash__, use the lookup table to contain the +function which returns the __hash__ value. This is an improvement over +looking up a string, and then testing that string to see what to do. -- 2.40.0