Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jan 26, 2026

I noticed that this was not doing anything in JSPI mode.

@sbc100 sbc100 requested review from brendandahl and kripken January 26, 2026 23:00
@sbc100 sbc100 force-pushed the instrumentFunction branch from 7c49a5c to 0bae8c9 Compare January 27, 2026 01:02
@sbc100 sbc100 requested a review from kripken January 27, 2026 01:19
I noticed that this was not doing anything in JSPI mode.
@sbc100 sbc100 force-pushed the instrumentFunction branch from 0bae8c9 to 75464ec Compare January 27, 2026 01:21
ret[x] = wrapper;

} else {
#endif
Copy link
Member

@kripken kripken Jan 27, 2026

Choose a reason for hiding this comment

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

Does this not match if ASYNCIFY == 2? (edit: from line 167) (if so, then my comment from above remains - can this not be reached with values 0 or 1?)

Please add a comment saying what it does match, if I'm reading this wrong.

Copy link
Member

Choose a reason for hiding this comment

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

Or is my confusion about JSPI - what is the value of ASYNCIFY when JSPI is enabled?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

JSPI means ASYNCIFY == 2 yes, they are the same thing.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, makes sense now.

ret[x] = wrapper;

} else {
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Thanks, makes sense now.

@sbc100
Copy link
Collaborator Author

sbc100 commented Jan 27, 2026

@brendandahl WDYT?

@sbc100 sbc100 merged commit cbd34e5 into emscripten-core:main Jan 27, 2026
36 checks passed
@sbc100 sbc100 deleted the instrumentFunction branch January 27, 2026 19:46
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.

3 participants