Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce hash contract for objects at runtime #5611

Open
Akirathan opened this issue Feb 10, 2023 · 2 comments
Open

Enforce hash contract for objects at runtime #5611

Akirathan opened this issue Feb 10, 2023 · 2 comments
Assignees
Labels
--bug Type: bug -compiler -syntax p-lowest Should be completed at some point

Comments

@Akirathan
Copy link
Member

According to the docs from Ordering.enso, hash contract means that for any objects x, y, z:

  • Hash consistency:
    • If x == y then hash(x) == hash(y)
    • If hash(x) != hash(y) then x != y
  • Consistency: if x == y then x == y for all the subsequent invocations.
  • Symmetry: if x == y then y == x
  • Transitivity: if x < y and y < z then x < z
  • if x > y then y < x

On the latest develop (1f8511d) we do not check any of these properties at runtime. We should check these properties at runtime, at least from time to time.

The following diff shows a naive approach, how to check hash consistency naively:

diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso
index 4752b8b80..59ac58063 100644
--- a/distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso
+++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso
@@ -113,13 +113,16 @@ type Any
         if Meta.is_same_object eq_self Incomparable then False else
             similar_type = Meta.is_same_object eq_self eq_that
             if similar_type.not then False else
-                case eq_self.is_ordered of
-                    True ->
-                        # Comparable.equals_builtin is a hack how to directly access EqualsNode from the
-                        # engine, so that we don't end up in an infinite recursion here (which would happen
-                        # if we would compare with `eq_self == eq_that`).
-                        Comparable.equals_builtin (eq_self.compare self that) Ordering.Equal
-                    False -> eq_self.equals self that
+                self_hash = eq_self.hash self
+                that_hash = eq_that.hash that
+                if Comparable.equals_builtin self_hash that_hash . not then False else
+                    case eq_self.is_ordered of
+                        True ->
+                            # Comparable.equals_builtin is a hack how to directly access EqualsNode from the
+                            # engine, so that we don't end up in an infinite recursion here (which would happen
+                            # if we would compare with `eq_self == eq_that`).
+                            Comparable.equals_builtin (eq_self.compare self that) Ordering.Equal
+                        False -> eq_self.equals self that
 
     ## ALIAS Inequality

Note that we do not want to check that in each invocation of Any.== because it could have major performance impact. For example, for an atom with a multilevel field hierarchy, checking equality is potentially faster than computing hash code.

Related discussion on the PR:

@Akirathan
Copy link
Member Author

An alternative solution is to provide a TCK for hash contract as a separate module. But it is possible that not many users would use that.

@Akirathan Akirathan removed this from Issues Board Feb 10, 2023
@jdunkerley jdunkerley added the p-lowest Should be completed at some point label Feb 10, 2023
@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Feb 14, 2023
@Akirathan
Copy link
Member Author

Let's implement this via asserts - #5973

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -compiler -syntax p-lowest Should be completed at some point
Projects
None yet
Development

No branches or pull requests

2 participants