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: ProxyTarget is always a function object in GetFunctionRealm #3496

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

kimjg1119
Copy link
Contributor

In GetFunctionRealm, step 3.c. call GetFunctionRealm recursively with proxyTarget which assumes that proxyTarget is a function object.

If my investigation is correct, proxy exotic objects are generated in ProxyCreate and it is callable only if the target is callable. Thus if a function object has a target, then it is also a function object thus it isn't a spec bug. However I think add an assertion here will be helpful.

Copy link
Member

@michaelficarra michaelficarra left a comment

Choose a reason for hiding this comment

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

This is great!

@kimjg1119
Copy link
Contributor Author

Can we also remove step 3.a? It seems that ValidateNonRevokedProxy only checks ProxyTarget, but it seems that it is always Object when obj is callable.

@michaelficarra
Copy link
Member

@kimjg1119 All proxies of callable objects (including revocable proxies) are themselves callable. When a callable revocable proxy is revoked, it remains callable. So ValidateNonRevokedProxy is needed to check that the proxy has not yet been revoked (and that [[ProxyTarget]] is a function object rather than null).

@bakkot bakkot added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Dec 18, 2024
ljharb pushed a commit to kimjg1119/ecma262 that referenced this pull request Dec 19, 2024
@ljharb ljharb force-pushed the get-function-realm branch from 04cca4e to 9d8ea1a Compare December 19, 2024 01:27
@ljharb ljharb force-pushed the get-function-realm branch from 9d8ea1a to c404297 Compare December 19, 2024 06:36
@ljharb ljharb merged commit c404297 into tc39:main Dec 19, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants