Skip to content

Commit

Permalink
Merge pull request skulpt#1435 from s-cork/pr/intern-ints
Browse files Browse the repository at this point in the history
make skulpt "is" equivalent to object identity
  • Loading branch information
bnmnetp authored Jul 8, 2022
2 parents f9eca3f + 2927aec commit e69932e
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 53 deletions.
11 changes: 9 additions & 2 deletions src/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -655,8 +655,15 @@ Compiler.prototype.ccompare = function (e) {

for (i = 0; i < n; ++i) {
rhs = this.vexpr(e.comparators[i]);
out("$ret = Sk.misceval.richCompareBool(", cur, ",", rhs, ",'", e.ops[i].prototype._astname, "', true);");
this._checkSuspension(e);
const op = e.ops[i];
if (op === Sk.astnodes.Is) {
out("$ret = ", cur, "===", rhs, ";");
} else if (op === Sk.astnodes.IsNot) {
out("$ret = ", cur, "!==", rhs, ";");
} else{
out("$ret = Sk.misceval.richCompareBool(", cur, ",", rhs, ",'", op.prototype._astname, "', true);");
this._checkSuspension(e);
}
out(fres, "=Sk.builtin.bool($ret);");
this._jumpfalse("$ret", done);
cur = rhs;
Expand Down
33 changes: 23 additions & 10 deletions src/int.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,15 @@ Sk.builtin.int_ = Sk.abstr.buildNativeClass("int", {
constructor: function int_(x) {
Sk.asserts.assert(this instanceof Sk.builtin.int_, "bad call to int use 'new'");
let v;
if (typeof x === "number" || JSBI.__isBigInt(x)) {
if (typeof x === "number") {
if (x > -6 && x < 257) {
return INTERNED_INT[x];
}
v = x;
} else if (JSBI.__isBigInt(x)) {
v = x;
} else if (x === undefined) {
v = 0;
return INT_ZERO;
} else if (typeof x === "string") {
v = stringToNumberOrBig(x);
} else if (x.nb$int) {
Expand All @@ -45,10 +50,7 @@ Sk.builtin.int_ = Sk.abstr.buildNativeClass("int", {
x = args[0];
base = Sk.builtin.none.none$;
} else {
args = Sk.abstr.copyKeywordsToNamedArgs("int", [null, "base"], args, kwargs, [
new Sk.builtin.int_(0),
Sk.builtin.none.none$,
]);
args = Sk.abstr.copyKeywordsToNamedArgs("int", [null, "base"], args, kwargs, [INT_ZERO, Sk.builtin.none.none$]);
x = args[0];
base = args[1];
}
Expand Down Expand Up @@ -110,7 +112,7 @@ Sk.builtin.int_ = Sk.abstr.buildNativeClass("int", {
(v, w) => v - w,
(v, w) => JSBI.numberIfSafe(JSBI.subtract(v, w))
),
nb$multiply: numberSlot((v, w) => v * w, JSBI.multiply),
nb$multiply: numberSlot((v, w) => v * w, (v, w) => v === JSBI.__ZERO || w === JSBI.__ZERO ? 0 : JSBI.multiply(v, w)),
nb$divide: trueDivide,
nb$floor_divide: numberDivisionSlot((v, w) => Math.floor(v / w), BigIntFloorDivide),
nb$remainder: numberDivisionSlot(
Expand Down Expand Up @@ -206,7 +208,7 @@ Sk.builtin.int_ = Sk.abstr.buildNativeClass("int", {
},
imag: {
$get() {
return new Sk.builtin.int_(0);
return INT_ZERO;
},
$doc: "the imaginary part of a complex number",
},
Expand Down Expand Up @@ -857,8 +859,12 @@ const shiftconsts = [
*/
Sk.builtin.lng = Sk.abstr.buildNativeClass("long", {
base: Sk.builtin.int_, // not technically correct but makes backward compatibility easy
constructor: function lng(x) {
Sk.builtin.int_.call(this, x);
constructor: function lng (x) {
let ret = Sk.builtin.int_.call(this, x);
if (ret !== undefined) {
// using an interned int
this.v = ret.v;
}
},
slots: /** @lends {Sk.builtin.lng.prototype} */ {
$r() {
Expand All @@ -875,3 +881,10 @@ Sk.builtin.lng = Sk.abstr.buildNativeClass("long", {
});

const intProto = Sk.builtin.int_.prototype;


const INTERNED_INT = [];
for (let i = -5; i < 257; i++) {
INTERNED_INT[i] = Object.create(Sk.builtin.int_.prototype, {v: {value: i}});
}
const INT_ZERO = INTERNED_INT[0];
27 changes: 3 additions & 24 deletions src/misceval.js
Original file line number Diff line number Diff line change
Expand Up @@ -409,33 +409,12 @@ Sk.misceval.richCompareBool = function (v, w, op, canSuspend) {
}

// handle identity and membership comparisons
// no longer called from compile code - left here for backwards compatibliity
if (op === "Is") {
if (v_type === w_type) {
if (v === w) {
return true;
} else if (v_type === Sk.builtin.float_) {
return v.v === w.v;
} else if (v_type === Sk.builtin.int_) {
if (typeof v.v === "number" && typeof v.v === "number") {
return v.v === w.v;
}
return JSBI.equal(JSBI.BigInt(v.v), JSBI.BigInt(w.v));
}
}
return false;
return v === w;
}

if (op === "IsNot") {
if (v_type !== w_type) {
return true;
} else if (v_type === Sk.builtin.float_) {
return v.v !== w.v;
} else if (v_type === Sk.builtin.int_) {
if (typeof v.v === "number" && typeof v.v === "number") {
return v.v !== w.v;
}
return JSBI.notEqual(JSBI.BigInt(v.v), JSBI.BigInt(w.v));
}
return v !== w;
}

Expand Down Expand Up @@ -468,7 +447,7 @@ Sk.misceval.richCompareBool = function (v, w, op, canSuspend) {
return Sk.misceval.isTrue(ret);
}
}
if ((ret = v[shortcut](w)) !== Sk.builtin.NotImplemented.NotImplemented$) {
if ((ret= v[shortcut](w)) !== Sk.builtin.NotImplemented.NotImplemented$) {
return Sk.misceval.isTrue(ret);
// techincally this is not correct along with the compile code see #1252
// richcompare slots could return any pyObject ToDo - would require changing compile code
Expand Down
20 changes: 10 additions & 10 deletions test/run/t429.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ def helper(x,y,expect):
l.append((x is y)==False)
l.append((x is not y)==True)
if all(l):
print True
print(True)
else:
print False,x,y,l
print(False,x,y,l)

print "\nINTEGERS"
print("\nINTEGERS")
helper(1,2,-1)
helper(1,1,0)
helper(2,1,1)
Expand All @@ -48,37 +48,37 @@ def helper(x,y,expect):
helper(-1,1,-1)
helper(1,-1,1)

print "\nLONG INTEGERS"
print("\nLONG INTEGERS")
helper(1L,2L,-1)
helper(2L,1L,1)
helper(-1L,1L,-1)
helper(1L,-1L,1)

print "\nFLOATING POINT"
print("\nFLOATING POINT")
helper(1.0,2.0,-1)
helper(1.0,1.0,0)
helper(2.0,1.0,1)
helper(-2.0,-1.0,-1)
helper(-2.0,-2.0,0)
# helper(-2.0,-2.0,0)
helper(-1.0,-2.0,1)
helper(-1.0,1.0,-1)
helper(1.0,-1.0,1)

print "\nLISTS"
print("\nLISTS")
helper([],[1],-1)
helper([1,2],[1,2],0)
helper([1,2,3],[1,2],1)
helper([1,2],[2,1],-1)
helper([1,2,3],[1,2,1,5],1)

print "\nTUPLES"
print("\nTUPLES")
helper(tuple(),(1,),-1)
#helper((1,2),(1,2),0)
helper((1,2,3),(1,2),1)
helper((1,2),(2,1),-1)
helper((1,2,3),(1,2,1,5),1)

print "\nSTRINGS"
print("\nSTRINGS")
helper('','a',-1)
helper('a','a',0)
helper('ab','a',1)
Expand All @@ -90,7 +90,7 @@ class A:
def __init__(self,x): self.x = x
def __cmp__(self,other): return self.x

print "\nUSER-DEFINED OBJECTS"
print("\nUSER-DEFINED OBJECTS")
helper(A(-1),A(1),-1)
helper(A(0),A(0),0)
helper(A(1),A(-1),1)
1 change: 0 additions & 1 deletion test/run/t429.py.real
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ True
True
True
True
True

LISTS
True
Expand Down
4 changes: 2 additions & 2 deletions test/run/t520.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
print z, type(z)

print hash(True), type(hash(True))
print hash(None) is hash(None), type(hash(None))
print hash("hello") is hash("hello"), type(hash("hello"))
# print hash(None) is hash(None), type(hash(None)) # not actually correct in cpython
# print hash("hello") is hash("hello"), type(hash("hello"))

a = hasattr("hello", "not_a_method")
print a, type(a)
2 changes: 0 additions & 2 deletions test/run/t520.py.real
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,4 @@ True <type 'bool'>
True <type 'bool'>
True <type 'bool'>
1 <type 'int'>
True <type 'int'>
True <type 'int'>
False <type 'bool'>
2 changes: 1 addition & 1 deletion test/unit3/test_compare.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def helper(x,y,expect):
self.assertTrue(helper(1.0,1.0,0))
self.assertTrue(helper(2.0,1.0,1))
self.assertTrue(helper(-2.0,-1.0,-1))
self.assertTrue(helper(-2.0,-2.0,0))
# self.assertTrue(helper(-2.0,-2.0,0))
self.assertTrue(helper(-1.0,-2.0,1))
self.assertTrue(helper(-1.0,1.0,-1))
self.assertTrue(helper(1.0,-1.0,1))
Expand Down
2 changes: 1 addition & 1 deletion test/unit3/test_suspensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_any(self):
self.assertEqual(all(sleeping_gen([4, 0, 5])), False)
def test_sum(self):
self.assertEqual(sum(sleeping_gen([1, 2, 3])), 6)
self.assertIs(sum(sleeping_gen([1, 2.0, 3])), 6.0)
self.assertEqual(sum(sleeping_gen([1, 2.0, 3])), 6.0)
self.assertEqual(sum(sleeping_gen([[1], [2]]), []), [1, 2])

def test_builtin_types(self):
Expand Down

0 comments on commit e69932e

Please sign in to comment.