feat(ecmascript): Implement Annex-B string prototype methods #544
Conversation
I also added some feature flags for including parts of or the whole of annex-b. Made them on-by-default, but might make more sense for them to be off?
eliassjogreen
force-pushed
the
feat/ecmascript-annex-b-string-methods
branch
from
f22b3ff to
61ce8a8
Compare
|
There are no expectations updated because we have turned off Annex B in the test runner, I ran it locally and it passed the string expectations, but I don't suppose we want to turn it on for the whole engine. |
There was a problem hiding this comment.
Looks mostly good to me, but let's change the function identities to match what the spec expects since it should be fairly easy to do.
We could also add the feature flag to the tests build and (eventually at least) turn on Annex B testing, but indeed let's not turn it on by default for the cli build. Don't want that whole mess.
We could also make PR to https://github.com/test262-fyi/test262.fyi/blob/main/scripts/engines/nova.sh to run with the Annex B feature flags.
| #[cfg(feature = "annex-b-string")] | ||
| struct StringPrototypeTrimLeft; | ||
| #[cfg(feature = "annex-b-string")] | ||
| impl Builtin for StringPrototypeTrimLeft { |
There was a problem hiding this comment.
nitpick (blocking, but not very strongly): The spec's terminology for TrimLeft and TrimRight is
The initial value of the "trimLeft" property is %String.prototype.trimStart%, defined in 22.1.3.34.
ie. it's not just another function that does the same thing but it's actually the same function object. You should impl BuiltinIntrinsic for StringPrototypeTrimStart and make StringPrototypeTrimLeft a reference to that. See f.ex. how ArrayPrototypeValues and how we use with_property to generate array.prototype[Symbol.Iterator].
| /// ECMAScript language value) and returns either a normal completion | ||
| /// containing a String or a throw completion. | ||
| #[cfg(feature = "annex-b-string")] | ||
| fn create_html<'gc>( |
I also added some feature flags for including parts of or the whole of annex-b. Made them on-by-default, but might make more sense for them to be off?