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.

Sign up for GitHub

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

Use to primitive for plus operation and date (taken from #1611 done by @tonygermano) #1685

Merged
merged 6 commits into from
Oct 11, 2024
Merged

Use to primitive for plus operation and date (taken from #1611 done by @tonygermano) #1685

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d9e7181
rewritten plus operator to use toPrimitive
rbri Oct 7, 2024
bbd4e24
NativeDate#jsConstructor uses toPrimitive
rbri Oct 7, 2024
4330b4f
Date.prototype[Symbol.toPrimitive] implemented
rbri Oct 7, 2024
4666d67
fix e4x handling
rbri Oct 8, 2024
00548d0
fix another backward compatibility issues
rbri Oct 9, 2024
1ae1fca
remove not needed ';'
rbri Oct 10, 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
52 changes: 41 additions & 11 deletions rhino/src/main/java/org/mozilla/javascript/NativeDate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,10 @@ protected void initPrototypeId(int id) {
arity = 1;
s = "toJSON";
break;
case SymbolId_toPrimitive:
initPrototypeMethod(
DATE_TAG, id, SymbolKey.TO_PRIMITIVE, "[Symbol.toPrimitive]", 1);
return;
default:
throw new IllegalArgumentException(String.valueOf(id));
}
Expand Down Expand Up @@ -323,6 +327,25 @@ public Object execIdCall(
}
return result;
}
case SymbolId_toPrimitive:
{
Scriptable o = ScriptRuntime.toObject(cx, scope, thisObj);
final Object arg0 = args.length > 0 ? args[0] : Undefined.instance;
final String hint = (arg0 instanceof CharSequence) ? arg0.toString() : null;
Class<?> typeHint = null;
if ("string".equals(hint) || "default".equals(hint)) {
typeHint = ScriptRuntime.StringClass;
} else if ("number".equals(hint)) {
typeHint = ScriptRuntime.NumberClass;
}
if (typeHint == null) {
throw ScriptRuntime.typeErrorById(
"msg.invalid.toprimitive.hint", ScriptRuntime.toString(arg0));
}
;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but do you know why there is a semicolon here?


return ScriptableObject.getDefaultValue(o, typeHint);
}
}

// The rest of Date.prototype methods require thisObj to be Date
Expand Down Expand Up @@ -1374,21 +1397,19 @@ private static Object jsConstructor(Context cx, Object[] args) {

// if called with just one arg -
if (args.length == 1) {
Object arg0 = args[0];
if (arg0 instanceof NativeDate) {
obj.date = ((NativeDate) arg0).date;
final Object value = args[0];
if (value instanceof NativeDate) {
obj.date = ((NativeDate) value).date;
return obj;
}
if (arg0 instanceof Scriptable) {
arg0 = ((Scriptable) arg0).getDefaultValue(null);
}
double date;
if (arg0 instanceof CharSequence) {
final Object v = ScriptRuntime.toPrimitive(value);
final double date;
if (v instanceof CharSequence) {
// it's a string; parse it.
date = date_parseString(cx, arg0.toString());
date = date_parseString(cx, v.toString());
} else {
// if it's not a string, use it as a millisecond date
date = ScriptRuntime.toNumber(arg0);
date = ScriptRuntime.toNumber(v);
}
obj.date = TimeClip(date);
return obj;
Expand Down Expand Up @@ -1900,6 +1921,14 @@ protected int findPrototypeId(String s) {
return id;
}

@Override
protected int findPrototypeId(Symbol key) {
if (SymbolKey.TO_PRIMITIVE.equals(key)) {
return SymbolId_toPrimitive;
}
return 0;
}

private static final int ConstructorId_now = -3,
ConstructorId_parse = -2,
ConstructorId_UTC = -1,
Expand Down Expand Up @@ -1950,7 +1979,8 @@ protected int findPrototypeId(String s) {
Id_setYear = 45,
Id_toISOString = 46,
Id_toJSON = 47,
MAX_PROTOTYPE_ID = Id_toJSON;
SymbolId_toPrimitive = 48,
MAX_PROTOTYPE_ID = SymbolId_toPrimitive;

private static final int Id_toGMTString = Id_toUTCString; // Alias, see Ecma B.2.6

Expand Down
76 changes: 39 additions & 37 deletions rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3030,55 +3030,58 @@ public static boolean isObject(Object value) {
// implement the '~' operator inline in the caller
// as "~toInt32(val)"

public static Object add(Object val1, Object val2, Context cx) {
if (val1 instanceof BigInteger && val2 instanceof BigInteger) {
return ((BigInteger) val1).add((BigInteger) val2);
}
if ((val1 instanceof Number && val2 instanceof BigInteger)
|| (val1 instanceof BigInteger && val2 instanceof Number)) {
throw ScriptRuntime.typeErrorById("msg.cant.convert.to.number", "BigInt");
public static Object add(Object lval, Object rval, Context cx) {
// if lval and rval are primitive numerics of the same type, give them priority
if (lval instanceof Integer && rval instanceof Integer) {
return add((Integer) lval, (Integer) rval);
}
if (val1 instanceof Integer && val2 instanceof Integer) {
return add((Integer) val1, (Integer) val2);
if (lval instanceof BigInteger && rval instanceof BigInteger) {
return ((BigInteger) lval).add((BigInteger) rval);
}
if (val1 instanceof Number && val2 instanceof Number) {
return wrapNumber(((Number) val1).doubleValue() + ((Number) val2).doubleValue());
}
if (val1 instanceof CharSequence && val2 instanceof CharSequence) {
// If we let this happen later, then the "getDefaultValue" logic
// undoes many optimizations
return new ConsString((CharSequence) val1, (CharSequence) val2);
if (lval instanceof Number
&& !(lval instanceof BigInteger)
&& rval instanceof Number
&& !(rval instanceof BigInteger)) {
return wrapNumber(((Number) lval).doubleValue() + ((Number) rval).doubleValue());
}
if (val1 instanceof XMLObject) {
Object test = ((XMLObject) val1).addValues(cx, true, val2);

// e4x extension start
if (lval instanceof XMLObject) {
Object test = ((XMLObject) lval).addValues(cx, true, rval);
if (test != Scriptable.NOT_FOUND) {
return test;
}
}
if (val2 instanceof XMLObject) {
Object test = ((XMLObject) val2).addValues(cx, false, val1);
if (rval instanceof XMLObject) {
Object test = ((XMLObject) rval).addValues(cx, false, lval);
if (test != Scriptable.NOT_FOUND) {
return test;
}
}
if ((val1 instanceof Symbol) || (val2 instanceof Symbol)) {
throw typeErrorById("msg.not.a.number");
// e4x extension end

// spec starts here for abstract operation ApplyStringOrNumericBinaryOperator
// where opText is "+".
final Object lprim = toPrimitive(lval);
final Object rprim = toPrimitive(rval);
if (lprim instanceof CharSequence || rprim instanceof CharSequence) {
final CharSequence lstr =
(lprim instanceof CharSequence) ? (CharSequence) lprim : toString(lprim);
final CharSequence rstr =
(rprim instanceof CharSequence) ? (CharSequence) rprim : toString(rprim);
return new ConsString(lstr, rstr);
}
if (val1 instanceof Scriptable) val1 = ((Scriptable) val1).getDefaultValue(null);
if (val2 instanceof Scriptable) val2 = ((Scriptable) val2).getDefaultValue(null);
if (!(val1 instanceof CharSequence) && !(val2 instanceof CharSequence)) {
Number num1 = val1 instanceof Number ? (Number) val1 : toNumeric(val1);
Number num2 = val2 instanceof Number ? (Number) val2 : toNumeric(val2);

if (num1 instanceof BigInteger && num2 instanceof BigInteger) {
return ((BigInteger) num1).add((BigInteger) num2);
}
if (num1 instanceof BigInteger || num2 instanceof BigInteger) {
throw ScriptRuntime.typeErrorById("msg.cant.convert.to.number", "BigInt");
}
return num1.doubleValue() + num2.doubleValue();
// Skipping (lval = lprim, rval = rprim) and using xprim values directly.
final Number lnum = toNumeric(lprim);
final Number rnum = toNumeric(rprim);
if (lnum instanceof BigInteger && rnum instanceof BigInteger) {
return ((BigInteger) lnum).add((BigInteger) rnum);
}
if (lnum instanceof BigInteger || rnum instanceof BigInteger) {
throw ScriptRuntime.typeErrorById("msg.cant.convert.to.number", "BigInt");
}
return new ConsString(toCharSequence(val1), toCharSequence(val2));
return lnum.doubleValue() + rnum.doubleValue();
}

/**
Expand Down Expand Up @@ -3600,8 +3603,7 @@ public static Object toPrimitive(Object input, Class<?> preferredType) {
&& !Undefined.isUndefined(exoticToPrim)) {
throw notFunctionError(exoticToPrim);
}
final Class<?> defaultValueHint = preferredType == null ? NumberClass : preferredType;
final Object result = s.getDefaultValue(defaultValueHint);
final Object result = s.getDefaultValue(preferredType);
if ((result instanceof Scriptable) && !isSymbol(result))
throw typeErrorById("msg.bad.default.value");
return result;
Expand Down
3 changes: 3 additions & 0 deletions rhino/src/main/resources/org/mozilla/javascript/resources/Messages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,9 @@ msg.invalid.date =\
msg.toisostring.must.return.primitive =\
toISOString must return a primitive value, but instead returned "{0}"

msg.invalid.toprimitive.hint =\
[Symbol.toPrimitive]: expected "string", "number", or "default", but got "{0}"

# NativeJSON
msg.json.cant.serialize =\
Do not know how to serialize a {0}
Expand Down
35 changes: 3 additions & 32 deletions tests/testsrc/test262.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ built-ins/DataView 254/550 (46.18%)
toindex-bytelength-sab.js {unsupported: [SharedArrayBuffer]}
toindex-byteoffset-sab.js {unsupported: [SharedArrayBuffer]}

built-ins/Date 90/770 (11.69%)
built-ins/Date 68/770 (8.83%)
now/not-a-constructor.js {unsupported: [Reflect.construct]}
parse/not-a-constructor.js {unsupported: [Reflect.construct]}
parse/year-zero.js
Expand Down Expand Up @@ -697,18 +697,7 @@ built-ins/Date 90/770 (11.69%)
prototype/setUTCMinutes/not-a-constructor.js {unsupported: [Reflect.construct]}
prototype/setUTCMonth/not-a-constructor.js {unsupported: [Reflect.construct]}
prototype/setUTCSeconds/not-a-constructor.js {unsupported: [Reflect.construct]}
prototype/Symbol.toPrimitive/hint-default-first-invalid.js
prototype/Symbol.toPrimitive/hint-default-first-non-callable.js
prototype/Symbol.toPrimitive/hint-default-first-valid.js
prototype/Symbol.toPrimitive/hint-invalid.js
prototype/Symbol.toPrimitive/hint-number-first-invalid.js
prototype/Symbol.toPrimitive/hint-number-first-non-callable.js
prototype/Symbol.toPrimitive/hint-number-first-valid.js
prototype/Symbol.toPrimitive/hint-string-first-invalid.js
prototype/Symbol.toPrimitive/hint-string-first-non-callable.js
prototype/Symbol.toPrimitive/hint-string-first-valid.js
prototype/Symbol.toPrimitive/length.js
prototype/Symbol.toPrimitive/name.js
prototype/Symbol.toPrimitive/called-as-function.js
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we made a bunch of new tests pass, but we also made a test that previously passed fail, from what it looks like. Do you know why this appears to be failing now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have seen this alteady but there was no time for that. Will check

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

called-as-function.js seems to be a more general problem we have the same with the toJSON prototype.
This function passes in the past because a type error was expected (and there was no toPrimitiveSupport).

Will have another look at this and maybe open an issue for that, but in gerneral it is nothing introduced by this one.

prototype/Symbol.toPrimitive/prop-desc.js
prototype/Symbol.toPrimitive/this-val-non-obj.js
prototype/toDateString/not-a-constructor.js {unsupported: [Reflect.construct]}
Expand Down Expand Up @@ -738,17 +727,6 @@ built-ins/Date 90/770 (11.69%)
proto-from-ctor-realm-two.js {unsupported: [Reflect]}
proto-from-ctor-realm-zero.js {unsupported: [Reflect]}
subclassing.js {unsupported: [Reflect]}
value-get-symbol-to-prim-err.js
value-symbol-to-prim-err.js
value-symbol-to-prim-invocation.js
value-symbol-to-prim-return-obj.js
value-symbol-to-prim-return-prim.js
value-to-primitive-call.js
value-to-primitive-call-err.js
value-to-primitive-get-meth-err.js
value-to-primitive-result-faulty.js
value-to-primitive-result-non-string-prim.js
value-to-primitive-result-string.js
year-zero.js

built-ins/Error 6/41 (14.63%)
Expand Down Expand Up @@ -3862,15 +3840,8 @@ language/eval-code 253/347 (72.91%)

~language/export

language/expressions/addition 9/48 (18.75%)
language/expressions/addition 2/48 (4.17%)
bigint-errors.js
bigint-toprimitive.js
bigint-wrapped-values.js
coerce-symbol-to-prim-err.js
coerce-symbol-to-prim-invocation.js
coerce-symbol-to-prim-return-obj.js
coerce-symbol-to-prim-return-prim.js
get-symbol-to-prim-err.js
order-of-evaluation.js

language/expressions/array 41/52 (78.85%)
Expand Down
Loading