Rename abort JS helper function #21778
Conversation
| _Noreturn void abort(void) | ||
| { | ||
| #if __EMSCRIPTEN__ | ||
| _abort_js(); | ||
| #else |
There was a problem hiding this comment.
What's the advantage of including this .c file if it just forwards out to JS? Implementing it in JS directly seems simpler.
There was a problem hiding this comment.
Sorry I should have mentioned, this was needed as part of an experiment I'm doing to remove the underscore name mangling. One place where that is an issue is where the new library function name abort vs _abort clashed with some existing JS symbol, in this case the JS abort utility defined in the preamble.
Avoiding these clashing names seems like a good thing to avoid confusion anyway.. since if you read the current JS abort library function it looks like its infinite recursion, when in fact its calling from the C-facing abort to the JS-flavored abort (that later takes a string argument).
There was a problem hiding this comment.
I see. Yeah, as a workaround for that problem this sounds reasonable (with a comment in the source maybe saying that is why it is needed).
There was a problem hiding this comment.
Can you also mention this in the commit description?
|
I can put this on hold for now until that experiment is closer to landing. |
|
(can or can't in the last comment?) |
Sorry, I can/will put this on hold. |
Also, remove the comment which I believe is not longer relevant with this change.
|
Actually I think this change is good to go now. I've also improved the standalone version of abort to trap rather than exit(1). |
| #if __EMSCRIPTEN__ | ||
| /* In emscripten we call out JS to perform the actual abort where it can | ||
| * produce a nice error. | ||
| * Note that the JS library function not called `abort` to avoid conflict with |
There was a problem hiding this comment.
| * Note that the JS library function not called `abort` to avoid conflict with | |
| * Note that the JS library function is not called `abort` to avoid conflict with |
| _Noreturn void abort(void) | ||
| { | ||
| #if __EMSCRIPTEN__ | ||
| _abort_js(); | ||
| #else |
There was a problem hiding this comment.
Can you also mention this in the commit description?
Also, remove the comment which I believe is not longer relevant with this change.