Make exception handling models better configurable for the wasm kernel

[anutosh491]

Description

This PR makes the wasm kernel better configurable towards exception handling model (native wasm exceptions or JS based exceptions or any others)

Type of change

Please tick all options which are relevant.

• Bug fix
• New feature
• Added/removed dependencies
• Required documentation updates

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.25%. Comparing base (3bd256b) to head (18f3724).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #440   +/-   ##
=======================================
  Coverage   81.25%   81.25%           
=======================================
  Files          21       21           
  Lines         864      864           
  Branches       78       78           
=======================================
  Hits          702      702           
  Misses        162      162           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[anutosh491]

One thing to make note of is that the main & the side modules go hand in hand.

We can't build the main modules (through the cmake) and the side modules (through the args in kernel.json) differently.

Else we end up with errors like this

Hence for building the wasm kernels using -fexceptions we need to do

emcmake cmake \
  -DCMAKE_BUILD_TYPE=Release \
  -DCMAKE_INSTALL_PREFIX=$PREFIX \
  -DXEUS_CPP_EMSCRIPTEN_WASM_BUILD=ON \
  -DCMAKE_FIND_ROOT_PATH=$PREFIX \
  -DSYSROOT_PATH=$SYSROOT_PATH \
  -DCMAKE_COMPILE_WARNING_AS_ERROR=ON \
  -DXEUS_CPP_WASM_KERNEL_EXTRA_FLAGS="-fexceptions" \
  -DXEUS_CPP_WASM_KERNEL_EXTRA_ARGS="--mllvm;--enable-emscripten-cxx-exceptions;--mllvm;--enable-emscripten-sjlj" \
  ..

And that should reflect accordingly if we inspect the cc1 command

We can probably point this out in the readme/build instructions !

[anutosh491]

cc @SylvainCorlay the doubt I had here is related to
making xeus-cpp configurable vs xeus configurable.

If you see what's done here, xeus hardcodes the EH model based on the emscripten version number.

Which means if we are building using -DXEUS_CPP_WASM_KERNEL_EXTRA_FLAGS="-fexceptions" against emscripten 4-x, we technically can't do that due to the hardcoding in xeus !

[mcbarton]

My understanding is that we want this change so xeus-cpp can remain compatible with the emsdk 3 branch on Emscripten forge which doesn't use wasm exceptions. So that we know the build is agnostic to the EMCC_CFLAGS from whatever emsdk version that user uses from emscripten forge I think this environment variable should be unset in the ci to stop it overwriting whatever the user has set for XEUS_CPP_WASM_KERNEL_EXTRA_FLAGS (probably want it unset in build instructions too). This should also allow CppInterOp to drop references to EMCC_CFLAGS when building xeus-cpp which it currently needs to do.

I would also like to be able to test this PR against CppInterOps ci.

One final remark, which I honestly don't know the answer to, does this test need to change depending on what exception model you use https://github.com/anutosh491/xeus-cpp/blob/3299d5c599fa220466c7751c4af279f4c6e22678/test/test_interpreter.cpp#L145 ?

[anutosh491]

CI has nothing to do here according to me.

xeus-cpp/environment-wasm-build.yml

Lines 3 to 7 in 12c4570

	- https://prefix.dev/emscripten-forge-4x
	- https://prefix.dev/conda-forge
	dependencies:
	- cmake
	- emscripten_emscripten-wasm32==4.0.9

We are using the emsdk (and all the dependencies) from the 4-x channel right ? Which means it will be activated with wasm exceptions right ? That's exactly what we want in the CI.

This PR is only meant to allow users to configure the EH model according to what they want. We would prefer and want the wasm EH model which is exactly what the CI does.

We won't be adding any duplication or testing the other EH model in the CI.

Offcourse, this would only work under the wasm exception model (and no we don't care to test it under the emscripten EH model !)

xeus-cpp/test/test_interpreter.cpp

Lines 142 to 150 in 12c4570

	#if defined(XEUS_CPP_EMSCRIPTEN_WASM_BUILD)
	TEST_CASE("Emscripten Exception Handling")
	{
	std::vector<const char*> Args = {
	"-std=c++20",
	"-v",
	"-fwasm-exceptions",
	"-mllvm", "-wasm-enable-sjlj"
	};

We can clearly mention that going forward our effort would be done with respect to the wasm EH model, if still someone wants to build xeus-cpp with another EH model, they can do it and even try stuff out (I've pasted an image above showing it works, but our default testing wouldn't account for that)

[anutosh491]

gentle ping @SylvainCorlay

I noticed the issue with the PR in CppInterOp which tested these changes against its ci. The Windows ci is now passing, and I have no more objections to this PR in its current form.

[anutosh491]

Thanks for the reviews Sylvain & Matthew. Merging !