Skip to content

Conversation

sandersn
Copy link
Member

Fixes #7212

When we check context sensitive functions inside an object literal we short-circuit checking of the return type because it might be circular. But the short-circuit means that the checker doesn't go back to check the function later (because it's in a call to checkExpressionCached), which means that this doesn't get marked as needing the _this = this emit.

Thanks @vladima for the psychic debugging here. We came up with two fixes with two variants:

  1. Call checkNodeDeferred in the short-circuit logic in checkFunctionExpressionOrObjectLiteralMethod, same as it is for the non-short-circuit path.
  2. When aggregating return types, call checkExpression instead of checkExpressionCached.

1a. Only call checkNodeDeferred in checkFunctionExpressionOrObjectLiteralMethod if the node hasn't already been added to the deferred node list. Maybe using a nodeflags or something.
2a. Only call checkExpression when the contextual mapper is identityMapper.

This PR goes with (1) since it's the simplest. @ahejlsberg, can you take a look?

@DanielRosenwasser
Copy link
Member

👍

@RyanCavanaugh
Copy link
Member

Orthogonal to the fix itself, I will note that again we have a high-severity bug caused by failing to typecheck every expression node in a program.

Is there anything stopping us from a) setting a flag on a node during typecheck and b) throwing during emit of that node if we don't see that flag? This could at least be enabled for nightly or debug builds.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 25, 2016

👍

@mhegazy
Copy link
Contributor

mhegazy commented Feb 25, 2016

please port this to release-1.8 as well.

sandersn added a commit that referenced this pull request Feb 25, 2016
…al-object

Invalid this emit in contextual object
@sandersn sandersn merged commit 7bbd899 into master Feb 25, 2016
@sandersn sandersn deleted the invalid-this-emit-in-contextual-object branch February 25, 2016 23:01
@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.

TS 1.8.x: invalid this reference inside arrow function
5 participants