Skip to content

Conversation

@pleath
Copy link
Contributor

@pleath pleath commented Aug 4, 2016

Call to a nested function from within a 'with' requires that the function's enclosing scope be represented as an object. We were missing a case where the function is located in a param scope that will later be merge with a body scope. Also adding asserts to verify the object-ness of a scope when we're generating byte code that requires it.

@pleath
Copy link
Contributor Author

pleath commented Aug 4, 2016

@dotnet-bot test Windows arm_test please
@dotnet-bot test Windows x86_debug please

@pleath
Copy link
Contributor Author

pleath commented Aug 4, 2016

@dotnet-bot test Windows arm_release please

@pleath
Copy link
Contributor Author

pleath commented Aug 5, 2016

@aneeshdk Related to merge-ability of param and body scopes.

…tion's enclosing scope be represented as an object. We were missing a case where the function is located in a param scope that will later be merge with a body scope. Also adding asserts to verify the object-ness of a scope when we're generating byte code that requires it.
parentFunc->SetChildHasWith();

if (parentFunc->GetBodyScope()->GetHasOwnLocalInClosure())
if (parentFunc->GetBodyScope()->GetHasOwnLocalInClosure() ||
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we already have a test case for this?

@aneeshdk
Copy link
Contributor

aneeshdk commented Aug 5, 2016

:shipit:

@chakrabot chakrabot merged commit 675a831 into chakra-core:master Aug 5, 2016
chakrabot pushed a commit that referenced this pull request Aug 5, 2016
…aining 'with'

Merge pull request #1388 from pleath:with

Call to a nested function from within a 'with' requires that the function's enclosing scope be represented as an object. We were missing a case where the function is located in a param scope that will later be merge with a body scope. Also adding asserts to verify the object-ness of a scope when we're generating byte code that requires it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants