Try blocks not identified as scope level

[megawac]

As far as I can tell after peaking at the spec, try blocks should also be included in the scopes while catch and finally shouldn't. However escope only creates adds a scope for catch blocks

import {parse} from 'acorn';
import escope from 'escope';

let ast = parse(`
const x = 2;
try {
  const x = 1;
  [1, 2, 3].map(x => x);
} catch(o_O) {
  a();
} finally {
  console.log(2);
}
`, { ecmaVersion: 6});

escope.analyze(ast);
// [{global scope}, {catch scope}]

[kumavis]

var overwrites as you expect it would

var x = 16
try {
  var x = 15
} catch (err) {}
console.log(x) //=> 15

in node im seeing an error for const, suggesting its the same scope

╭─kumavis@xyzs-MacBook-Pro  ~/dev/vapor-meta   (master*) 
╰─$ node -v
v4.0.0
╭─kumavis@xyzs-MacBook-Pro  ~/dev/vapor-meta   (master*) 
╰─$ node
> const x = 16
undefined
> try {
...   const x = 15
... } catch (err) {}
TypeError: Identifier 'x' has already been declared
    at repl:1:1
    at REPLServer.defaultEval (repl.js:164:27)
    at bound (domain.js:250:14)
    at REPLServer.runBound [as eval] (domain.js:263:12)
    at REPLServer.<anonymous> (repl.js:392:12)
    at emitOne (events.js:82:20)
    at REPLServer.emit (events.js:169:7)
    at REPLServer.Interface._onLine (readline.js:210:10)
    at REPLServer.Interface._line (readline.js:546:8)
    at REPLServer.Interface._ttyWrite (readline.js:823:14)
> console.log(x) //=> 15
16
undefined
> 
(^C again to quit)
> 

however with let it does behave as a separate scope

╭─kumavis@xyzs-MacBook-Pro  ~/dev/vapor-transform ‹node-v4.0.0›  (master*) 
╰─$ node          
> 
> let x = 16
undefined
> try {
...   let x = 15
... } catch (err) {}
undefined
> console.log(x) //=> 15
16
undefined
> 

[kumavis]

referencing the ES6 meta issue https://github.com/estools/escope/issues/33

[michaelficarra]

node (V8) doesn't implement const properly.

[kumavis]

a fork in the js consensus! quick buy more ASIC v8 miners to gain quorum! wait i must be lost

[kumavis]

hmm that does make things a bit tricky -- what behavior do we track? the spec? or the most common runtime's behavior?

[kumavis]

I think this is the relevant v8 issue: https://code.google.com/p/v8/issues/detail?id=2198&q=const%20scope&colspec=ID%20Type%20Status%20Priority%20Owner%20Summary%20HW%20OS%20Area%20Stars

[es6] Initial support for let/const bindings in sloppy mode

Allow let in sloppy mode with --harmony-sloppy

Allow ES'15 const in sloppy mode with --harmony-sloppy --no-legacy-const

[RReverser]

what behavior do we track? the spec? or the most common runtime's behavior?

Well, it's not "most common", it was just something not implemented in one particular engine yet, so yes, we should always strive for the spec compatibility. Closing as it looks like non-bug to me.

[michaelficarra]

Re-opening until we can confirm that all blocks are given a scope, including try, catch, and finally.

[RReverser]

@michaelficarra If that's the goal (all blocks), it's much broader issue than current one. Can you please submit it as a separate issue?

[michaelficarra]

Fine, just these three. All blocks is just generally related to correctness, so we'll assume they work until we hear otherwise.

[RReverser]

@michaelficarra In that case, what's special about those that we would want to create scopes for them even when they're redundant?

[michaelficarra]

@RReverser What's redundant about these blocks? They each create a scope object.

[RReverser]

@michaelficarra Regular blocks do as well. I'm suggesting that we keep consistency and either create scope objects for any blocks or keep current behavior where scopes are created for functions + only those blocks that actually have block-scoped variables.

[michaelficarra]

@RReverser Yeah, I was assuming we'd keep the current behaviour. If you look at the original example, it is claiming that we're not representing the try scope, even though it has a block-scoped declaration (const).

[RReverser]

@michaelficarra Ah, I see where the misunderstanding came from. Agreed, that's a bug.