bt2: add `__hash__()` method on hashable fields

Implement the `__hash__()` method of selected fields so that they can be
used as keys in dictionaries. This commit only implements the
`__hash__()` method of _NumericFieldConst and _StringFieldConst fields
after considering the following facts:
  1. "[...] the implementation of hashable collections requires that a key’s
    hash value is immutable (if the object’s hash value changes, it will be
    in the wrong hash bucket)." [1],
  2. Python built-in types bool, int, float, and string are hashable,
      and sequences and dictionaries are not.

So I suggest that only const fields should be hashable as they are
immutable, and that Container fields (variant, static array, etc.) should not
be hashable (even if they are const) to be consistent with Python
built-in types.

[1]: https://docs.python.org/3/reference/datamodel.html?highlight=__hash__#object.__hash__

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I3dc246802d628603a3f06ecd3a87b7e38b3f1e27
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2032
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>

diff --git a/src/bindings/python/bt2/bt2/field.py b/src/bindings/python/bt2/bt2/field.py
index 4f89d1e43a10ccedcf36c9eff846f9a1f6f09f0e..1e2ceac30efa0c6ab80cef0f5db7f84f6e6e2913 100644 (file)
--- a/src/bindings/python/bt2/bt2/field.py
+++ b/src/bindings/python/bt2/bt2/field.py
@@ -180,6 +180,9 @@ class _NumericFieldConst(_FieldConst):
         except Exception:
             return False
 
+    def __hash__(self):
+        return hash(self._value)
+
     def __rmod__(self, other):
         return self._extract_value(other) % self._value
 
@@ -242,7 +245,10 @@ class _NumericFieldConst(_FieldConst):
 
 
 class _NumericField(_NumericFieldConst, _Field):
-    pass
+    def __hash__(self):
+        # Non const field are not hashable as their value may be modified
+        # without changing the underlying Python object.
+        raise TypeError('unhashable type: \'{}\''.format(self._NAME))
 
 
 class _IntegralFieldConst(_NumericFieldConst, numbers.Integral):
@@ -483,6 +489,9 @@ class _StringFieldConst(_FieldConst):
     def __bool__(self):
         return bool(self._value)
 
+    def __hash__(self):
+        return hash(self._value)
+
     def _repr(self):
         return repr(self._value)
 
@@ -513,6 +522,11 @@ class _StringField(_StringFieldConst, _Field):
         )
         return self
 
+    def __hash__(self):
+        # Non const field are not hashable as their value may be modified
+        # without changing the underlying Python object.
+        raise TypeError('unhashable type: \'{}\''.format(self._NAME))
+
 
 class _ContainerFieldConst(_FieldConst):
     def __bool__(self):

diff --git a/tests/bindings/python/bt2/test_field.py b/tests/bindings/python/bt2/test_field.py
index cdacd7ffa63971e462029877080ef744a58dc58a..a0ff8e7639398088326c2b3c621475ede49d2928 100644 (file)
--- a/tests/bindings/python/bt2/test_field.py
+++ b/tests/bindings/python/bt2/test_field.py
@@ -660,6 +660,18 @@ class _TestNumericField:
     def test_str_op(self):
         self.assertEqual(str(self._def), str(self._def_value))
 
+    def test_hash_op(self):
+        with self.assertRaises(TypeError):
+            hash(self._def)
+
+    def test_const_hash_op(self):
+        self.assertEqual(hash(self._def_const), hash(self._def_value))
+
+    def test_const_hash_dict(self):
+        my_dict = {}
+        my_dict[self._def_const] = 'my_value'
+        self.assertEqual(my_dict[self._def_value], 'my_value')
+
     def test_eq_none(self):
         # Ignore this lint error:
         #   E711 comparison to None should be 'if cond is None:'
@@ -1287,6 +1299,10 @@ _inject_numeric_testing_methods(_TestIntegerFieldCommon)
 
 
 class SignedIntegerFieldTestCase(_TestIntegerFieldCommon, unittest.TestCase):
+    @staticmethod
+    def _const_value_setter(field):
+        field.value = 17
+
     def _create_fc(self, tc):
         return tc.create_signed_integer_field_class(25)
 
@@ -1297,10 +1313,17 @@ class SignedIntegerFieldTestCase(_TestIntegerFieldCommon, unittest.TestCase):
         self._def = _create_field(self._tc, self._create_fc(self._tc))
         self._def.value = 17
         self._def_value = 17
+        self._def_const = _create_const_field(
+            self._tc, self._create_fc(self._tc), self._const_value_setter
+        )
         self._def_new_value = -101
 
 
 class SignedEnumerationFieldTestCase(_TestIntegerFieldCommon, unittest.TestCase):
+    @staticmethod
+    def _const_value_setter(field):
+        field.value = 17
+
     def _create_fc(self, tc):
         fc = tc.create_signed_enumeration_field_class(32)
         fc.add_mapping('something', bt2.SignedIntegerRangeSet([(17, 17)]))
@@ -1318,6 +1341,9 @@ class SignedEnumerationFieldTestCase(_TestIntegerFieldCommon, unittest.TestCase)
         self._def = _create_field(self._tc, self._create_fc(self._tc))
         self._def.value = 17
         self._def_value = 17
+        self._def_const = _create_const_field(
+            self._tc, self._create_fc(self._tc), self._const_value_setter
+        )
         self._def_new_value = -101
 
     def test_str_op(self):
@@ -1342,6 +1368,10 @@ class SignedEnumerationFieldTestCase(_TestIntegerFieldCommon, unittest.TestCase)
 
 
 class RealFieldTestCase(_TestNumericField, unittest.TestCase):
+    @staticmethod
+    def _const_value_setter(field):
+        field.value = 52.7
+
     def _create_fc(self, tc):
         return tc.create_real_field_class()
 
@@ -1349,6 +1379,9 @@ class RealFieldTestCase(_TestNumericField, unittest.TestCase):
         self._tc = get_default_trace_class()
         self._field = _create_field(self._tc, self._create_fc(self._tc))
         self._def = _create_field(self._tc, self._create_fc(self._tc))
+        self._def_const = _create_const_field(
+            self._tc, self._tc.create_real_field_class(), self._const_value_setter
+        )
         self._def.value = 52.7
         self._def_value = 52.7
         self._def_new_value = -17.164857
@@ -1541,6 +1574,18 @@ class StringFieldTestCase(unittest.TestCase):
         self._def_value += to_append
         self.assertEqual(self._def, self._def_value)
 
+    def test_hash_op(self):
+        with self.assertRaises(TypeError):
+            hash(self._def)
+
+    def test_const_hash_op(self):
+        self.assertEqual(hash(self._def_const), hash(self._def_value))
+
+    def test_const_hash_dict(self):
+        my_dict = {}
+        my_dict[self._def_const] = 'my_value'
+        self.assertEqual(my_dict[self._def_value], 'my_value')
+
 
 class _TestArrayFieldCommon:
     def _modify_def(self):