Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jun 10, 2021

This is really step 1 of moving more of this code to be
native.

@sbc100 sbc100 force-pushed the native_emscripten_exception_def branch from c953563 to 365d6a3 Compare June 10, 2021 19:23
@sbc100 sbc100 changed the base branch from main to emscripten_exceptions June 10, 2021 19:23
@sbc100 sbc100 requested a review from kripken June 10, 2021 19:23
@sbc100 sbc100 changed the title Define emscripten exception struct in native code Define emscripten exception struct in native code. NFC Jun 10, 2021
@sbc100 sbc100 force-pushed the native_emscripten_exception_def branch from 365d6a3 to b1627b4 Compare June 10, 2021 20:00
@sbc100 sbc100 force-pushed the emscripten_exceptions branch from 6b1d670 to 554a344 Compare June 10, 2021 20:08
@sbc100 sbc100 force-pushed the native_emscripten_exception_def branch from b1627b4 to 9cbe3c4 Compare June 10, 2021 20:12
@sbc100 sbc100 force-pushed the emscripten_exceptions branch from 554a344 to f74ed6b Compare June 10, 2021 20:14
@sbc100 sbc100 force-pushed the native_emscripten_exception_def branch 2 times, most recently from 88d83d0 to 0cf5292 Compare June 10, 2021 20:55
Base automatically changed from emscripten_exceptions to main June 11, 2021 15:05
return expected;
uintptr_t expected = (uintptr_t)t;
__c11_atomic_compare_exchange_strong((_Atomic uintptr_t*)p, &expected, (uintptr_t)s, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
return (void*)expected;
Copy link
Member

Choose a reason for hiding this comment

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

Are these atomic changes related?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is needed so that the header is compatible with C++.. this is musl internal header and it normally nevers goes through the C++ compiler.

I could split struct_info_internal.json into two.. one for C headers and one for C++ headers.. but it seemed better/simpler to fix this header to avoid the C++ error.

for line in lines:
arg = line[1:].strip()
if '::' in arg:
arg = arg.split('::', 1)[1]
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised this works... is the namespace used in the file, I guess, so it's optional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This just allows us to write C_STRUCTS.__cxa_exception in the JS library code instead of C_STRUCTS.__cxxabiv1::__cxa_exception.. as long as there are no collisions it works fine.

@sbc100 sbc100 force-pushed the native_emscripten_exception_def branch from 0cf5292 to 07fa9ce Compare June 11, 2021 18:51
This is really step 1 of moving more of this code to be
native.
@sbc100 sbc100 force-pushed the native_emscripten_exception_def branch from 07fa9ce to 83630d5 Compare June 11, 2021 19:27
@sbc100 sbc100 merged commit 6b80c5d into main Jun 11, 2021
@sbc100 sbc100 deleted the native_emscripten_exception_def branch June 11, 2021 23:53
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