-
Notifications
You must be signed in to change notification settings - Fork 850
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
Use to primitive for plus operation and date (taken from #1611 done by @tonygermano) #1685
Use to primitive for plus operation and date (taken from #1611 done by @tonygermano) #1685
Conversation
From my point of view this is ready to be squash-merged... what do you think? |
throw ScriptRuntime.typeErrorById( | ||
"msg.invalid.toprimitive.hint", ScriptRuntime.toString(arg0)); | ||
} | ||
; |
There was a problem hiding this comment.
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?
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Yes, it looks good, but I have two minor points -- can you check them out? Thanks! |
OK -- thanks for looking at the test and removing that huge problem with the semicolon ;-) |
This is another carve out from #1661.
Please squash this on merge.