Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Downlevel emit for let\const #2161

Merged
merged 24 commits into from
Feb 28, 2015
Merged

Downlevel emit for let\const #2161

merged 24 commits into from
Feb 28, 2015

Conversation

vladima
Copy link
Contributor

@vladima vladima commented Feb 27, 2015

  • changes in tests related to computed properties are related to change createNode -> createSynthesizedNode in emitter.
  • capturing block scoped variables in closures inside loops is not supported scenario in this PR though it can be added later

@@ -1329,6 +1330,7 @@ module ts {

// Values for enum members have been computed, and any errors have been reported for them.
EnumValuesComputed = 0x00000080,
BlockScopedBindingInLoop = 0x00000100,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loop body, not loop initializer, right?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to ES6 spec, let in the loop initializer is also scoped per iteration (so subject to the same downlevel codegen issues). Beware that browsers currently get it wrong (tested in IE11, FF33).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I learned that while in the middle of reviewing this PR. I still do not quite understand the difference between the loop scope and the body scope, given that they are both per iteration. I suppose the difference is that you can access the loop scoped bindings in the initializer, guard, and incrementor of the loop, but they will be shadowed as soon as the body starts for that iteration. Though I still don't see how there could be a difference for for...in and for...of.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest I don't think that there is any useful difference between the two. But there is a technical difference, as the loop body is a new scope nested inside the loop scope itself. It means that you can redeclare let i both in the loop initializer and the loop body. The initializer, loop increment and loop condition share one scope, the loop body is a nested scope.

Allen makes it very clear in his answer to this question: https://esdiscuss.org/topic/in-es6-do-for-loops-with-a-let-const-initializer-create-a-separate-scope

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for explaining that!

@@ -77,6 +78,13 @@ module ts {
return new Symbol(flags, name);
}

function setBlockScopeContainer(node: Node, cleanLocals: boolean) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what cleanLocals means without reading the function, and even then I don't know what the implications for this are. Can you leave a comment regarding the situations in which this is useful?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this. Why is blockScopedContainer.locals populated? I thought it should always be empty to start.

@JsonFreeman
Copy link
Contributor

👍

vladima added a commit that referenced this pull request Feb 28, 2015
@vladima vladima merged commit 8abf4ff into master Feb 28, 2015
@vladima vladima deleted the letConstES5Minus branch February 28, 2015 07:02
@basarat
Copy link
Contributor

basarat commented Mar 5, 2015

this will be a good better alternative to typeguards in my code:

var foo : Fancy|SuperFancy;
if(foo.superFancy){
  let superFancy : SuperFancy = foo;
}
else{
  let fancy : Fancy = foo;
}

@JsonFreeman
Copy link
Contributor

@basarat, will that actually work? You can only dot into members that exist in all parts of the union.

@basarat
Copy link
Contributor

basarat commented Mar 5, 2015

@JsonFreeman if nothing else, you don't even need the union

var foo : any; // Note
if(foo.superFancy){
  let superFancy : SuperFancy = foo;
}
else{
  let fancy : Fancy = foo;
}

@JsonFreeman
Copy link
Contributor

Yes, with any, that would work quite nicely.

@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants