Skip to content

Move emit helper flags to binder.#6495

Merged
rbuckton merged 6 commits into
masterfrom
moveEmitFlagsToBinder
Jan 21, 2016
Merged

Move emit helper flags to binder.#6495
rbuckton merged 6 commits into
masterfrom
moveEmitFlagsToBinder

Conversation

@rbuckton
Copy link
Copy Markdown
Contributor

When investigating #6113 I found the issue also is also reproducible for __extends and other emit helpers.

Previously, when we check a source file we use scoped variables (emitExtends, emitAwaiter, etc.) in the checker that we reset before checking each source file. However, during the widening of object literals we check the object literal expression, which can then in turn check function expression bodies or object literal method bodies. When the function or method body contains a reference to a method defined in a different file, it both triggers the same checks as above, but for the wrong source file. By the time the checker gets to the source file, the expression's type has been cached. As a result, the expression is not rechecked and the triggers to set the scoped variables are never fired for the file.

Since we are guaranteed to do a full walk of one file at a time in the binder, I've moved these checks for our various emit helpers there.

Comment thread src/compiler/binder.ts
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of this, what if you just looked for await expressions? Technically you don't need an await helper unless you use await right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I need the helper for any async function, even if it does not have an await.

@rbuckton
Copy link
Copy Markdown
Contributor Author

@DanielRosenwasser any further comments? @mhegazy, @RyanCavanaugh any comments?

Comment thread src/compiler/binder.ts Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider writing a nodeIsDecoratedCorrectly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

While I'm not keen on the proposed name, I think I will clean up the nodeIsDecorated function, as it can rely on nodeCanBeDecorated.

@DanielRosenwasser
Copy link
Copy Markdown
Member

👍

rbuckton added a commit that referenced this pull request Jan 21, 2016
@rbuckton rbuckton merged commit 2287b46 into master Jan 21, 2016
@rbuckton rbuckton deleted the moveEmitFlagsToBinder branch January 21, 2016 18:31
@vladima vladima mentioned this pull request Jan 25, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 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.

3 participants