Skip to content

Conversation

@zherczeg
Copy link
Member

#3802 should go in first

}
else
{
uintptr_t diff = (uintptr_t) (&((vm_executable_object_t *) 0x100)->frame_ctx) - 0x100;
Copy link
Contributor

@galpeter galpeter Jun 12, 2020

Choose a reason for hiding this comment

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

What does the 0x100 represents here? And what do we want to do here?

@dbatyai dbatyai added the ES.next Related to ES2015+ features label Jun 12, 2020
@zherczeg zherczeg force-pushed the do_await branch 3 times, most recently from 11d0944 to 72d68ef Compare June 15, 2020 12:28
@zherczeg zherczeg force-pushed the do_await branch 2 times, most recently from e7a2d53 to 6fb7fcd Compare June 16, 2020 10:25
}
stack_top_p = executable_object_p->frame_ctx.stack_top_p;

if (executable_object_p->frame_ctx.context_depth > 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

This if and the while after it seems to me used in this form multiple times (with seemingly minimal changes). Would it be possible to extract them into a method?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is only used twice, and I would not combine them because they do opposite things.

Copy link
Contributor

Choose a reason for hiding this comment

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

ohh I see one is ref and the other is deref. Sorry for the noise I've missed this.

@zherczeg
Copy link
Member Author

Thank you for the reviews. I hope I did all requests, please check the patch again.

JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg [email protected]
Copy link
Member

@rerobika rerobika left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@galpeter galpeter left a comment

Choose a reason for hiding this comment

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

lgtm

@zherczeg zherczeg merged commit 8719f72 into jerryscript-project:master Jun 17, 2020
@zherczeg zherczeg deleted the do_await branch June 17, 2020 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ES.next Related to ES2015+ features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect await behavior? async/await support

4 participants