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

Shouldn't ExecuteModule use the Execution Context constructed in InitializeEnvironment? #2660

Open
jmdyck opened this issue Feb 11, 2022 · 0 comments · May be fixed by #2886
Open

Shouldn't ExecuteModule use the Execution Context constructed in InitializeEnvironment? #2660

jmdyck opened this issue Feb 11, 2022 · 0 comments · May be fixed by #2886

Comments

@jmdyck
Copy link
Collaborator

jmdyck commented Feb 11, 2022

I just noticed that 16.2.1.6.4 InitializeEnvironment and 16.2.1.6.5 ExecuteModule (for a Source Text Module Record) both have:

1. Let _moduleContext_ be a new ECMAScript code execution context.
1. Set the Function of _moduleContext_ to *null*.
1. Set the Realm of _moduleContext_ to _module_.[[Realm]].
1. Set the ScriptOrModule of _moduleContext_ to _module_.
1. Set the VariableEnvironment of _moduleContext_ to _module_.[[Environment]].
1. Set the LexicalEnvironment of _moduleContext_ to _module_.[[Environment]].

Originally, those steps were only in ExecuteModule, and then PR #1670 moved them all to InitializeEnvironment (putting the resulting _moduleContext_ into _module_.[[Context]], which was then used in ExecuteModule).

But then PR #2408 (Top Level Await) added those steps back to ExecuteModule (and dropped the reference to _module_.[[Context]]).

I'm pretty sure this was unintentional (just an ill-resolved rebase conflict), because:

Can someone confirm that ExecuteModule should just use _module_.[[Context]]?

jmdyck added a commit to jmdyck/ecma262 that referenced this issue Sep 3, 2022
In ExecuteModule, use the execution context that
InitializeEnvironment created and put in _module_.[[Context]].

See issue tc39#2660 for backstory.

Resolves tc39#2660.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant