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

Editorial: Widen parameter type of CreateDynamicFunction #3380

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

tmdghks
Copy link
Contributor

@tmdghks tmdghks commented Jul 25, 2024

According to the current specification, type of the newTarget in CreateDynamicFunction needs to be a constructor. However, there is the case that newTarget is undefined as follows:

Function();

When this code executes, Function calls CreateDynamicFunction in third step as follows:

<emu-alg>
1. Let _C_ be the active function object.
1. If _bodyArg_ is not present, set _bodyArg_ to the empty String.
1. Return ? CreateDynamicFunction(_C_, NewTarget, ~normal~, _parameterArgs_, _bodyArg_).
</emu-alg>

In this situation, undefined is assigned to NewTarget . Therefore, type-mismatch occurs in the parameter newTarget of CreateDynamicFunction. Therefore, I think that the parameter type of the newTarget should be widened into a constructor or undefined.

@jmdyck
Copy link
Collaborator

jmdyck commented Jul 30, 2024

Looks correct to me.

@tmdghks
Copy link
Contributor Author

tmdghks commented Jul 30, 2024

Thank you very much for your review!

@bakkot bakkot added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Sep 2, 2024
@ljharb ljharb force-pushed the fix-createdynamicfunction-newtarget-type branch from 104d49c to 62bb5c5 Compare September 3, 2024 03:33
@ljharb ljharb merged commit 62bb5c5 into tc39:main Sep 3, 2024
8 checks passed
@tmdghks tmdghks deleted the fix-createdynamicfunction-newtarget-type branch September 4, 2024 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change 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.

5 participants