let block bindings not enforcing TDZ errors #1382

Open
getify opened this Issue Sep 27, 2014 · 8 comments

Comments

Projects
None yet
4 participants
@getify

getify commented Sep 27, 2014

Example:

var a = 5;
if (a){ console.log(a); let a = 2; }
console.log(a); 

The spec says that an error should happen on line 2. But currently traceur is transpiling that snippet to:

var a$__24;
var a = 5;
if (a) {
  console.log(a$__24);
  a$__24 = 2;
}
console.log(a);

And thus no such error is occurring as it should.

Will traceur change its transpiling to enforce such errors? Or will it statically find them and report errors during compilation?

@rwaldron

This comment has been minimized.

Show comment
Hide comment
@rwaldron

rwaldron Sep 27, 2014

Contributor

statically find them and report errors during compilation?

Ideally this

Contributor

rwaldron commented Sep 27, 2014

statically find them and report errors during compilation?

Ideally this

@UltCombo

This comment has been minimized.

Show comment
Hide comment
@UltCombo

UltCombo Sep 27, 2014

Contributor

Traceur does not yet support TDZ as commented by @arv in some other issue.

I also hope TDZ to throw errors during compilation when it is implemented.

Contributor

UltCombo commented Sep 27, 2014

Traceur does not yet support TDZ as commented by @arv in some other issue.

I also hope TDZ to throw errors during compilation when it is implemented.

@arv

This comment has been minimized.

Show comment
Hide comment
@arv

arv Sep 29, 2014

Collaborator

There are some static checks that can be done but to fully handle it you would need dynamic checks. Due to function hoisting you cannot do this statically.

let a = f();
const b = 2;
function f() { return b; }

One option is to add a guard on all lookups but that is pretty expensive.

function assertInitialized(v) {
  if (v === UNITIALIZED) throw new ReferenceError();
  return v;
}
var a = UNITIALIZED;
a = 1;
f();
var b = UNITIALIZED;
b = 2;
function f() {
  return assertInitialized(b);
}

but combining this with some static analysis we could probably remove most of them.

Collaborator

arv commented Sep 29, 2014

There are some static checks that can be done but to fully handle it you would need dynamic checks. Due to function hoisting you cannot do this statically.

let a = f();
const b = 2;
function f() { return b; }

One option is to add a guard on all lookups but that is pretty expensive.

function assertInitialized(v) {
  if (v === UNITIALIZED) throw new ReferenceError();
  return v;
}
var a = UNITIALIZED;
a = 1;
f();
var b = UNITIALIZED;
b = 2;
function f() {
  return assertInitialized(b);
}

but combining this with some static analysis we could probably remove most of them.

@UltCombo

This comment has been minimized.

Show comment
Hide comment
@UltCombo

UltCombo Sep 29, 2014

Contributor

@arv consider this code:

let x;
x; // undefined

Accessing x is supposed to return undefined, but if I'm reading your transpiling logic above correctly it'd throw an error instead.

Also your example above doesn't make much sense to me. The return value of f() is never assigned to a, also f() executes before b = UNITIALIZED so the assertion wouldn't throw.

Contributor

UltCombo commented Sep 29, 2014

@arv consider this code:

let x;
x; // undefined

Accessing x is supposed to return undefined, but if I'm reading your transpiling logic above correctly it'd throw an error instead.

Also your example above doesn't make much sense to me. The return value of f() is never assigned to a, also f() executes before b = UNITIALIZED so the assertion wouldn't throw.

@UltCombo

This comment has been minimized.

Show comment
Hide comment
@UltCombo

UltCombo Sep 29, 2014

Contributor

I guess the first part of my comment above can be solved by explicitly assigning undefined to x when transpiling, as for the second part you'd need to hoist the declaration and = UNITIALIZED initialization.

Contributor

UltCombo commented Sep 29, 2014

I guess the first part of my comment above can be solved by explicitly assigning undefined to x when transpiling, as for the second part you'd need to hoist the declaration and = UNITIALIZED initialization.

@arv

This comment has been minimized.

Show comment
Hide comment
@arv

arv Sep 29, 2014

Collaborator

@UltCombo That was mostly illustrative. It has errors and is incomplete. The compiled output should be something more along the lines of:

$traceurRuntime.UNITIALIZED = {};
$traceurRuntime.assertInitialized = function(v) {
  if (v === UNITIALIZED) throw new ReferenceError();
  return v;
};
var a = $traceurRuntime.UNITIALIZED;
var b = $traceurRuntime.UNITIALIZED;
a = f();
b = 2;
function f() {
  return $traceurRuntime.assertInitialized(b);
}

On the second point. let x is just short for let x = undefined so an assignment would be needed.

Collaborator

arv commented Sep 29, 2014

@UltCombo That was mostly illustrative. It has errors and is incomplete. The compiled output should be something more along the lines of:

$traceurRuntime.UNITIALIZED = {};
$traceurRuntime.assertInitialized = function(v) {
  if (v === UNITIALIZED) throw new ReferenceError();
  return v;
};
var a = $traceurRuntime.UNITIALIZED;
var b = $traceurRuntime.UNITIALIZED;
a = f();
b = 2;
function f() {
  return $traceurRuntime.assertInitialized(b);
}

On the second point. let x is just short for let x = undefined so an assignment would be needed.

@UltCombo

This comment has been minimized.

Show comment
Hide comment
@UltCombo

UltCombo Sep 29, 2014

Contributor

@arv makes perfect sense, thanks. =]

Contributor

UltCombo commented Sep 29, 2014

@arv makes perfect sense, thanks. =]

@arv

This comment has been minimized.

Show comment
Hide comment
@arv

arv Sep 30, 2014

Collaborator

My sample didn't show that we need to test assignment too.

Collaborator

arv commented Sep 30, 2014

My sample didn't show that we need to test assignment too.

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