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.

Sign up for GitHub

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

Jump to bottom

Editorial: Rename some aliases #2353

Merged

ljharb merged 2 commits into tc39:main from jmdyck:rename_aliases

Jan 20, 2022

Collaborator

jmdyck commented Mar 20, 2021

No description provided.

jmdyck added the editorial change label

syg reviewed

View reviewed changes

Contributor

syg left a comment

I like the scope to env change, but still like the original closure name. Happy to hear preferences from @bakkot and @michaelficarra.

Member

michaelficarra commented Mar 24, 2021 •

edited

Loading

Slightly in favour of scope to env, no preference on closure to function.

edit: Also in favour of body to script rename.

syg added the editor call label

Contributor

syg commented Mar 31, 2021

@jmdyck Discussed this on the editor call. We favor the _scope_ to _env_ change, but let's keep the _closure_ ones as is.

syg removed the editor call label

jmdyck force-pushed the rename_aliases branch from 77ad0df to fc897ab Compare

April 1, 2021 01:07

Collaborator Author

jmdyck commented Apr 1, 2021

(force-pushed to resolve merge conflicts)

Collaborator Author

jmdyck commented Apr 1, 2021

let's keep the _closure_ ones as is.

So is it that the gain is not worth the churn, or do you actively prefer _closure_ in these cases? Because if the latter, then there are other parallel cases that don't use _closure_ that you might want changed:

Currently, every use of _closure_ for a function object is getting the direct result of OrdinaryFunctionCreate. But for about a third of such calls, the alias used is _F_ rather than _closure_. Do you want _F_ changed to _closure_ in those cases?

Contributor

syg commented Apr 28, 2021

So is it that the gain is not worth the churn, or do you actively prefer closure in these cases? Because if the latter, then there are other parallel cases that don't use closure that you might want changed:

I personally actively prefer closure, but IIRC @bakkot's preference here was leaning towards less churn, so I think for this PR we can leave the uses of F as is.

Collaborator Author

jmdyck commented Apr 30, 2021

Okay, dropping the first commit then.

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request


          Editorial: Rename _scope_ to _env_ (and similar cases) (tc39#2353)

94c7da6

Specifically:
    `_scope_`      -> `_env_` (or sometimes `_outerEnv_`)
    `_Scope_`      -> `_env_`
    `_classScope_` -> `_classEnv_`

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request


          Editorial: Rename some script-related aliases (tc39#2353)

40d89a3

In ParseScript:
ParseText is invoked with |Script| as the goal,
so the result (if it isn't errors) is an instance of |Script|,
so change the alias `_body_` to `_script_`.

And note that the Script Record's [[ECMAScriptCode]] field
is also an instance of |Script|.

In ScriptEvaluation:
Since _scriptRecord_.[[ECMAScriptCode]] is an instance of |Script|,
change the alias `_scriptBody_` to `_script_`.

jmdyck force-pushed the rename_aliases branch from fc897ab to 40d89a3 Compare

April 30, 2021 04:21

Collaborator Author

jmdyck commented Apr 30, 2021

force-pushed to:

rebase to master
drop the _closure_ -> _function_ commit

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request


          Editorial: Rename _scope_ to _env_ (and similar cases) (tc39#2353)

e4992ae

Specifically:
    `_scope_`      -> `_env_` (or sometimes `_outerEnv_`)
    `_Scope_`      -> `_env_`
    `_classScope_` -> `_classEnv_`

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request


          Editorial: Rename some script-related aliases (tc39#2353)

In ParseScript:
ParseText is invoked with |Script| as the goal,
so the result (if it isn't errors) is an instance of |Script|,
so change the alias `_body_` to `_script_`.

And note that the Script Record's [[ECMAScriptCode]] field
is also an instance of |Script|.

In ScriptEvaluation:
Since _scriptRecord_.[[ECMAScriptCode]] is an instance of |Script|,
change the alias `_scriptBody_` to `_script_`.

jmdyck force-pushed the rename_aliases branch from 40d89a3 to 6436301 Compare

May 13, 2021 15:08

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request


          Editorial: Rename _scope_ to _env_ (and similar cases) (tc39#2353)

68b4c79

Specifically:
    `_scope_`        -> `_env_` (or sometimes `_outerEnv_`)
    `_Scope_`        -> `_env_`
    `_classScope_`   -> `_classEnv_`
    `_privateScope_` -> `_privateEnv_`
    `_PrivateScope_` -> `_privateEnv_`

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request


          Editorial: Rename some script-related aliases (tc39#2353)

61988e8

In ParseScript:
ParseText is invoked with |Script| as the goal,
so the result (if it isn't errors) is an instance of |Script|,
so change the alias `_body_` to `_script_`.

And note that the Script Record's [[ECMAScriptCode]] field
is also an instance of |Script|.

In ScriptEvaluation:
Since _scriptRecord_.[[ECMAScriptCode]] is an instance of |Script|,
change the alias `_scriptBody_` to `_script_`.

jmdyck force-pushed the rename_aliases branch from 6436301 to 61988e8 Compare

May 13, 2021 15:12

Collaborator Author

jmdyck commented May 13, 2021

force-pushed to:

rebase to master, resolve merge conflicts
handle _privateScope_ (from PR Normative: Add public and private class fields, static class features, private methods & getters/setters #1668) in first commit

ljharb force-pushed the master branch 3 times, most recently from 3d0c24c to 7a79833 Compare

June 29, 2021 02:21

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request


          Editorial: Rename _scope_ to _env_ (and similar cases) (tc39#2353)

223d362

Specifically:
    `_scope_`        -> `_env_` (or sometimes `_outerEnv_`)
    `_Scope_`        -> `_env_`
    `_classScope_`   -> `_classEnv_`
    `_privateScope_` -> `_privateEnv_`
    `_PrivateScope_` -> `_privateEnv_`

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request


          Editorial: Rename some script-related aliases (tc39#2353)

fc931ca

In ParseScript:
ParseText is invoked with |Script| as the goal,
so the result (if it isn't errors) is an instance of |Script|,
so change the alias `_body_` to `_script_`.

And note that the Script Record's [[ECMAScriptCode]] field
is also an instance of |Script|.

In ScriptEvaluation:
Since _scriptRecord_.[[ECMAScriptCode]] is an instance of |Script|,
change the alias `_scriptBody_` to `_script_`.

jmdyck force-pushed the rename_aliases branch from 61988e8 to fc931ca Compare

July 18, 2021 03:54

Collaborator Author

jmdyck commented Jul 18, 2021

force-pushed to:

rebase to master, and
resolve merge conflicts from PR 545

michaelficarra approved these changes

View reviewed changes

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request


          Editorial: Rename _scope_ to _env_ (and similar cases) (tc39#2353)

645ee5a

Specifically:
    `_scope_`        -> `_env_` (or sometimes `_outerEnv_`)
    `_Scope_`        -> `_env_`
    `_classScope_`   -> `_classEnv_`
    `_privateScope_` -> `_privateEnv_`
    `_PrivateScope_` -> `_privateEnv_`

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request


          Editorial: Rename some script-related aliases (tc39#2353)

366e0c5

In ParseScript:
ParseText is invoked with |Script| as the goal,
so the result (if it isn't errors) is an instance of |Script|,
so change the alias `_body_` to `_script_`.

And note that the Script Record's [[ECMAScriptCode]] field
is also an instance of |Script|.

In ScriptEvaluation:
Since _scriptRecord_.[[ECMAScriptCode]] is an instance of |Script|,
change the alias `_scriptBody_` to `_script_`.

jmdyck force-pushed the rename_aliases branch from fc931ca to 366e0c5 Compare

September 14, 2021 15:56

Collaborator Author

jmdyck commented Sep 14, 2021

force-pushed to rebase to master, resolve merge-conflicts

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request


          Editorial: Rename _scope_ to _env_ (and similar cases) (tc39#2353)

56adf4f

Specifically:
    `_scope_`        -> `_env_` (or sometimes `_outerEnv_`)
    `_Scope_`        -> `_env_`
    `_classScope_`   -> `_classEnv_`
    `_privateScope_` -> `_privateEnv_`
    `_PrivateScope_` -> `_privateEnv_`

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request


          Editorial: Rename some script-related aliases (tc39#2353)

4505fa9

In ParseScript:
ParseText is invoked with |Script| as the goal,
so the result (if it isn't errors) is an instance of |Script|,
so change the alias `_body_` to `_script_`.

And note that the Script Record's [[ECMAScriptCode]] field
is also an instance of |Script|.

In ScriptEvaluation:
Since _scriptRecord_.[[ECMAScriptCode]] is an instance of |Script|,
change the alias `_scriptBody_` to `_script_`.

jmdyck force-pushed the rename_aliases branch from 366e0c5 to 4505fa9 Compare

September 24, 2021 21:08

syg approved these changes

View reviewed changes

Contributor

syg left a comment

lgtm, thanks for updating.

bakkot approved these changes

View reviewed changes

michaelficarra added the ready to merge label

jmdyck added 2 commits

January 19, 2022 19:38


          Editorial: Rename _scope_ to _env_ (and similar cases) (tc39#2353)

Specifically:
    `_scope_`        -> `_env_` (or sometimes `_outerEnv_`)
    `_Scope_`        -> `_env_`
    `_classScope_`   -> `_classEnv_`
    `_privateScope_` -> `_privateEnv_`
    `_PrivateScope_` -> `_privateEnv_`


          Editorial: Rename some script-related aliases (tc39#2353)

38a2584

In ParseScript:
ParseText is invoked with |Script| as the goal,
so the result (if it isn't errors) is an instance of |Script|,
so change the alias `_body_` to `_script_`.

And note that the Script Record's [[ECMAScriptCode]] field
is also an instance of |Script|.

In ScriptEvaluation:
Since _scriptRecord_.[[ECMAScriptCode]] is an instance of |Script|,
change the alias `_scriptBody_` to `_script_`.

ljharb force-pushed the rename_aliases branch from 4505fa9 to 38a2584 Compare

January 20, 2022 03:38

ljharb merged commit 38a2584 into tc39:main

jmdyck deleted the rename_aliases branch

January 20, 2022 22:36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

editorial change ready to merge