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

Support ES2019 Function.toString() revision #1300

Open
p-bakker opened this issue Feb 3, 2023 · 9 comments · Fixed by #1520
Open

Support ES2019 Function.toString() revision #1300

p-bakker opened this issue Feb 3, 2023 · 9 comments · Fixed by #1520
Labels
Ecma Incompatibility Issues about Rhino being incompatible with the EcmaScript spec feature Issues considered a new feature
Milestone

Comments

@p-bakker
Copy link
Collaborator

p-bakker commented Feb 3, 2023

See:

Some examples:
Currently:

=> Math.pow
function pow() { [native code for Math.pow, arity=2] }

Expected:

=> Math.pow.toString()
function pow() { [native code] }

Currently:

=> (function(){}).bind({}).toString()
function () {
	[native code, arity=0]
}

Expected:

=> (function(){}).bind({}).toString()
function () { [native code] }

The spec revision also details that the .toString() of user-defined function inside JavaScript must return the source exactly as authored, which I'm not sure we're compliant with

@p-bakker p-bakker added the Ecma Incompatibility Issues about Rhino being incompatible with the EcmaScript spec label Feb 3, 2023
@p-bakker p-bakker added this to the ES2019 milestone Feb 3, 2023
@p-bakker
Copy link
Collaborator Author

p-bakker commented Feb 8, 2023

The part about .toString() returning the source code as authored might be fixed by #1188

@tuchida
Copy link
Contributor

tuchida commented Jul 15, 2024

It would be possible to handle this by removing the code that adds arity= to the result of Function.prototype.toString.
However, since #939(#326) is not implemented, the harness for test262 does not work.

// nativeFunctionMatcher.js
  const eatWhitespace = () => {
    while (pos < source.length) {
      const c = source[pos]; // `c` does not change. There are three similar codes.
      if (isWhitespace(c) || isNewline(c)) {
        pos += 1;
        continue;
      }

This works by replacing const with let.

Besides native code, getter/setter test was failing.

@p-bakker
Copy link
Collaborator Author

But besides the tests for this in test262 failing, code in Rhino needs to be fixed still to make it functionally work?

@tuchida
Copy link
Contributor

tuchida commented Jul 15, 2024

As for arity=, the following codes needs to be removed.

sb.append("[native code, arity=");
sb.append(getArity());
sb.append("]\n");

sb.append(", arity=");
sb.append(getArity());
sb.append(justbody ? "]\n" : "] }\n");

I have not looked too closely at the getter/setter.

@tuchida
Copy link
Contributor

tuchida commented Jul 17, 2024

This issue is not yet fixed.

@p-bakker p-bakker reopened this Jul 18, 2024
@p-bakker
Copy link
Collaborator Author

To clarify: #1520 fixed the .toString() behaviour on user-authored JavaScript functions, but not yet on native (Java) methods

@tuchida
Copy link
Contributor

tuchida commented Jul 22, 2024

What should the Java method be?

https://262.ecma-international.org/15.0/index.html#sec-function.prototype.tostring

  1. If func is an Object and IsCallable(func) is true, return an implementation-defined String source code representation of func. The representation must have the syntax of a NativeFunction.

I think this sentence expects [native code].
Now Rhino returns a string like this.

js> java.math.BigInteger.valueOf.toString()
function valueOf() {/*
java.math.BigInteger valueOf(long)
*/}

In GraalJS, this is what returned.

> Java.type('java.math.BigInteger').valueOf.toString()
function valueOf() { [native code] }

@p-bakker
Copy link
Collaborator Author

[native code] is correct afaik

@rbri
Copy link
Collaborator

rbri commented Jul 23, 2024

switching back to only [native code] will be great - have done some adjustments for the change in HtmlUnit to be browser compatible (HtmlUnit/htmlunit@0c18c77)

Hopefully i can remove more code and only insert the line break that ff has in the output...

@p-bakker p-bakker added the feature Issues considered a new feature label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ecma Incompatibility Issues about Rhino being incompatible with the EcmaScript spec feature Issues considered a new feature
Projects
None yet
3 participants