[EmscriptenEH] Always use custom JS class for C/C++ exceptions#26523
[EmscriptenEH] Always use custom JS class for C/C++ exceptions#26523sbc100 merged 1 commit intoemscripten-core:mainfrom
Conversation
sbc100
force-pushed
the
emscripten_exception_classes
branch
from
a6a0939 to
0f4b37a
Compare
|
I took a stab at what we discussed in today's meeting.. |
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
Yeah, maybe this is worth it for the simplicity.
| _emval_throw: (object) => { | ||
| object = Emval.toValue(object); | ||
| #if !WASM_EXCEPTIONS | ||
| #if !WASM_EXCEPTIONS && !DISABLE_EXCEPTION_CATCHING |
There was a problem hiding this comment.
Should the latter be !DISABLE_EXCEPTION_THROWING? We allow throwing exceptions even when DISABLE_EXCEPTION_CATCHING is true. We should be throwing CppException and not a number in that case too, no?
There was a problem hiding this comment.
No, I believe that then catching is disabled we just abort instead of throwing so there should be nothing to actually throw.
There was a problem hiding this comment.
WDYT @aheejin , OK to land this? Tests seems happy.
There was a problem hiding this comment.
Yeah, forgot that we call abort in that case. LGTM
sbc100
force-pushed
the
emscripten_exception_classes
branch
2 times, most recently
from
0a897cd to
2d33bff
Compare
|
I could split this PR up into smaller ones if you prefer? |
This has a slight codesize cost but it means we no longer have to worry about the ambiguity in when we catch a Number.
sbc100
force-pushed
the
emscripten_exception_classes
branch
from
2d33bff to
10ae340
Compare
This is not necessarily anymore after emscripten-core#26523.
This is not necessarily anymore after #26523.
3 participants
This has a slight codesize cost but it means we no longer have to worry about the ambiguity in when we catch a Number.