Editorial: Eliminate Lexical Environment #1697

jmdyck · 2019-09-16T20:16:16Z

... as suggested in PR #1473.

In effect, I've merged the concept/model of Lexical Environment into that of Environment Record. In my opinion, there's no point where the spec suffers from this merge. The only question is whether the improvement in the spec is worth the hassle of the transition.

The bulk of the change happens over the first 4 commits. Note that these commits are not logically separate -- the intermediate versions of the spec are not self-consistent. The only reason I separated them is to make it somewhat easier for someone who wants to check my work. You'd probably want to squash them when merging.

The last three commits are convenient add-ons.

Downstream effects:

HTML appears to be unaffected.
WebIDL doesn't refer to Lexical Environments explcitly, but does involve them implicitly. Fixing this would mostly amount to changing X's EnvironmentRecord to just X.

devsnek · 2019-09-16T21:26:21Z

i'll try to get this tested in engine262 before the end of the week

jmdyck · 2019-10-14T03:23:41Z

(force-pushed to resolve merge conflicts from PR #1670 and PR #1725)

jmdyck · 2019-10-24T00:14:11Z

(force-pushed to resolve merge conflicts from PR #1740 and PR #1733)

michaelficarra · 2019-10-25T01:05:02Z

I'm neutral on this change. Not everything that has a 1-to-1 relationship needs to be merged. It's also sometimes easier to not have bits you don't care about available to you.

I'd prefer we get #1477 in and later make a decision about whether we do this.

jmdyck · 2019-10-25T02:09:31Z

It's also sometimes easier to not have bits you don't care about available to you.

This sentence had me stumped for a while. Am I right in thinking that, in the context of this PR, "you" is an operation that deals with Environment Records, and the "bits you don't care about" is the 'outer environment reference' of the corresponding Lexical Environment?

michaelficarra · 2019-10-25T02:12:45Z

Correct! Sorry for the loose phrasing.

spec.html

bergus · 2019-11-01T19:49:42Z

I'm not so fond of getting rid of the term "lexical". It serves as a good reminder that JS uses lexical scoping, and that this data structure is the representation of that model.
How about naming the new structure "Lexical Environment Record"? As a bonus, anyone searching for the old terms would find the new one :-) Alternatively, given @allenwb's suggestion, "Lexical Bindings Record"?

jmdyck · 2019-11-04T17:30:04Z

How about naming the new structure "Lexical Environment Record"?

So then all the subtypes would add "Lexical"?

declarative Lexical Environment Record
function Lexical Environment Record
module Lexical Environment Record
object Lexical Environment Record
global Lexical Environment Record

bergus · 2019-11-04T21:18:02Z

@jmdyck Oh, I completely forgot about that. Scrap the idea. An object environment record is definitely not lexical. At best, we could have the hierarchy

Environment Record
- Lexical Environment Record
  - Function Environment Record
  - Module Environment Record
- Object Environment Record
- Global Environment Record

where the term "lexical" now replaced "declarative", for no good reason.

devsnek · 2019-11-05T02:07:05Z

https://tc39.es/ecma262/#sec-globalthis needs to be updated too

devsnek · 2019-11-05T02:17:48Z

With the change for globalThis, this patchset is passing test262 on engine262 :)

jmdyck · 2019-11-06T03:25:12Z

https://tc39.es/ecma262/#sec-globalthis needs to be updated too

Whoops, that snuck in on a rebase. Thanks for catching it.

jmdyck · 2019-11-06T19:00:54Z

Added 2 commits:

Fix globalThis (pointed out by @devsnek).
Fix "Changes to EvalDeclarationInstantiation" (I noticed while reviewing).

Conceptually, these both fit in this PR's second commit ("Merge Lexical Environment into Environment Record"), but I kept them separate rather than squashing them in because I'm expecting all the commits will be squashed on merge anyway (if this PR is accepted).

jmdyck · 2019-11-06T19:59:39Z

Added a commit to fix another change that snuck in on a rebase.

syg

The contents lgtm. At the end of the day I am also pretty neutral on the change. I'll bring it up at the next editor call to try to get a less tepid temperature.

…c39#1697) (currently attached to Lexical Environment, but that will change)

…ithms (tc39#1697) Mostly this amounts to erasing the distinction between _LE_ and "_LE_'s EnvironmentRecord". For the merged thing, I always used the metavariable/alias that had formerly been used for the Lexical Environment, even though this sometimes resulted in more diff lines.

…1697) ... mostly by changing "Lexical Environment" to "Environment Record". For lowercase "lexical environment", the treatment varies, because it's used with a few senses: - same as uppercase, - the LexicalEnvironment component of an execution context, and - the general (non-ES-specific) sense.

…ical Environments (tc39#1697)

…9#1697) (except for function Environment Record, which already has one)

) ... for _realm_.[[GlobalEnv]] and _module_.[[Environment]]

ljharb · 2020-04-24T02:44:38Z

@jmdyck apologies on not giving you the heads up you asked for; please feel free to make a quick followup for final touches

jmdyck · 2020-04-24T17:34:57Z

It looks like nothing else snuck in on a rebase, but I did notice something minor that I should have changed, so I'll add that to #1965.

... in FunctionDeclarationInstantiation, to balance the NOTE in the other arm of the if-else, and also for consistency with the NOTE at 27.a. (I should have done this in PR tc39#1697.)

Editorial: Reinstate an SDO rule PR tc39#1933 deleted SDO rules that are handled by the chain production rule, but it also deleted this one which isn't. (It has "TV" on the left and "TRV" on the right.) Editorial: Move an <emu-note> element PR tc39#1490 (among other things) moved the evaluation semantics for ArrowFunction from the Evaluation SDO to the NamedEvaluation SDO. The accompanying <emu-note> should have moved at the same time (in particular because of the reference to "step 3"). Editorial: Delete <emu-note> in TimeClip clause PR tc39#1827 (among other things) removed step 4 from the algorithm for TimeClip, obsoleting the accompanying emu-note that describes "the point of step 4". Conceivably, the note could be reworded to describe the effect of 'ToInteger' on step 3, but I don't think it'd be worth the bother. Editorial: Change "Step 2.a" to "Step 2.b" in RepeatMatcher note PR tc39#1889 (among other things) inserted a step before the former 2.a, but didn't update the note that referenced it. Editorial: Change step 7 to step 6 in SortCompare note Commit 9c1e076 (2015-10-26) introduced the '?' abbreviation for ReturnIfAbrupt. This caused the ToString call on step 7 to move to step 6, but the note that referred to it wasn't updated. Editorial: Fix typo: "Descritor" -> "Descriptor" Editorial: Fix typo: "GeneratorObject" -> "generator object" (There's no such thing as a GeneratorObject.) Editorial: Delete "as a parameter" after "is present" (It's the only place in the spec where we use that phrasing.) Editorial: Change "which" to "that" ... in "{String,Array,Map,Set} methods which return such iterators" Editorial: Insert a comma in SetDefaultGlobalBindings() Formerly, it read like "containing" modified "the property", when it actually modified "the property descriptor". Editorial: Change "lexical environment" to "Environment Record" ... in FunctionDeclarationInstantiation, to balance the NOTE in the other arm of the if-else, and also for consistency with the NOTE at 27.a. (I should have done this in PR tc39#1697.) Editorial: Change "step 3" to "step 4" in Note ... that accompanies the NamedEvaluation semantics for ArrowFunction : ArrowParameters `=>` ConciseBody PR tc39#1870 (among other things) inserted a step before the former step 3, but didn't update the note that referenced it.

See tc39/ecma262#1697. Also improve cross-references to |PipelineTopicBody|.

Formerly, 7.3 "The Window object" referred to `(realm).[[GlobalEnv]]'s EnvironmentRecord's [[GlobalThisValue]]` Here, `(realm).[[GlobalEnv]]` was a Lexical Environment, whose `EnvironmentRecord` component was a global Environment Record, with a `[[GlobalThisValue]]` field. However, tc39/ecma262#1697 eliminated Lexical Environments, merging them into Environment Records. So now `(realm).[[GlobalEnv]]` is a global Environment Record.

Formerly, 7.3 "The Window object" referred to `(realm).[[GlobalEnv]]'s EnvironmentRecord's [[GlobalThisValue]]` Here, `(realm).[[GlobalEnv]]` was a Lexical Environment, whose `EnvironmentRecord` component was a global Environment Record, with a `[[GlobalThisValue]]` field. However, tc39/ecma262#1697 eliminated Lexical Environments, merging them into Environment Records. So now `(realm).[[GlobalEnv]]` is a global Environment Record, and the reference should be `(realm).[[GlobalEnv]].[[GlobalThisValue]]`

See tc39/ecma262#1697 for context. Also adjust the style as per Web IDL.

jmdyck added the editorial change label Sep 16, 2019

ljharb requested review from zenparsing, allenwb, devsnek and jasonwilliams September 16, 2019 21:20

jmdyck mentioned this pull request Sep 17, 2019

Rename "Environment Record" as "Scope Record", and recast Lexical Environment as a Record #1477

Closed

jmdyck force-pushed the eliminate-lex-env branch from 2c68ee5 to 3d4d88d Compare October 14, 2019 03:20

syg self-requested a review October 17, 2019 00:19

jmdyck force-pushed the eliminate-lex-env branch from 3d4d88d to e844ad1 Compare October 24, 2019 00:12

bergus reviewed Nov 1, 2019

View reviewed changes

spec.html Show resolved Hide resolved

michaelficarra added the editor call to be discussed in the next editor call label Nov 4, 2019

syg reviewed Nov 12, 2019

View reviewed changes

ljharb requested review from michaelficarra and bakkot and removed request for jasonwilliams and zenparsing November 13, 2019 22:54

jmdyck added 7 commits April 23, 2020 16:34

Editorial: Replace "outer environment reference" with [[OuterEnv]] (t…

13fc82f

…c39#1697) (currently attached to Lexical Environment, but that will change)

Editorial: Reorg the intro for Environment Records and the former Lex…

e98d123

…ical Environments (tc39#1697)

Editorial: Add <dfn> elements for subtypes of Environment Record (tc3…

ca9abf6

…9#1697) (except for function Environment Record, which already has one)

Editorial: Specify a particular subtype of Environment Record (tc39#1697

a014888

) ... for _realm_.[[GlobalEnv]] and _module_.[[Environment]]

Editorial: Rename _lex_ to _env_ and _thisLex_ to _thisEnv_ (tc39#1697)

0b988b7

ljharb force-pushed the eliminate-lex-env branch from bdd3771 to 0b988b7 Compare April 23, 2020 23:34

ljharb merged commit 0b988b7 into tc39:master Apr 23, 2020

ljharb mentioned this pull request Apr 23, 2020

eliminate Lexical Environment? #1473

Closed

jmdyck deleted the eliminate-lex-env branch May 1, 2020 22:01

This was referenced Jan 23, 2021

Editorial: Object Environment Record improvements #2287

Merged

Normative: add #x in obj syntax #2290

Merged

js-choi added a commit to tc39/proposal-hack-pipes that referenced this pull request Mar 12, 2021

Spec: Update cross-references to main spec

9ea691b

See tc39/ecma262#1697. Also improve cross-references to |PipelineTopicBody|.

jmdyck mentioned this pull request Jan 26, 2022

Editorial: Remove outdated use of "EnvironmentRecord" whatwg/html#7535

Merged

annevk pushed a commit to whatwg/html that referenced this pull request Jan 26, 2022

Editorial: remove outdated use of "EnvironmentRecord"

b0cf2bc

See tc39/ecma262#1697 for context. Also adjust the style as per Web IDL.

asankah pushed a commit to asankah/html that referenced this pull request Feb 7, 2022

Editorial: remove outdated use of "EnvironmentRecord"

a99b41b

See tc39/ecma262#1697 for context. Also adjust the style as per Web IDL.

demimurych mentioned this pull request Apr 11, 2022

Глава 2-3-Context.md. Определение для Lexical environment не соответствует спецификации HowProgrammingWorks/Book#95

Open

mfreed7 pushed a commit to mfreed7/html that referenced this pull request Jun 3, 2022

Editorial: remove outdated use of "EnvironmentRecord"

3ab8b30

See tc39/ecma262#1697 for context. Also adjust the style as per Web IDL.

jmdyck mentioned this pull request Sep 4, 2022

Editorial: Have NewGlobalEnvironment invoke NewDeclarativeEnvironment #2891

Merged

piotrpospiech mentioned this pull request Nov 11, 2022

Out of date information about the Lexical Environment javascript-tutorial/en.javascript.info#3261

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editorial: Eliminate Lexical Environment #1697

Editorial: Eliminate Lexical Environment #1697

jmdyck commented Sep 16, 2019 •

edited

devsnek commented Sep 16, 2019

jmdyck commented Oct 14, 2019

jmdyck commented Oct 24, 2019

michaelficarra commented Oct 25, 2019

jmdyck commented Oct 25, 2019

michaelficarra commented Oct 25, 2019

bergus commented Nov 1, 2019

jmdyck commented Nov 4, 2019

bergus commented Nov 4, 2019

devsnek commented Nov 5, 2019

devsnek commented Nov 5, 2019

jmdyck commented Nov 6, 2019

jmdyck commented Nov 6, 2019

jmdyck commented Nov 6, 2019

syg left a comment

ljharb commented Apr 24, 2020

jmdyck commented Apr 24, 2020

Editorial: Eliminate Lexical Environment #1697

Editorial: Eliminate Lexical Environment #1697

Conversation

jmdyck commented Sep 16, 2019 • edited

devsnek commented Sep 16, 2019

jmdyck commented Oct 14, 2019

jmdyck commented Oct 24, 2019

michaelficarra commented Oct 25, 2019

jmdyck commented Oct 25, 2019

michaelficarra commented Oct 25, 2019

bergus commented Nov 1, 2019

jmdyck commented Nov 4, 2019

bergus commented Nov 4, 2019

devsnek commented Nov 5, 2019

devsnek commented Nov 5, 2019

jmdyck commented Nov 6, 2019

jmdyck commented Nov 6, 2019

jmdyck commented Nov 6, 2019

syg left a comment

Choose a reason for hiding this comment

ljharb commented Apr 24, 2020

jmdyck commented Apr 24, 2020

jmdyck commented Sep 16, 2019 •

edited