Avoid StackOverflow when comparing unknown foreign objects (#7780)

Closes #7677 by eliminating the _stackoverflow execption_. In general it seems _too adventurous_ to walk members of random foreign objects. There can be anything including cycles. Rather than trying to be too smart in these cases, let's just rely on `InteropLibrary.isIdentical` message.

# Important Notes
Calling `sort` on the `numpy` array no longer yields an error, but the array isn't sorted - that needs a fix on the Python side: https://github.com/oracle/graalpython/issues/354 - once it is in, the elements will be treated as numbers and the sorting happens automatically (without any changes in Enso code).
This commit is contained in:
Jaroslav Tulach 2023-09-19 17:39:04 +02:00 committed by GitHub
parent 5c198145a1
commit 3b790606e1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 114 additions and 49 deletions

View File

@ -370,50 +370,10 @@ public abstract class EqualsComplexNode extends Node {
@Shared("typesLib") @CachedLibrary(limit = "10") TypesLibrary typesLib, @Shared("typesLib") @CachedLibrary(limit = "10") TypesLibrary typesLib,
@Shared("equalsNode") @Cached EqualsNode equalsNode, @Shared("equalsNode") @Cached EqualsNode equalsNode,
@Shared("hostValueToEnsoNode") @Cached HostValueToEnsoNode valueToEnsoNode) { @Shared("hostValueToEnsoNode") @Cached HostValueToEnsoNode valueToEnsoNode) {
try { if (interop.isIdentical(selfObject, otherObject, interop)) {
Object selfMembers = interop.getMembers(selfObject);
Object otherMembers = interop.getMembers(otherObject);
assert interop.getArraySize(selfMembers) < Integer.MAX_VALUE
: "Long array sizes not supported";
int membersSize = (int) interop.getArraySize(selfMembers);
if (interop.getArraySize(otherMembers) != membersSize) {
return false;
}
// Check member names
String[] memberNames = new String[membersSize];
for (int i = 0; i < membersSize; i++) {
String selfMemberName = interop.asString(interop.readArrayElement(selfMembers, i));
String otherMemberName = interop.asString(interop.readArrayElement(otherMembers, i));
if (!equalsNode.execute(selfMemberName, otherMemberName)) {
return false;
}
memberNames[i] = selfMemberName;
}
// Check member values
for (int i = 0; i < membersSize; i++) {
if (interop.isMemberReadable(selfObject, memberNames[i])
&& interop.isMemberReadable(otherObject, memberNames[i])) {
Object selfMember =
valueToEnsoNode.execute(interop.readMember(selfObject, memberNames[i]));
Object otherMember =
valueToEnsoNode.execute(interop.readMember(otherObject, memberNames[i]));
if (!equalsNode.execute(selfMember, otherMember)) {
return false;
}
}
}
return true; return true;
} catch (UnsupportedMessageException } else {
| InvalidArrayIndexException return false;
| UnknownIdentifierException e) {
throw new IllegalStateException(
String.format(
"One of the interop objects has probably wrongly specified interop API "
+ "for members. selfObject = %s ; otherObject = %s",
selfObject, otherObject),
e);
} }
} }
@ -549,6 +509,9 @@ public abstract class EqualsComplexNode extends Node {
if (interop.isTime(object)) { if (interop.isTime(object)) {
return false; return false;
} }
if (interop.hasHashEntries(object)) {
return false;
}
return interop.hasMembers(object); return interop.hasMembers(object);
} }

View File

@ -49,11 +49,16 @@ spec = Test.group "Python Examples" <|
main = dir/"src"/"Main.enso" main = dir/"src"/"Main.enso"
main.exists . should_be_true main.exists . should_be_true
code = """ code = """
from Standard.Base import all
foreign python random_array s = """ foreign python random_array s = """
import numpy import numpy
return numpy.random.normal(size=s) return numpy.random.normal(size=s)
main = random_array 10 main =
arr = random_array 10
vec = arr.to_vector.sort
[ arr, vec ]
code . write main on_existing_file=Existing_File_Behavior.Overwrite code . write main on_existing_file=Existing_File_Behavior.Overwrite

View File

@ -0,0 +1,22 @@
package org.enso.base_test_helpers;
public final class IntHolderEquals {
public final int value;
public final Integer boxed;
public IntHolderEquals(int value) {
this.value = value;
this.boxed = value;
}
public int hashCode() {
return value;
}
public boolean equals(Object obj) {
if (obj instanceof IntHolderEquals other) {
return value == other.value;
}
return false;
}
}

View File

@ -760,13 +760,13 @@ type_spec name alter = Test.group name <|
alter [T.Value 1 2, T.Value 3 3, T.Value 1 2] . distinct . should_equal [T.Value 1 2, T.Value 3 3] alter [T.Value 1 2, T.Value 3 3, T.Value 1 2] . distinct . should_equal [T.Value 1 2, T.Value 3 3]
alter [T.Value 1 2, T.Value 3 3, T.Value 1 2, Nothing] . distinct . should_equal [T.Value 1 2, T.Value 3 3, Nothing] alter [T.Value 1 2, T.Value 3 3, T.Value 1 2, Nothing] . distinct . should_equal [T.Value 1 2, T.Value 3 3, Nothing]
alter [Nothing, T.Value 1 2, T.Value 3 3, T.Value 1 2, Nothing] . distinct . should_equal [Nothing, T.Value 1 2, T.Value 3 3] alter [Nothing, T.Value 1 2, T.Value 3 3, T.Value 1 2, Nothing] . distinct . should_equal [Nothing, T.Value 1 2, T.Value 3 3]
alter [T.Value 1 2, Date.new hours=3] . distinct . should_equal [T.Value 1 2, Date.new hours=3] alter [T.Value 1 2, Date.new year=1973] . distinct . should_equal [T.Value 1 2, Date.new year=1973]
Test.specify "should return a vector containing only unique elements up to some criteria" <| Test.specify "should return a vector containing only unique elements up to some criteria" <|
alter [Pair.new 1 "a", Pair.new 2 "b", Pair.new 1 "c"] . distinct (on = _.first) . should_equal [Pair.new 1 "a", Pair.new 2 "b"] alter [Pair.new 1 "a", Pair.new 2 "b", Pair.new 1 "c"] . distinct (on = _.first) . should_equal [Pair.new 1 "a", Pair.new 2 "b"]
Test.specify "should be able to sort a heterogenous vector" <| Test.specify "should be able to sort a heterogenous vector" <|
arr = [ 1, 1.3, "hi", Date.today, Date_Time.now, [ 0 ] ] arr = alter [ 1, 1.3, "hi", Date.today, Date_Time.now, [ 0 ] ]
(arr.sort on=(.to_text) . map .to_text) . should_equal (arr.map .to_text . sort) (arr.sort on=(.to_text) . map .to_text) . should_equal (arr.map .to_text . sort)
(arr.sort on=(_.to_text) . map .to_text) . should_equal (arr.map .to_text . sort) (arr.sort on=(_.to_text) . map .to_text) . should_equal (arr.map .to_text . sort)
(arr.sort on=(x-> x.to_text) . map .to_text) . should_equal (arr.map .to_text . sort) (arr.sort on=(x-> x.to_text) . map .to_text) . should_equal (arr.map .to_text . sort)

View File

@ -3,8 +3,11 @@ from Standard.Base import all
from Standard.Test import Test, Test_Suite from Standard.Test import Test, Test_Suite
import Standard.Test.Extensions import Standard.Test.Extensions
polyglot java import java.nio.file.Path as JavaPath polyglot java import java.math.BigInteger as Java_Big_Integer
polyglot java import java.nio.file.Path as Java_Path
polyglot java import java.util.Random as Java_Random polyglot java import java.util.Random as Java_Random
polyglot java import org.enso.base_test_helpers.IntHolder
polyglot java import org.enso.base_test_helpers.IntHolderEquals
type CustomEqType type CustomEqType
C1 f1 C1 f1
@ -110,6 +113,24 @@ foreign js js_true = """
foreign js js_text_foo = """ foreign js js_text_foo = """
return 'foo' return 'foo'
foreign js js_json a b = """
return JSON.parse(`
{
"a" : ${a},
"b" : ${b}
}`);
foreign js js_map a b = """
var m = new Map()
m.set("a", a)
m.set("b", b);
return m
foreign js js_obj a b = """
var m = {};
m.a = a;
m.b = b;
return m;
spec = spec =
Test.group "Operator ==" <| Test.group "Operator ==" <|
@ -158,8 +179,8 @@ spec =
((CustomEqType.C1 0) == (CustomEqType.C2 7 3)).should_be_false ((CustomEqType.C1 0) == (CustomEqType.C2 7 3)).should_be_false
Test.specify "should dispatch to equals on host values" <| Test.specify "should dispatch to equals on host values" <|
path1 = JavaPath.of "home" "user" . resolve "file.txt" path1 = Java_Path.of "home" "user" . resolve "file.txt"
path2 = JavaPath.of "home" "user" "file.txt" path2 = Java_Path.of "home" "user" "file.txt"
(path1 == path2).should_be_true (path1 == path2).should_be_true
path3 = path1.resolve "subfile.txt" path3 = path1.resolve "subfile.txt"
(path3 == path2).should_be_false (path3 == path2).should_be_false
@ -203,5 +224,59 @@ spec =
dupl_tree = tree.deep_copy dupl_tree = tree.deep_copy
Test.with_clue "Seed sed to 42" (tree == dupl_tree).should_be_true Test.with_clue "Seed sed to 42" (tree == dupl_tree).should_be_true
Test.specify "partially applied constructors aren't == " <|
f1 = CustomEqType.C2 10
f2 = CustomEqType.C2 10
f1==f2 . should_be_false
Test.group "Polyglot Operator ==" <|
Test.specify "should not try to compare members" <|
x = IntHolder.new 5
y = IntHolder.new 5
z = IntHolder.new 3
x==z . should_be_false
x==y . should_be_false
y==y . should_be_true
x==x . should_be_true
z==z . should_be_true
Test.specify "should not try to compare members in JS object" <|
x = js_obj 5 3
y = js_obj 5 3
z = js_obj 3 5
x==z . should_be_false
x==y . should_be_false
y==y . should_be_true
x==x . should_be_true
z==z . should_be_true
Test.specify "should invoke equals" <|
x = IntHolderEquals.new 5
y = IntHolderEquals.new 5
z = IntHolderEquals.new 3
x==z . should_be_false
x==y . should_be_true
y==y . should_be_true
x==x . should_be_true
z==z . should_be_true
Test.specify "should compare big integer" <|
x = Java_Big_Integer.new "54024430107564432"
y = Java_Big_Integer.new "54024430107564432"
x==y . should_be_true
Test.specify "JSON is found different" <|
x = js_json 10 5
y = js_json 10 5
x==y . should_be_false
Test.specify "Identical JSON is found equal" <|
x = js_json 10 5
x==x . should_be_true
Test.specify "JavaScript Map is found same" <|
x = js_map 10 5
y = js_map 10 5
x==y . should_be_true
main = Test_Suite.run_main spec main = Test_Suite.run_main spec