Skip to content
This repository was archived by the owner on Apr 25, 2025. It is now read-only.

[js-api] Few small fixes #200

Merged
merged 4 commits into from
Feb 15, 2022
Merged

[js-api] Few small fixes #200

merged 4 commits into from
Feb 15, 2022

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Feb 10, 2022

These are few small fixes or typos. Some fixes can be wrong if I'm
miunderstanding something, in which case please let me know!

These are few small fixes or typos. Some fixes can be wrong if I'm
miunderstanding something, in which case please let me know!
@aheejin aheejin requested a review from Ms2ger February 10, 2022 10:15
Copy link
Contributor

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

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

Sorry for the delay.

1. If |v| [=implements=] {{Exception}},
1. Let |type| be |v|.\[[Type]].
1. Let |payload| be |v|.\[[Payload]].
1. If |result| [=implements=] {{Exception}},
Copy link
Contributor

Choose a reason for hiding this comment

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

result is a completion record, so can't be an object. I think a "Let v be result.[[Value]]" is missing.

Copy link
Member Author

@aheejin aheejin Feb 10, 2022

Choose a reason for hiding this comment

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

Isn't that just "Let v be result"? Below we are using v.[[Type]] and v.[[Payload]]. I don't think result.[[Value]].[[Type]] makes sense..? Then if v == result, do we need another variable v?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not? result is a completion record, which has a [[Value]] field. The [[Value]] field of result contains a WebAssembly.Exception object, which has [[Type]] and [[Payload]] fields.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought this meant [[Type]] and [[Payload]] exist within the completion record, because it looks this line says so:

1. Note: The expectation is that [=func_invoke=] will be updated to return (|store|, <var ignore>val</var>* | [=error=] | (exception |exntag| |payload|)).

It looks the completion record is extended to contain a tag and a payload.

But as you said, WebAssembly.Exception also has [[Type]] and [[Payload]] field. Do these two fields exist within both entities, WebAssembly.Exception and the completion record?

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 follow. func_invoke is a wasm algorithm, so returns wasm values, not completion records.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I reverted this too.

Copy link
Contributor

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

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

Thanks!

@Ms2ger Ms2ger merged commit 539be03 into WebAssembly:main Feb 15, 2022
@aheejin aheejin deleted the js_api_fix branch February 15, 2022 08:12
@aheejin
Copy link
Member Author

aheejin commented Feb 15, 2022

Thanks! This PR didn't matter, but please let me merge my PRs next time! :)

@Ms2ger
Copy link
Contributor

Ms2ger commented Feb 15, 2022

🤦 Thanks for reminding me!

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.

2 participants