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

ES5-Test262 progress #39

Draft
wants to merge 111 commits into
base: master
Choose a base branch
from
Draft

ES5-Test262 progress #39

wants to merge 111 commits into from

Conversation

anba
Copy link
Contributor

@anba anba commented Mar 21, 2012

This is a bit of my work towards ECMAScript 5 compliance. For now I've mainly focused on test262 to find out what is currently missing for ES5 conformance in Rhino, so this is by far not a complete. (Out of the full test262 test suite, only seven test cases still fail. Yeah!)

As the change set itself actually has grown quite big, I don't think it's possible (nor reasonable) to just apply it as a whole (at least not into the master branch). And I deliberately did not squash all commits into a single one in order to reveal somewhat the thought process behind the changes.

I'm open for suggestions how to get this into Rhino in a sane way.

Cheers,
André

anba added 30 commits March 21, 2012 02:41
TokenStream:
- remove check for isJSFormatChar() in getChar[IgnoreLineEnd](), no longer neccessary in ES5
- amend readRegExp() to catch some additional invalid regular expression literals
- update keyword list in stringToKeyword() to match ES5
- getToken():
-- unicode escaped identifiers still need to comply to identifier naming rules
-- save string for Token.RESERVED (needed in Parser)
-- hex-integer needs at least one hex-digit
-- invalid escape sequences in string literals now result in an error

Parser:
- remove unused second argument of propertyName() to facilitate code reuse
- no longer check for isReservedKeywordAsIdentifier() in convertToName() since ES5 allows identifier names in object literals and property access
- handle Token.RESERVED in convertToName()
- prevent changing [[Prototype]] on non-extensible objects, cf. ch08/8.6/8.6.2/S8.6.2_A8.js
Scriptable:
- hasInstance(): change argument from Scriptable to Object to comply with specification, which defines [[HasInstance]] as SpecOp(any) -> Boolean
- this is a public API change!
- update the following classes to conform to the new interface:
-- Matrix, BaseFunction, BoundFunction, Delegator, NativeIterator, NativeJavaArray, NativeJavaClass, NativeJavaObject, NativeWith, XMLCtor, XMLObjectImpl

ScriptableObject:
- default implementation of hasInstance() now throws a 'TypeError' to conform to the spec

ScriptRuntime:
- move primitive check from instanceOf() to jsDelegatesTo()
- add updated version of the "Abstract Relational Comparison Algorithm" with flags to control evaluation order
- deprecate old cmp_LT() and cmp_LE() methods

Interpreter:
- ensure proper left-to-right evaluation order for SUB, MUL, DIV, MOD
- ensure proper evaluation order for GE, LE, GT, LT

Parser:
- track property-type in addition to property-name to detect duplicate properties (code adapted from SpiderMonkey parser)
- always warn for duplicate getter/setter independent of strict-mode (SpiderMonkey behaviour)
- add check for parameter count in case of getter/setter functions
Update generated code to use ScriptRuntime#cmp_LT(Object, Object, boolean, boolean)
ECMAScript5 has changed "15.3.4.4  Function.prototype.call" to longer coerce the `thisObject` by means of ToObject.
That requires a few API changes in Rhino, notably for Callable#call() and IdFunctionCall#execIdCall(). As both
methods are fundamental aspects of any host object in Rhino, the API change triggered changes all over the place.
In the rest of this commit message, file changes are not separately listed below if the changes are only updates
for either call() or execIdCall().

Callable:
- changed `thisObject` type from "Scriptable" to "Object" to conform to ES5
IdFunctionCall:
- changed `thisObject` type from "Scriptable" to "Object" to conform to ES5

xmlbeans/XMLList:
- add cast to XMLObject [short-cut instead of calling ScriptRuntime.toObject()]

Arguments:
- add initial default data-descriptor with {enumerable: true, writable: true, configurable: true} per 10.6 Arguments Object, step 11.b
- required at this point otherwise ScriptableObject#defineOwnProperty() sets the attributes to {enumerable: false, writable: false, configurable: false}
- handle case when only 'writable' is set
- replace isFalse() call with additional check whether 'writable' is actually present in descriptor
- also see [[DefineOwnProperty]] for Arguments step 5.b.i and 5.b.ii

BaseFunction:
- arity of toString() is 1
- realFunction() should default to use `thisObject` as function if valueOf() returns non-function object [valueOf() redefining the function seems to a rhino-specific implementation choice?]
- left backward compatible call() method definition to help users move to new call() interface

IdScriptableObject:
- left backward compatible execIdCall() method definition to help users move to new execIdCall() interface

InterprectedFunction:
- user functions still coerce 'null' and 'undefined' to the global object

Codegen:
- update generated code to match InterprectedFunction

NativeArray:
- [[Put]] cannot add new entries if [[Extensible]] is false, cf. 8.12.4 [[CanPut]]
- added 15.4.5.1 [[DefineOwnProperty]] for the 'length' property on Array
- added 15.4.5.1 [[DefineOwnProperty]] for the index properties on Array
- intercept 'length' and index properties and redirect to the methods listed above
- 'length' must be intercepted for both defineOwnProperty() methods, see defineOwnProperty() in super-class
- copied more or less from setLength(), but includes additional checks and always deletes index properties in descending order as specified in 15.4.5.1 [[DefineOwnProperty]]
- helper method just like setElem(), but uses [[DefineOwnProperty]] instead of [[Put]]
- follow spec for 15.4.4.11  Array.prototype.sort more closely in respect to undefined and not-defined properties
- explicitely call deleteElem() because `thisObject` may not be an Array instance
- always call setLengthProperty() to ensure update to 'length' property b/c it may be implicitely updated due to call to ToUint32 in step 3 of 15.4.4.13  Array.prototype.unshift
- add missing calls to [[Delete]], see step 12.d of 15.4.4.12  Array.prototype.splice
- 15.4.4.4  Array.prototype.concat uses [[DefineOwnProperty]] instead of [[Put]]
- handle case when first argument is `undefined`
- 15.4.4.10  Array.prototype.slice uses [[DefineOwnProperty]] instead of [[Put]]
- removed method, indexOf and lastIndexOf now use seperate methods since the amount of shared lines was negliglible to justify the added non-uniformness compared to the other js_* methods
- 15.4.4.14  Array.prototype.indexOf uses [[Get]], therefore add search for prototype values in the dense-array case
- 15.4.4.15  Array.prototype.lastIndexOf uses [[Get]], therefore add search for prototype values in the dense-array case
- use caution to follow algorithm steps more closely
- 'filter' and 'map' use [[DefineOwnProperty]] instead of [[Put]]
- use caution to follow algorithm steps more closely

NativeDate:
- length of 'constructor' and 'UTC' is 7
- handle case when Date.prototype.toISOString is redefined for Date objects
- Date.prototype.toJSON uses [[GET]] to obtain the "toISOString" property
- don't use Java's DateFormat to parse/format dates from/into ISO-8601 Extended Format strings since DateFormat doesn't follow the syntax from 15.9.1.15  Date Time String Format
- add DaysInMonth method with same contract as DaysInMonth from jsdate.cpp
- add parseISOString method with same contract as in jsdate.cpp, but use a simple state machine since Java doesn't support macros
- parseISOString supports the following two deviations from the spec:
  1) time-only format, e.g. "08:00:00Z"
  2) timezone descriptor without ':' as separator, e.g. "08:00:00+0100" instead of "08:00:00+01:00"
- parseISOString also supports the same input validation as in jsdata.cpp, the spec isn't really clear what to do with invalid input data
- add js_toISOString method to format date values in ISO-8601 Extended Format with expanded year representation if necessary

NativeError:
- properly set default attributes for "name" and "message"
- handle `undefined` value for message in constructor
- follow spec for 15.11.4.4  Error.prototype.toString more closely

NativeGlobal:
- remove IE-support shim for "ConversionError"
- no longer replace 0xFFFE and 0xFFFF with 0xFFFD when decoding URI's

NativeNumber:
- stick with the spec and don't allow non-standard precisions

NativeObject:
- Object.prototype.toLocaleString no longer alias for toString instead follow spec and retrieve current "toString" with [[Get]]
- Object.prototype.valueOf calls ToObject on `thisObject` per spec
- handle `undefined` arguments for Object.prototype.hasOwnProperty and Object.prototype.propertyIsEnumerable
- Object.prototype.isPrototypeOf calls ToObject after argument validation per spec algorithm

NativeString:
- arity of String.prototype.replace is 2
- call CheckObjectCoercible on `thisObject` if required by spec for String.prototype methods
- add more overriden methods to be compliant to 15.5.5.2 [[GetOwnProperty]]
- handle `undefined` arguments in 15.5.4.13  String.prototype.slice

ScriptRuntime:
- add additional defaultObjectToString() method to support 15.2.4.2  Object.prototype.toString
- #numberToString(): first check radix, then process other special values like Infinity etc.
- add #checkObjectCoercible() per 9.10 CheckObjectCoercible
-- don't map `null` or `undefined` to the global object if specified as `thisObject`, cf. 15.3.4.4  Function.prototype.call
-- don't call ToObject for the `thisObject`, cf. 15.3.4.4  Function.prototype.call
- use [[DefineOwnProperty]] rather than [[Put]] in #newArrayLiteral() when creating new Array objects

ScriptableObject:
- add helper methods for defineProperty() which work on integer indices instead of strings
- default getter resp. setter to `undefined` when creating a new accessor property
- 'enumerable' must be present before attempting to reject property change in #checkPropertyChange()

SpecialRef:
- handle the case when an object actually has an own property named "__proto__"

NativeRegExp:
- RegExp is no longer callable, also see https://bugzilla.mozilla.org/show_bug.cgi?id=582717
- change 'lastIndex' property from double to Object to allow lazy evaluation of the property
- handle `undefined` argument when compiling a new RegExp
- make RegExp.prototype.exec work with `undefined` argument
- add lazy evaluation of 'lastIndex' in RegExp.prototype.exec
- additional check to report an error if a RegExp flag appears more than once
- don't allow unqualified '{', cf. `/a{1}{1}/`
- arity of RegExp.prototype.compile is 2

NativeRegExpCtor:
- arity of RegExp constructor is 2

RegExpImpl:
- String.prototype.replace erroneously created a RegExp if the first argument was not already an instanceof RegExp
- move NativeRegExp creation out of matchOrReplace into a new method
- handle `undefined` arguments for String.prototype.{match, search, replace}
- follow String.prototype.split algorithm steps more closely

error.tostring.doctest:
- update expected values, double-checked with SM shell
…or accessor shenanigans when creating property descriptor
@hns
Copy link
Contributor

hns commented Mar 21, 2012

Thanks André! As I said on buzilla, this is one massive contribution!

The next step would probably be to filter out and group commits by their value/risk ratio and decide what (if any) should go into the next 1.7 release and what belongs in a later 1.8 release.

I notice quite a bit of breakage in existing Rhino and Mozilla test suites, so we'll have to update those tests and/or filter out the tests that no longer apply under ES5.

…LL_EVAL

Parser: remove merger marker, d'oh...!
NativeRegExpCtor: implement setInstanceIdAttributes() so that `Object.freeze(RegExp)` works again
arguments.doctest: update doctest to assert correct behaviour (double checked with current spidermonkey)
object.defineproperty.doctest: update doctest to assert correct behaviour
object.getownpropertydescriptor.doctest: update doctest to assert correct behaviour
@anba
Copy link
Contributor Author

anba commented Mar 21, 2012

I've pushed a new change to anba/rhino@638d09d, this should help to resolve some of the test failures you've seen. Concerning the MozillaTestSuite, most of the failures seem to be related to different timezone settings resp. to https://bugzilla.mozilla.org/show_bug.cgi?id=731380.

IIRC public API changes should be avoided for 1.7 if possible, right? If so, most of the changes need to be deferred for 1.8 since they depend on the updated Scriptable and Callable interfaces.

@hns
Copy link
Contributor

hns commented May 4, 2012

Right, public API should not change in 1.7R3 (unless it's a "marginal" API and for very good reason).

If you can think of any individual patches that don't rely on public API changes and fix real bugs please add a note here.

@anba
Copy link
Contributor Author

anba commented May 29, 2012

I've added #45 and #46, both cover most of the changes which don't rely on any API updates.

Copy link
Collaborator

@rbri rbri left a comment

Choose a reason for hiding this comment

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

made a separate PR (#694) based on this

Copy link
Collaborator

@rbri rbri left a comment

Choose a reason for hiding this comment

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

As of today this is in the code (2020-06-01)

Copy link
Collaborator

@rbri rbri left a comment

Choose a reason for hiding this comment

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

As of today this is in the code (2020-06-01)

@p-bakker
Copy link
Collaborator

p-bakker commented Sep 5, 2020

Is there any plan to merge the remainder of this PR, the part with the breaking changes to the Scriptable and Callable interfaces?

At least I assume they are still needed to progress in ES Latest compliance?

@p-bakker p-bakker added the Strict Mode Issues related to non-compliance with the ES5 Strict Mode spec label Nov 29, 2021
@p-bakker p-bakker marked this pull request as draft September 8, 2024 06:56
@p-bakker
Copy link
Collaborator

p-bakker commented Sep 8, 2024

Converted to draft as this one is very outdated. Just not choosing it because there's some stuff in here that is still somewhat useful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Strict Mode Issues related to non-compliance with the ES5 Strict Mode spec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants