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

NativeFunction vs web reality #1941

Closed
bathos opened this issue Apr 10, 2020 · 12 comments · Fixed by #1948
Closed

NativeFunction vs web reality #1941

bathos opened this issue Apr 10, 2020 · 12 comments · Fixed by #1948

Comments

@bathos
Copy link
Contributor

bathos commented Apr 10, 2020

Testing with Map.prototype.__lookupGetter__('size').toString():

Engine Result
JSC function get size() {\n [native code]\n}
Spidermonkey function size() {\n [native code]\n}
V8 function get size() { [native code] }

The JSC and V8 results don’t conform to NativeFunction. The part between "function" and "(" is PropertyName[opt], which would allow for a computed name like e.g. ["get size"], but not the two bare identifiers get size. SpiderMonkey does seem to conform by omitting the get.

I’m not sure if this is an implementation issue for those engines or if the NativeFunction grammar itself is overconstrained. I believe (though I’m not certain) that the main objective of the NativeFunction requirement is to ensure the reported string cannot be successfully evaluated, and that property is maintained by the JSC and V8 results.

Results are similar for non-intrinsic native functions in platform APIs.

@michaelficarra
Copy link
Member

Looks like we need a test262 test that covers this, since V8 supposedly passes all of the F.p.toString tests: https://test262.report/browse/built-ins/Function/prototype/toString

  • ChakraCore: 25%
  • JavaScriptCore: 19%
  • SpiderMonkey: 95%
  • V8: 100%
  • Moddable XS: 98%
  • Hermes: 16%
  • QuickJS: 100%
  • engine262: 81%

I'd be interested to know if any engines have chosen to wilfully violate the spec here or if this is just an implementation oversight. I assume that it's the latter because this portion of the spec was refined not long ago.

@devsnek
Copy link
Member

devsnek commented Apr 14, 2020

The spec is just kind of nonsensical in this case (emphasis mine):

19.2.3.5 Function.prototype.toString ( )

  1. ...
  2. If func is a bound function exotic object or a built-in function object, then return an implementation-dependent String source code representation of func. The representation must have the syntax of a NativeFunction. Additionally, if func is a Well-known Intrinsic Object and is not identified as an anonymous function, the portion of the returned String that would be matched by PropertyName must be the initial value of the "name" property of func.

PropertyName cannot ever match get x or set x (you could alternatively read this as a string replacement, where it doesn't matter if it matches the production or not, but even if that's the case we should make it way clearer and clarify the proceeding sentence). I don't know what the intention of the authors was, but personally I'd like to require the get and set prefixes in toString.

@bathos
Copy link
Contributor Author

bathos commented Apr 14, 2020

Good catch. There’s no way for an implementation to adhere to both requirements, then, so this at least partly a spec issue.

(Would add that PropertyName does match "get x" if the quotation marks are literally present. Context ended up making it clear that that was meant to be read as its string value, but since I was confused initially, figured I would clarify that for anybody else who scans it wrong on the first pass.)

Related: is there any concept of a canonical representation of a property name? E.g. — pseudo algorithm — “if it could be the valid string value of IdentifierName or is a canonical numeric string, return it as is; otherwise if it is a string, return JSON.stringify(name); otherwise [do something with symbol]...”

@devsnek
Copy link
Member

devsnek commented Apr 14, 2020

I think the canonical thing is PropName, which returns empty for PropertyName : ComputedPropertyName and a representative string value for PropertyName : LiteralPropertyName. Note that because it specializes on PropertyName, it ignores the get and set prefixes.

@michaelficarra
Copy link
Member

@devsnek

The spec is just kind of nonsensical in this case (emphasis mine):

No it's not. Pay attention to the first part of that sentence.

Additionally, if func is a Well-known Intrinsic Object and is not identified as an anonymous function,

All Well-Known Intrinsic Objects that are functions have a "name" data property with an initial value that either matches PropertyName or is the empty string.

I don't know what the intention of the authors was

I am the author. The intention was that the synthesised string would be matched by NativeFunction.

@bathos
Copy link
Contributor Author

bathos commented Apr 14, 2020

@michaelficarra Are you saying that this works because no WKIOs are non-anonymous accessors (whose initial name values contain a space)?

(If so, it seems a bit odd if this implies a hidden(?) requirement that e.g. %Map.prototype.size% must not become “well-known” on account of having a name that contains a space.)

@michaelficarra
Copy link
Member

That is correct.

@devsnek
Copy link
Member

devsnek commented Apr 14, 2020

@michaelficarra interesting. is that actually meant to be well known intrinsics and not all intrinsics? being a well known intrinsic is just whether or not we have a internal name for something, it's not detectable or anything.

@ljharb
Copy link
Member

ljharb commented Apr 14, 2020

I’m surprised to hear that relationship; anything is eligible to be a well-known intrinsic, including getters and setters.

@michaelficarra
Copy link
Member

See tc39/Function-prototype-toString-revision#29 for the thread about built-in getters and tc39/Function-prototype-toString-revision#10 about intrinsics.

We can always make the requirement stronger by expanding it to other intrinsics. You're free to try to take on that work with a follow-on proposal.

@ExE-Boss
Copy link
Contributor

I’d personally prefer if we could get away with returning the get or set keyword in place of the function keyword for getters and setters.

@gibson042
Copy link
Contributor

There appears to be a majority desire on the committee for native accessors to be represented similarly to author-defined accessors, which manifest differently based on how they are defined:

  1. get PropertyName () { FunctionBody } in an object initializer evaluates with name matching "get " + PropertyName and [[SourceText]] containing the matched source text starting with "get" per 14.3 Method Definitions.
  2. get () { FunctionBody } in a property descriptor evaluates with name "get" and [[SourceText]] containing the matched source text starting with "get" per the same.
  3. get: function ( FormalParameters ) { FunctionBody } in a property descriptor evaluates with name "get" and [[SourceText]] containing the function expression starting with "function" per 12.2.6.8 Runtime Semantics: PropertyDefinitionEvaluation.

Given that built-in accessors have "get " or "set " prepended to their name per 17 ECMAScript Standard Built-in Objects, it seems like option 1 is the best fit and Map.prototype.__lookupGetter__('size').toString() should return something like "get size() { [native code] }" (without a "function" substring).

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

Successfully merging a pull request may close this issue.

6 participants