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

Normative: Define default constructors using spec steps #2216

Merged
merged 1 commit into from
Feb 28, 2021

Conversation

devsnek
Copy link
Member

@devsnek devsnek commented Oct 27, 2020

Fixes: #2212

@devsnek devsnek added needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Oct 27, 2020
@ljharb ljharb added the normative change Affects behavior required to correctly evaluate some ECMAScript source text label Oct 27, 2020
spec.html Outdated Show resolved Hide resolved
@devsnek

This comment has been minimized.

@devsnek devsnek force-pushed the default-constructor-refactor branch from a199c87 to beee2f7 Compare October 27, 2020 19:41
devsnek pushed a commit to devsnek/test262 that referenced this pull request Oct 27, 2020
@devsnek
Copy link
Member Author

devsnek commented Oct 27, 2020

Tests: tc39/test262#2878

spec.html Outdated Show resolved Hide resolved
devsnek added a commit to devsnek/test262 that referenced this pull request Oct 27, 2020
@ljharb ljharb added has consensus This has committee consensus. and removed needs consensus This needs committee consensus before it can be eligible to be merged. labels Nov 17, 2020
@devsnek devsnek force-pushed the default-constructor-refactor branch 3 times, most recently from 3e64e8a to 81f24c9 Compare November 19, 2020 02:44
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
@devsnek devsnek force-pushed the default-constructor-refactor branch 2 times, most recently from a99f1b2 to 6a3ac96 Compare November 23, 2020 17:50
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
@rwaldron rwaldron added has test262 tests and removed needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Nov 25, 2020
spec.html Outdated Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
rwaldron pushed a commit to tc39/test262 that referenced this pull request Feb 26, 2021
@ljharb ljharb added has test262 tests and removed needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Feb 26, 2021
@ljharb ljharb added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Feb 28, 2021
@akamsmrhill

This comment has been minimized.

@ljharb ljharb force-pushed the default-constructor-refactor branch from 7768b89 to d9e5692 Compare February 28, 2021 20:35
@ljharb ljharb merged commit d9e5692 into tc39:master Feb 28, 2021
@devsnek devsnek deleted the default-constructor-refactor branch February 28, 2021 21:22
pull bot pushed a commit to Mu-L/v8 that referenced this pull request Jul 29, 2021
Each time a constructor is being called without new operator, a TypeError
is thrown. The TypeError should be the realm's one according to 10.2.1.5.b.

Refs: https://tc39.es/ecma262/#sec-ecmascript-function-objects-call-thisargument-argumentslist
Refs: tc39/ecma262#2216
Bug: v8:11530
Change-Id: Iff10a78e96fb547fe2062c86b9f93a30d2a8be20
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3056830
Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76002}
jugglinmike added a commit to tc39/test262 that referenced this pull request Sep 3, 2021
Normative: Define default constructors using spec steps
tc39/ecma262#2216
jugglinmike added a commit to tc39/test262 that referenced this pull request Sep 3, 2021
Normative: Define default constructors using spec steps
tc39/ecma262#2216

See also #2978
jugglinmike added a commit to tc39/test262 that referenced this pull request Sep 3, 2021
Normative: Define default constructors using spec steps
tc39/ecma262#2216

Resolves #2978
rwaldron pushed a commit to tc39/test262 that referenced this pull request Sep 7, 2021
Normative: Define default constructors using spec steps
tc39/ecma262#2216

Resolves #2978
@@ -25192,9 +25210,9 @@ <h1>Function.prototype.toString ( )</h1>
<p>When the `toString` method is called, the following steps are taken:</p>
<emu-alg>
1. Let _func_ be the *this* value.
1. If _func_ is a <emu-xref href="#sec-built-in-function-objects">built-in function object</emu-xref>, return an implementation-defined String source code representation of _func_. The representation must have the syntax of a |NativeFunction|. Additionally, if _func_ has an [[InitialName]] internal slot and _func_.[[InitialName]] is a String, the portion of the returned String that would be matched by |NativeFunctionAccessor?| |PropertyName| must be the value of _func_.[[InitialName]].
Copy link

@bergus bergus Jan 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Coming from #2608 (comment))

I see why this change was necessary: default constructors that are created using CreateBuiltinFunction now are "built-in functions" and have an [[InitialName]] slot; however they should still return the [[SourceText]] from the user's program.

But the new spec text does permit implementations to return the [[SourceText]] for basically arbitrary built-in functions now, especially for those that are implemented as ECMAScript function objects. I do not think that change was intentional, I believe that all functions which are not defined by user code (but rather, by the implementation) should have to return a string in NativeFunction format. At least this was listed as a goal in the Function.prototype.toString revision proposal.

Would it be sensible to rollback this swapping of Function.prototype.toString steps, and instead change CreateBuiltinFunction to not create an [[InitialName]] slot for default constructors?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would implementations be forced to omit source text? An engine should always be free to provide meaningful source text in .toString if it so chooses.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's the (not unreasonable) general expectation that native functions can be detected by looking for [native code] in the toString representation, and this is used in many codebases. See https://davidwalsh.name/detect-native-function or https://stackoverflow.com/questions/6598945/detect-if-function-is-native-to-browser for reference.
The Function.prototype.toString revision proposal did mean to "standardise the string representation of built-in functions and host objects". There has gone a lot of thought into this, and there were requests to also ensure proper function name representations and even exact whitespace patterns in the NativeFunction syntax.

If you want to change this, fine, but it seems this happened without discussion in TC39, likely even by accident.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is an unreasonable expectation, since (function () {}).bind() produces the same toString representation.

To my recollection, it was very much not an accident to permit native functions to have any source text they want, as long as it's either valid ecmascript, or, the native code representation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus. has test262 tests normative change Affects behavior required to correctly evaluate some ECMAScript source text ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop delegating to Array.prototype[Symbol.iterator] in default class constructor
9 participants