Add Comparator conversion for all types #4067

Akirathan · 2023-01-19T10:46:29Z

Pull Request Description

Add Comparator type class emulation for all types. Migrate all the types in stdlib to this new Comparator API. The main documentation is in Ordering.enso.

Fixes these pivotals:

Important Notes

The new Comparator API forces users to specify both equals and hash methods on their custom comparators.
All the compare_to overrides were replaced by definition of a custom ordered comparator.
All the call sites of x.compare_to y method were replaced with Ordering.compare x y.
- Ordering.compare is essentially a shortcut for Comparable.from x . compare x y.
The default comparator for Any is Default_Unordered_Comparator, which just forwards to the builtin EqualsNode and HashCodeNode nodes.
For x, one can get its hash with Comparable.from x . hash x.
- This makes hash as hidden as possible. There are no other public methods to get a hash code of an object.
Comparing x and y can be done either by Ordering.compare x y or Comparable.from x . compare x y instead of x.compare_to y.

Checklist

Please include the following checklist in your PR:

The documentation has been updated if necessary.
All code conforms to the
Scala,
Java,
and
Rust
style guides.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed: Enso GUI was tested when built using BOTH
  ./run ide build and ./run ide watch.

- Revert changes in Any, so that we have still the old behavior - Define new Any.=== , Any.<== and Any.>== operators as extension methods in Eq.enso - Make Any.hash_code accessible as a temporary workaround

- Must not contain `self` keyword.

…_Values

- We should remove Eq_Spec in the future.

…atom with custom comparator is encountered. - EqualsNode calls `Any.==`. - HashCodeNode calls `Comparable.hash_callback`.

… members to Eq/Hash nodes

This reverts commit c0e08e9.

hubertp

Conditional approval based on our internal discussion.

Please file a follow up ticket for removing the need to import Standard.Base.Data.Any.Any when performing simple equality operation.

JaroslavTulach

I am really glad to see this endeavor converging! I might have some suggestions for a change, but I believe it is more important to integrate this while we are green and only then solve problems that might appear anywhere.

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Ordering.enso

JaroslavTulach · 2023-02-09T13:00:12Z

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Ordering.enso

+      - 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
+    - Reflexivity: x == x


Reflexivity should be enforced. Meta_is_same_object x y implies x == y.

JaroslavTulach · 2023-02-09T13:09:11Z

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Ordering.enso

+      - 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


Symmetry could be enforced. Just like we check hash code consistency we could also verify (from time to time) that x == y and y == x is consistent.

distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso

Critical performance improvements after #4067 # Important Notes - Replace if-then-else expressions in `Any.==` with case expressions. - Fix caching in `EqualsNode`. - This includes fixing specializations, along with fallback guard.

Akirathan force-pushed the wip/akirathan/custom-eq-183945328 branch from e997c5f to 0d6560f Compare January 19, 2023 10:48

Copy initial sources from jtulach/DefaultCustomEq branch

a170e53

Akirathan force-pushed the wip/akirathan/custom-eq-183945328 branch from 0d6560f to a170e53 Compare January 19, 2023 10:51

Akirathan added 6 commits January 23, 2023 15:56

Comparable.from returns union of three types

91c227d

- Revert changes in Any, so that we have still the old behavior - Define new Any.=== , Any.<== and Any.>== operators as extension methods in Eq.enso - Make Any.hash_code accessible as a temporary workaround

Fix Eq.assert_ordered_comparators.

7e4df51

- Must not contain `self` keyword.

Hide hash_code method as much as possible - Move it to Meta.hash_code

5b5fc70

When comparators are not of same type, comparison throws Incomparable…

f9c9bdd

…_Values

Any.== uses comparator pattern. Any.< is a builtin

d12c485

Refactor Eq_Spec to the newest comparator pattern.

93ed516

- We should remove Eq_Spec in the future.

Akirathan force-pushed the wip/akirathan/custom-eq-183945328 branch from fd14bb8 to 93ed516 Compare January 24, 2023 16:45

Akirathan added 20 commits January 24, 2023 17:45

Merge branch 'develop' into wip/akirathan/custom-eq-183945328

1399502

Any.== uses equals_builtin to avoid infinite recursion

803ffb1

Implement LessThanAnyNode.build - fix for generated code.

3d76e9f

Text has Default_Ordered_Comparator

f4e12f8

Fix a small bug in Type.getMetaParents

2958fa8

Comparable and default comparators are builtin types

0bfde33

Implement Comparable.equals_builtin and Comparable.less_builtin

468691b

Implement special handling for both EqualsNode and HashCodeNode once …

3a099b3

…atom with custom comparator is encountered. - EqualsNode calls `Any.==`. - HashCodeNode calls `Comparable.hash_callback`.

Handle Comparable.from conversion for numbers in Numbers module

f75b5c1

Migrate Equals_Spec to the new comparator API

ed03031

Cosmetics

c971644

Fix Map visualization

c0e08e9

Rename EqualsAnyNode to EqualsNode

fafd285

Rename LessThanAnyNode to LessThanNode

07e79d9

EqualsNode and LessThanNode is a static builtin method

5087323

Handle cases when there is no conversion to Comparable in Any.==

c1699ea

Add specializations for types, files, modules and interop values with…

bd2a5ff

… members to Eq/Hash nodes

Array.sort respect new comparator API

f293752

Improve signature of operators on Any

99c41cb

Fix some warnings

c6d5628

Akirathan and others added 8 commits February 8, 2023 19:26

Address some nitpicks from review

229ed44

Temporarily ignore Any == Boolean tests.

cb12a66

Merge branch 'develop' into wip/akirathan/custom-eq-183945328

867a30e

import Default_Unordered_Comparator in Map_Spec

610eaf2

Small fix in Vector_Spec

731b10e

Remove Eq_Spec

001e792

Revert "Fix Map visualization"

c3cb84f

This reverts commit c0e08e9.

Merge branch 'develop' into wip/akirathan/custom-eq-183945328

534a557

Akirathan requested review from JaroslavTulach and hubertp February 9, 2023 09:42

Akirathan added the -compiler label Feb 9, 2023

hubertp approved these changes Feb 9, 2023

View reviewed changes

JaroslavTulach approved these changes Feb 9, 2023

View reviewed changes

Akirathan added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Feb 9, 2023

fmt

658af5a

Akirathan removed the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Feb 9, 2023

Merge branch 'develop' into wip/akirathan/custom-eq-183945328

e7d0114

Akirathan added the CI: Ready to merge This PR is eligible for automatic merge label Feb 9, 2023

Merge branch 'develop' into wip/akirathan/custom-eq-183945328

2305fb2

JaroslavTulach added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Feb 10, 2023

mergify bot merged commit 1f8511d into develop Feb 10, 2023

mergify bot deleted the wip/akirathan/custom-eq-183945328 branch February 10, 2023 09:22

Akirathan mentioned this pull request Feb 10, 2023

Comparison operators should not require importing Standard.Base.Any module #5610

Closed

radeusgd reviewed Feb 10, 2023

View reviewed changes

distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso Show resolved Hide resolved

This was referenced Feb 10, 2023

Migrate stdlib to the new comparator API #5626

Closed

Severe performance regression of the Engine after Comparator API #5636

Closed

Akirathan mentioned this pull request Feb 17, 2023

Performance improvements for Comparators #5687

Merged

4 tasks

This was referenced Feb 22, 2023

Cache Comparator for every Type #5740

Closed

sort handles Incomparable types by default #5742

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Comparator conversion for all types #4067

Add Comparator conversion for all types #4067

Akirathan commented Jan 19, 2023 •

edited

Loading

hubertp left a comment

JaroslavTulach left a comment

JaroslavTulach Feb 9, 2023

JaroslavTulach Feb 9, 2023

Add Comparator conversion for all types #4067

Add Comparator conversion for all types #4067

Conversation

Akirathan commented Jan 19, 2023 • edited Loading

Pull Request Description

Important Notes

Checklist

hubertp left a comment

Choose a reason for hiding this comment

JaroslavTulach left a comment

Choose a reason for hiding this comment

JaroslavTulach Feb 9, 2023

Choose a reason for hiding this comment

JaroslavTulach Feb 9, 2023

Choose a reason for hiding this comment

Akirathan commented Jan 19, 2023 •

edited

Loading