Fully explain variable redefinition across inputs

[bmeck]

Some differences in expectations found from talking to @hashseed

• some expect let redeclarations to collide with existing let declarations on the script context table, i.e. let variables declared in preceding scripts
• some expect let redeclarations to just work
• some expect redeclarations from the repl to shadow ones declared in preceding scripts

We should specify why variable bindings should work and be more specific in what ways they do work during a single input and across multiple REPL Source Text Records.

[hashseed]

cc @ajklein

[hashseed]

cc @ak239

[bmeck]

My goals are to have variable bindings with a few specific use cases:

1. Ability to use scrollback to run lines multiple times in the REPL.

If you can use the history to declare a variable once, it should operate the same way when run a second time. For example, having multiple imports import _ from 'lodash';

2. Ability to remove "dead" bindings.

If you run some code such as throw 0; let x; in REPLs you can force a binding into a TDZ that spans multiple inputs.

3. Separation from the global scope

A REPL such as DevTools or Node should be somewhat safe as a sandbox allowing creation of any necessary bindings. If people wish to affect global scope there are alternative ways to do so, such as indirect eval. This is particularly relevant if combined with the ability to clobber bindings. It would be very new behavior for a const/let binding on the global scope to be able to be replaced, go from non-TDZ to TDZ, etc.

With those in mind, I would prefer the use a new Environment Record to store bindings created at top level of REPL input.

[syg]

Thanks for proposing this, this has been one of my biggest pet peeves for a long time.

First, a clarification. Does "just work" mean clobbering semantics? It will override the old binding in the environment?

The shadowing semantics is the most ergonomic to me.

1. It enables all the use cases you list.
2. It doesn't require us to relitigate binding (TDZ) semantics for a special REPL mode.

The biggest con I see, given that the REPL in the browser is often used in conjunction with debugging facilities, giving an N-line REPL a >=N-length environment chain might be unfortunate. I don't think that's a very big deal.

[hashseed]

That matches what I had in mind too. Just chaining toplevel let scopes (ScriptContext in V8) but allowing shadowing for REPL seems fine.

[bmeck]

Does "just work" mean clobbering semantics?

IDK, somewhat flexible as long as we have a reasonable explanation.

It will override the old binding in the environment?

That was my expectation, yes. It would give TDZs proper behavior in isolated inputs.

[syg]

I just realized the one surprising thing about chaining environments is it'll make var behavior surprising. A var can't shadow a let, e.g. { let x; { var x; } } is a redeclaration error, but it's pretty unergonomic to make the following throw.

REPL> let x = 42
REPL> var x = 42

[hashseed]

That's what I was suggesting above. Shadowing needs to be allowed. Even for repeated let declarations.

[syg]

@hashseed Oh sorry! That's what you meant by "shadowing".

[bmeck]

One thing that I am a bit concerned about mentally with shadowing and forming an environment record chain is closures:

// input 1
let _ = 1;
function print() {
  console.log(_);
}

// input 2
let _ = 2;
print();

With shadowing I would think the example will output 1. If the main complaint is about TDZ we could talk about that, but I don't think nested environment records makes a lot of sense if shadowing causes closures to point to "old" variables.

[hashseed]

Shadowing would only affect toplevel let. Toplevel var are global properties.

[bmeck]

@hashseed changed my example to use let.

[hashseed]

Guess what I would say is... don't use let if you don't want sensible scoping :)

[syg]

Honestly I can see people being divided on whether the example should print 1 or 2. Both intuitions are somewhat reasonable, but I think I lean towards what @hashseed said. The group of people who expect let to be able to redeclare previous lets in the same scope when using a REPL must keep two sets of semantics in mind. It’s definitely more cognitive burden.

[bmeck]

With the model of nesting / shadowing, we just discussed a potential issue that we need to resolve around REPL being sloppy by default and allowing "use strict".

< 0 
> 0
< 1
> 1

{
  "use strict";
  0
  {
     1
  }
}

Would have the input 1 evaluated in Strict Mode.

[Jamesernator]

Something interesting about the { eval(one); { eval(two); { /* etc */ } } } approach is that it does actually allow a kinda undo feature where you can pop off scopes you don't need anymore:

function createChainableEvalContext(script) {
  function ___createScript(script) {
    return `
      let ___nextChainedEval;
      const __value = eval(\`
        {
          ___nextChainedEval = s => eval(___createScript(s));

          undefined; // If script is empty then eval() value should be
                     // undefined not ___nextChainedEval

          ${ script }
        }
      \`);
      ({ nextChainEval: ___nextChainedEval, result: __value });
    `
  }

  return eval(___createScript(script))
}

class ChainEval {
  _chain = [createChainableEvalContext(``).nextChainEval];

  eval(script) {
    const currentChainEval = this._chain[this._chain.length - 1];
    const { result, nextChainEval } = currentChainEval(script);
    this._chain.push(nextChainEval)
    return result;
  }

  popEval() {
    if (this._chain.length > 1) {
      this._chain.pop();
    }
  }
}

const chainEval = new ChainEval();

chainEval.eval(`let x = 12;`);
chainEval.eval(`let x = 30;`);
chainEval.eval(`x // 1`); // 30
chainEval.popEval() // pop `x // 1`
chainEval.popEval() // pop `let x = 30`
chainEval.eval(`x // 2`) // 12
chainEval.popEval() // pop `x // 2`
chainEval.popEval() // pop `let x = 12`
chainEval.eval(`x`) // ReferenceError

[mikesamuel]

Should chainEval.eval(let x = 20;); be chainEval.eval(let x = 12;);?

What purpose does the undefined before ${ script } serve?

[Jamesernator]

Should chainEval.eval(let x = 20;); be chainEval.eval(let x = 12;);?

Yes I updated it.

What purpose does the undefined before ${ script } serve?

I added a comment, basically in the case the script is just whitespace or comments it makes sure that it doesn't eval to ___nextChainedEval as that was the last statement.

[devsnek]

I achieved this by creating a new environment record type which is a subtype of module environment records. It exists only to override two of the methods, HasLexicalDeclaration and CreateImportBinding. HasLexicalDeclaration always returns false. CreateImportBinding is exactly the same expect that 2. Assert: envRec does not already have a binding for N. is removed. It is shared across all the repl input modules created.

As far as I can tell, this matches the semantics asked for above:

This also means we don't run into nested strict issues:

It is also a module environment, so it inherits top-level await!

Changes to engine262 here, which sorta match changes to spec steps: engine262/engine262@8c107a9