Skip to content

Conversation

@aardvark179
Copy link
Contributor

@aardvark179 aardvark179 commented Oct 9, 2025

This fixes #2104 and will enable a follow PR to resolve issues with property descriptor creation.

@aardvark179
Copy link
Contributor Author

Making this as a draft because I think there are things that I should tidy up in the byte code compiler, but haven't checked those yet.

@aardvark179 aardvark179 marked this pull request as ready for review October 9, 2025 19:39
@aardvark179 aardvark179 force-pushed the aardvark179-call-scope-fix branch from 13cf163 to fd73cea Compare October 10, 2025 16:07
@aardvark179
Copy link
Contributor Author

I've rebased this on top of master. The improved PromiseBindTest actually showed that I had changed the precise place where the promise queue was processed, so I have also fixed that.

I think there may be further lurking horrors around promises, but I don't think they are something we should fix until we've sorted out this handling everywhere and else and stomped down all the other problems round realms.

@aardvark179 aardvark179 force-pushed the aardvark179-call-scope-fix branch from fd73cea to 55f46f3 Compare October 16, 2025 17:13
@gbrail
Copy link
Collaborator

gbrail commented Oct 21, 2025

Thanks -- I'm all for cleaning up the weirdness and complexity about scopes. This is a pretty important and fundamental change, though -- can we put some documentation in the Function interface, and maybe even elsewhere, to explain to future users of Rhino what the difference is and what each type of scope means? Thanks!

@aardvark179 aardvark179 force-pushed the aardvark179-call-scope-fix branch from 55f46f3 to d7f9e21 Compare October 21, 2025 08:06
@aardvark179
Copy link
Contributor Author

I've added a little Javadoc to Function describing what this method represents, and why we want to distinguish this from the general case of parent objects.

Copy link
Collaborator

@gbrail gbrail left a comment

Choose a reason for hiding this comment

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

OK -- I get what we're doing here and I am supportive of moving away from the confusing world of parent scopes for objects. I also am worried about the many projects that may have depended on some of this functionality over the years, since we had to change a few really old tests to make this work.

I still think that this is OK, but I am going to wait a day or so to see if anyone else has some strong opinions. Thanks!

@aardvark179 aardvark179 force-pushed the aardvark179-call-scope-fix branch from d7f9e21 to 2bd9fd3 Compare October 22, 2025 11:04
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.

Incorrect scope handling in LambdaFunction and LambdaConstructor.

2 participants