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

Make NativeGlobal not an IdFunctionCall #1827

Merged
merged 2 commits into from
Feb 18, 2025

Conversation

andreabergia
Copy link
Contributor

@andreabergia andreabergia commented Feb 12, 2025

This makes NativeGlobal not implement IdFunctionCall and turns all the method into LambdaFunction or LambdaConstructor.

There's a few complicated details for handling the built-in errors, but it all seems to work and fix a few small test262 cases.

@andreabergia andreabergia changed the title WIP: Make NativeGlobal not an IdFunctionCall - global functions WIP: Make NativeGlobal not an IdFunctionCall Feb 12, 2025
@andreabergia andreabergia force-pushed the global-as-lambda-constructor branch from 8a18114 to 1cd8e55 Compare February 12, 2025 16:57
@gbrail
Copy link
Collaborator

gbrail commented Feb 12, 2025 via email

@andreabergia andreabergia force-pushed the global-as-lambda-constructor branch from 1cd8e55 to 5d73221 Compare February 13, 2025 11:54
@andreabergia andreabergia changed the title WIP: Make NativeGlobal not an IdFunctionCall Make NativeGlobal not an IdFunctionCall Feb 13, 2025
@andreabergia andreabergia marked this pull request as ready for review February 13, 2025 13:43
private static Boolean js_isXMLName(Context cx, Scriptable scope, Object[] args) {
Object name = (args.length == 0) ? Undefined.instance : args[0];
XMLLib xmlLib = XMLLib.extractFromScope(scope);
return ScriptRuntime.wrapBoolean(xmlLib.isXMLName(cx, name));
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is minor, but I'm noticing that we don't really need "wrapBoolean" or even "wrapInt" any more, since auto-boxing just does that. If you agree we could take it out of a few places and be even cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing! I didn't really look at those methods honestly, just refactored them out of the IdScriptableObject's switch as they were. 🙂

@gbrail
Copy link
Collaborator

gbrail commented Feb 16, 2025

This looks great, and your "style" of doing this is nicer than what I was doing so I may make some changes.

I have one minor nit as to whether we actually need "wrapBoolean" any more. I'm open to opinions either way, but I think that we can stop using it.

@rbri
Copy link
Collaborator

rbri commented Feb 16, 2025

Cool....

@andreabergia andreabergia force-pushed the global-as-lambda-constructor branch from 5d73221 to dd490de Compare February 17, 2025 07:47
@andreabergia
Copy link
Contributor Author

andreabergia commented Feb 17, 2025

Rebased on top of master and removed all various wrap calls.

@gbrail
Copy link
Collaborator

gbrail commented Feb 18, 2025

Thanks!

@gbrail gbrail merged commit 01141e1 into mozilla:master Feb 18, 2025
3 checks passed
@andreabergia andreabergia deleted the global-as-lambda-constructor branch February 18, 2025 15:39
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 this pull request may close these issues.

3 participants