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

Function.prototype.toString for a built-in function with a name that is invalid as an identifier #4039

Open
sosukesuzuki opened this issue Apr 1, 2024 · 4 comments

Comments

@sosukesuzuki
Copy link
Contributor

The test /Function/prototype/toString/built-in-function-object.js checks the result of Function.prototype.toString for methods of Well-known Intrinsic Objects.

However, how should we handle the result of Function.prototype.toString for built-in functions with invalid names as identifiers (e.g., RegExp["$+"] that is not standardized yet)?

Many current implementations produce the following output:

Object.getOwnPropertyDescriptor(RegExp, "$+").get.toString();
// function get $+() { [native code] }

This behavior causes the test /Function/prototype/toString/built-in-function-object.js to fail because $+ is invalid as an identifier, failing the check in nativeFunctionMatcher.js.

Is the expected output something like function get ["$+"]() { [native code] } with the property name in parentheses and quotes?

But, upon checking the Function.prototype.toString section of ECMA-262, current output does not seem to violate the specification. Therefore, it may be necessary to update the test /Function/prototype/toString/built-in-function-object.js?

@ljharb
Copy link
Member

ljharb commented Apr 1, 2024

I would assume that it indeed outputs with the square brackets and quotes.

In my Safari, it outputs function $+() { as the first line; in Chrome, it outputs function get $+() {; in FF, it matches Safari.

The spec allows a lot of things that are unwise; I would hope that since the browsers don't agree, that it would be feasible (altho surely low priority) for them all to change to the reasonable thing?

@anba
Copy link
Contributor

anba commented Apr 1, 2024

FWIW: This issue is also filed at https://bugzilla.mozilla.org/show_bug.cgi?id=1658492.

@ptomato
Copy link
Contributor

ptomato commented Apr 1, 2024

However, how should we handle the result of Function.prototype.toString for built-in functions with invalid names as identifiers (e.g., RegExp["$+"] that is not standardized yet)?

It is not standardized yet, but it is part of https://github.com/tc39/proposal-regexp-legacy-features which is a stage 3 proposal. Therefore it's fair game to have tests in test262 for it.

By my reading, the unambiguously correct output according to the spec is function get $+() { [native code] } so I'd say that's what we should test for, but...

I'd suggest opening an issue in that proposal. In ECMA-262 as far as I know, there are no builtin functions that have a name that is a String but isn't a LiteralPropertyName, so step 4 of SetFunctionName has no need to emit a ComputedPropertyName when name is a String. With the introduction of Legacy RegExp Features, that assumption becomes incorrect (note that it might not be an intended assumption, though.) This seems like it's worth bringing a normative change to committee.

@ptomato
Copy link
Contributor

ptomato commented Apr 1, 2024

Actually, I see there already is such an issue: tc39/proposal-regexp-legacy-features#19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants