Editorial: tweak note in FunctionDeclarationInstantiation

[bakkot]

I noticed in this thread that FunctionDeclarationInstantiation step 19.a is misleading. It reads

19. If strict is true or if hasParameterExpressions is false, then
    a. NOTE: Only a single Environment Record is needed for the parameters and top-level vars.

The NOTE is identical to that in step 27.a, from which it was presumably copied:

27. If hasParameterExpressions is false, then
    a. NOTE: Only a single Environment Record is needed for the parameters and top-level vars.

But the conditions are different, and in a relevant way. Step 19 actually has nothing to do with top-level var declarations; rather, it's about where to put var declarations arising from sloppy-mode direct calls to eval in parameter expressions. (Step 27/28 is what governs where to put var declarations from the function body.)

So the NOTE in step 19 is misleading. I've changed it to talk about eval instead.

This was introduced in #1046. See also #623 for discussion of whether to keep these branches at all - they're not actually observable to user code (that is, taking the "else" branch in both cases is equivalent to the current state). I think we should probably pursue that eventually, but in the mean time the notes should at least be correct.

[mkaizad]

@bakkot not sure if you'd want to include this for completeness, but step 19 is also needed to preserve backward compatibility with ES5 (see comment by Dmitry Soshnikov on 26/08). Parameter defaults didn’t exist back then, so we'd have to use manual defaults like:

function foo(x, y) {
  if (typeof y == 'undefined') {
    y = function() { x = 2; };
  }
  var x = 3;
  y(); // is `x` shared?
  console.log(x); // yes! 2
}
 
foo();

[bakkot]

@meena-kaizad In the presence of parameter expressions step 27/28 would create a separate scope anyway, so step 19 doesn't really have any bearing on the backwards compatibility concern.

But even if step 19 were eliminated and step 20 made mandatory, and step 27 were eliminated and step 28 made mandatory, backwards compatibility would still be maintained by 28.e. Your example doesn't get at this because it doesn't attempt to read from x before it's assigned to in the body (remember, var declarations are hoisted to the top of the containing scope), but you can observe it with

function foo(x) {
  console.log(x);
  var x = 2;
}
foo(1);

which prints 1 rather than undefined, as it would in the absence of 28.e (assuming 28 was taken unconditionally instead of 27). This happens whether or not you have parameter expressions:

function foo(x, y = 'an expression') {
  console.log(x);
  var x = 2;
}
foo(1);

also prints 1.

You can actually observe the fact that there's two scopes with

function foo(x, probeX = () => x) {
  console.log(x);
  var x = 2;
  console.log(x);
  console.log(probeX());
}
 
foo(1);

which prints 1, 2, 1 - but of course you can't observe this without parameter expressions, so it's a valid optimization to collapse it into a single scope if there are no parameter expressions. But it's just an optimization; the backwards compatible behavior would be maintained either way by 28.e.

[mkaizad]

But it's just an optimization; the backwards compatible behavior would be maintained either way by 28.e.

Oh right, I forgot about 27/28 and wouldn't have made that connection even if I'd remembered. Thanks so much for clearing that up @bakkot.