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

Equality with conversions #9070

Merged
merged 29 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
dce1b25
Enabling equality tests for Complex and Number
JaroslavTulach Feb 15, 2024
7c46739
Adding one more node in front of previously existing EqualsNode
JaroslavTulach Feb 15, 2024
b0a4e48
First of all compute results only then verify them
JaroslavTulach Feb 15, 2024
9f4cde3
Number can be equal to different type. Check with ==
JaroslavTulach Feb 15, 2024
a214c0b
Supporting conversions during evaluation of ==
JaroslavTulach Feb 15, 2024
a2bfb88
Removing accidentally committed fix from other PR
JaroslavTulach Feb 15, 2024
10f7941
TruffleObject vs. boolean specialization
JaroslavTulach Feb 15, 2024
b308014
Encapsulating in equalityCheck method
JaroslavTulach Feb 15, 2024
290b63b
Uncached version of the node may not use cached libraries
JaroslavTulach Feb 15, 2024
bbbbf73
Fool is now equal to anything as there is a Fool.from (that:Any) conv…
JaroslavTulach Feb 15, 2024
3fde985
Don't serialize caches in unit tests
JaroslavTulach Feb 15, 2024
ecd4b90
Enforce consistency of == and hash for converted objects
JaroslavTulach Feb 15, 2024
69510cd
Fool shall not be == unless it defines additional conversions
JaroslavTulach Feb 15, 2024
46666db
Merge remote-tracking branch 'origin/develop' into wip/jtulach/Equali…
JaroslavTulach Feb 16, 2024
0e249c0
Single letter typo fix
JaroslavTulach Feb 16, 2024
a5c0674
Both from conversions must be present in the module defining Complex …
JaroslavTulach Feb 16, 2024
145fc55
Equalities (and inequalities) with conversions test
JaroslavTulach Feb 17, 2024
6fac961
Ensure inconsistency of hash is reported when assert is on
JaroslavTulach Feb 17, 2024
1d16e40
Benchmark equality with conversion
JaroslavTulach Feb 17, 2024
866425c
Wrap also non-double values
JaroslavTulach Feb 17, 2024
3524bd9
Place the == on a separate line to make it more easily visible in IGV
JaroslavTulach Feb 17, 2024
2d708de
Reference to binary operator multiple dispatch
JaroslavTulach Feb 19, 2024
5d52d51
Describing multi-type custom equality
JaroslavTulach Feb 19, 2024
0b7f55c
Verify Comparator for both types is the same
JaroslavTulach Feb 19, 2024
8c88cc1
Use only one WithConversion node
JaroslavTulach Feb 19, 2024
8b48232
Merge remote-tracking branch 'origin/develop' into wip/jtulach/Equali…
JaroslavTulach Feb 19, 2024
12fd925
No need for Convert record. Boolean is enough.
JaroslavTulach Feb 19, 2024
f9af3ad
Fixing typo
JaroslavTulach Feb 19, 2024
d3510a8
Hide InvokeFunctionNode.build behind @TruffleBoundary
JaroslavTulach Feb 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ Number.should_equal : Float -> Float -> Integer -> Spec_Result
Number.should_equal self that epsilon=0 frames_to_skip=0 =
matches = case that of
_ : Number -> self.equals that epsilon
_ -> False
_ -> self==that
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved
case matches of
True -> Spec_Result.Success
False ->
Expand Down Expand Up @@ -519,7 +519,7 @@ Error.should_contain_the_same_elements_as self that frames_to_skip=0 =

It checks that all elements from `self` are also present in `that`. It does
not require that all elements of `that` are contained in `self`. Arities of
elements are not checked, so `self` may still contain more elements than
elements are not checked, so `self` may still contain more elements than
`that` by containing duplicates.

It will work on any collection which supports the methods
Expand Down
38 changes: 38 additions & 0 deletions docs/syntax/functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,44 @@ the addition by invoking `Num.+`. This behavior allows one to write libraries
that extend existing number types with `Complex_Number`, `Rational_Number` and
make them behave as first class citizen numbers.

### Custom Equality

The `==` operator is special. A consistency with hash code is necessary to make
any Enso object behave correctly and work effectively in `Set` and `Map`
implementations. To guarantee such level of consistency there is a `Any.==`
definition providing _universal equality_ that **shall not be overriden**.

The `==` behavior is predefined for builtin types, atoms and other Enso objects.
In addition to that it remains possible to define own _comparators_, including a
comparator capable to work with already existing types. To create such
comparator define:

- conversion between existing type and the new type (as described in
[previous section](#type-ascriptions-and-operator-resolution))
- comparator (see documentation of `Ordering` type)
- define **two conversion method** that return the same comparator

To extend the previous definition of `Num` also for equality one might do for
example:

```ruby
type Num_Comparator
compare a:Num b:Num = # compare somehow
hash a:Num = # hash somehow

Num.from (that:Integer) = # convert somehow
Comparable.from (_:Num) = Num_Comparator
Comparable.from (_:Integer) = Num_Comparator
```

with such a structure the internal implementation of `Any.==` performs necessary
conversions of `Integer` argument in case the other argument is `Num` and
invokes the `Num_Comparator.compare` to handle the comparision.

A care must be taken to keep consistency between `hash` values of original and
converted types - e.g. hash of `n:Integer` and hash of `Num.from n` must be the
same (otherwise consistency required for `Set` and `Map` would be compromised).

### Precedence

Operator precedence in Enso is a collection of rules that reflect conventions
Expand Down
11 changes: 7 additions & 4 deletions docs/types/dynamic-dispatch.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,13 @@ another.

## Multiple Dispatch

It is an open question as to whether we want to support proper multiple dispatch
in Enso. Multiple dispatch refers to the dynamic dispatch target being
determined based not only on the type of the `this` argument, but the types of
the other arguments to the function.
Multiple dispatch is currently used for
[binary operators](../syntax/functions.md#type-ascriptions-and-operator-resolution).

Supporting for general functions remains an open question as to whether we want
to support proper multiple dispatch in Enso. Multiple dispatch refers to the
dynamic dispatch target being determined based not only on the type of the
`this` argument, but the types of the other arguments to the function.

To do multiple dispatch properly, it is very important to get a rigorous
specification of the specificity algorithm. It must account for:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,21 @@ public void initializeBenchmark(BenchmarkParams params) throws Exception {
new StringBuilder(
"""
import Standard.Base.Data.Range.Extensions
import Standard.Base.Data.Numbers.Number
import Standard.Base.Data.Ordering.Ordering
import Standard.Base.Data.Ordering.Comparable
import Standard.Base.Data.Ordering.Default_Comparator

type Num
Value n

Num.from (that:Number) = Num.Value that
Comparable.from (_:Number) = Num_Comparator
Comparable.from (_:Num) = Num_Comparator

type Num_Comparator
compare x:Num y:Num = Ordering.compare x.n y.n
hash x:Num = Default_Comparator.hash x.n

type Node
C1 f1
Expand All @@ -87,7 +102,9 @@ public void initializeBenchmark(BenchmarkParams params) throws Exception {

eq_vec vec1 vec2 =
(0.up_to vec1.length).map idx->
(vec1.at idx) == (vec2.at idx)
v1 = vec1.at idx
v2 = vec2.at idx
v1 == v2

eq x y = x == y
""");
Expand All @@ -108,10 +125,37 @@ public void initializeBenchmark(BenchmarkParams params) throws Exception {
primitiveVectorSize / 64);
codeBuilder
.append(
generateVectorOfPrimitives(primitiveVectorSize, "vec1", 42, trueExpectedAt, random))
generateVectorOfPrimitives(
primitiveVectorSize, "vec1", 42, trueExpectedAt, random, "%d", "%f"))
.append("\n")
.append(
generateVectorOfPrimitives(
primitiveVectorSize, "vec2", 42, trueExpectedAt, random, "%d", "%f"))
.append("\n");
}
case "equalsWithConversion" -> {
trueExpectedAt =
Set.of(
primitiveVectorSize / 2,
primitiveVectorSize / 4,
primitiveVectorSize / 8,
primitiveVectorSize / 16,
primitiveVectorSize / 32,
primitiveVectorSize / 64);
codeBuilder
.append(
generateVectorOfPrimitives(
primitiveVectorSize, "vec1", 42, trueExpectedAt, random, "%d", "%f"))
.append("\n")
.append(
generateVectorOfPrimitives(primitiveVectorSize, "vec2", 42, trueExpectedAt, random))
generateVectorOfPrimitives(
primitiveVectorSize,
"vec2",
42,
trueExpectedAt,
random,
"Num.Value %d",
"Num.Value %f"))
.append("\n");
}
case "equalsStrings" -> {
Expand Down Expand Up @@ -141,7 +185,7 @@ public void initializeBenchmark(BenchmarkParams params) throws Exception {
}

codeBuilder.append("""
bench x = eq_vec vec1 vec2
bench _ = eq_vec vec1 vec2
""");

module = ctx.eval(SrcUtil.source(benchmarkName, codeBuilder.toString()));
Expand Down Expand Up @@ -171,6 +215,11 @@ public void equalsPrimitives(Blackhole blackHole) {
performBenchmark(blackHole);
}

@Benchmark
public void equalsWithConversion(Blackhole blackHole) {
performBenchmark(blackHole);
}

@Benchmark
public void equalsStrings(Blackhole blackhole) {
performBenchmark(blackhole);
Expand Down Expand Up @@ -207,7 +256,9 @@ private static String generateVectorOfPrimitives(
String vecName,
Object identityElem,
Collection<Integer> constantIdxs,
Random random) {
Random random,
String intFormat,
String doubleFormat) {
var partSize = totalSize / 2;
List<Object> primitiveValues = new ArrayList<>();
random.ints(partSize).forEach(primitiveValues::add);
Expand All @@ -221,9 +272,9 @@ private static String generateVectorOfPrimitives(
sb.append(vecName).append(" = [");
for (Object primitiveValue : primitiveValues) {
if (primitiveValue instanceof Double dbl) {
sb.append(String.format("%f", dbl)).append(",");
sb.append(String.format(doubleFormat, dbl)).append(",");
} else {
sb.append(primitiveValue).append(",");
sb.append(String.format(intFormat, primitiveValue)).append(",");
}
}
// Replace last comma
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
package org.enso.interpreter.test;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.util.List;
import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.PolyglotException;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;

public class EqualsConversionsTest extends TestBase {
private static Context context;

@BeforeClass
public static void initContextAndData() {
context = createDefaultContext();
}

@AfterClass
public static void disposeContext() {
context.close();
}

@Test
public void testBasicInequalities() {
var results =
TestBase.evalModule(
context,
"""
from Standard.Base import all

Text.from (that:Number) = that.to_text

main =
r0 = "4"+"2" == "42"
r1 = 42 == "42"
r2 = "42" == 42
[ r0, r1, r2 ]
""")
.as(List.class);

assertTrue("strings are equal: " + results, (boolean) results.get(0));
assertFalse("string is not equal to number: " + results, (boolean) results.get(1));
assertFalse("number is not equal to string: " + results, (boolean) results.get(2));
}

@Test
public void testNumWrapperAroundIntegerIsEqualToInteger() {
var gen = new DefineComparableWrapper();
gen.intNumConversion = true;
gen.intComparator = true;
gen.numComparator = true;
assertTrue("Num.Value equal to Integer: ", gen.evaluate());
}

@Test
public void testMissingIntegerNumConversion() {
var gen = new DefineComparableWrapper();
gen.intNumConversion = false;
gen.intComparator = true;
gen.numComparator = true;
assertFalse("Num.Value not equal to Integer: ", gen.evaluate());
}

@Test
public void testMissingIntComparator() {
var gen = new DefineComparableWrapper();
gen.intNumConversion = true;
gen.intComparator = false;
gen.numComparator = true;
assertFalse("Num.Value not equal to Integer: ", gen.evaluate());
}

@Test
public void testMissingNumComparator() {
var gen = new DefineComparableWrapper();
gen.intNumConversion = true;
gen.intComparator = true;
gen.numComparator = false;
assertFalse("Num.Value not equal to Integer: ", gen.evaluate());
}

@Test
public void testInconsistentHashFunction() {
var gen = new DefineComparableWrapper();
gen.intNumConversion = true;
gen.intComparator = true;
gen.numComparator = true;
gen.hashFn = "x.n*31";

boolean r;
try {
r = gen.evaluate();
} catch (PolyglotException ex) {
assertTrue(ex.getMessage(), ex.getMessage().contains("Different hash code!"));
return;
}
fail("Expecting assertion error, not: " + r);
}

private static final class DefineComparableWrapper {
boolean intNumConversion;
boolean numComparator;
boolean intComparator;
String hashFn = "Default_Comparator.hash x.n";

boolean evaluate() {
var block0 =
"""
from Standard.Base import all

type Num
Value n:Integer

type Num_Comparator
compare x:Num y:Num = Ordering.compare x.n y.n
hash x:Num = {hashFn}
"""
.replace("{hashFn}", hashFn);

var block1 =
!intNumConversion
? ""
: """
Num.from (that:Integer) = Num.Value that
""";

var block2 =
!numComparator
? ""
: """
Comparable.from (_:Num) = Num_Comparator
""";

var block3 =
!intComparator ? "" : """
Comparable.from (_:Integer) = Num_Comparator
""";

var mainBlock =
"""
main =
num42 = Num.Value 42

r0 = 42 == num42
r0
""";
var res = TestBase.evalModule(context, block0 + block1 + block2 + block3 + mainBlock);
return res.asBoolean();
}
}
}
Loading
Loading