From d5850b0b3a950476409d21b87a407a3ce5462cfc Mon Sep 17 00:00:00 2001 From: anutosh491 Date: Fri, 14 Mar 2025 17:49:49 +0530 Subject: [PATCH 1/4] Fix all crashing tests for emscripten build --- include/clang/Interpreter/CppInterOp.h | 6 +-- lib/Interpreter/CMakeLists.txt | 11 ++-- lib/Interpreter/CppInterOp.cpp | 6 +-- lib/Interpreter/exports.ld | 9 ++++ unittests/CppInterOp/CMakeLists.txt | 50 +++++++++++-------- .../CppInterOp/FunctionReflectionTest.cpp | 9 ++-- unittests/CppInterOp/ScopeReflectionTest.cpp | 12 ----- unittests/CppInterOp/TypeReflectionTest.cpp | 3 -- .../CppInterOp/VariableReflectionTest.cpp | 5 +- 9 files changed, 57 insertions(+), 54 deletions(-) diff --git a/include/clang/Interpreter/CppInterOp.h b/include/clang/Interpreter/CppInterOp.h index 76b5f973f..987d97f47 100644 --- a/include/clang/Interpreter/CppInterOp.h +++ b/include/clang/Interpreter/CppInterOp.h @@ -127,12 +127,12 @@ namespace Cpp { : m_Kind(K), m_DestructorCall(C), m_FD(Dtor) {} /// Checks if the passed arguments are valid for the given function. - bool AreArgumentsValid(void* result, ArgList args, void* self) const; + CPPINTEROP_API bool AreArgumentsValid(void* result, ArgList args, void* self) const; /// This function is used for debugging, it reports when the function was /// called. - void ReportInvokeStart(void* result, ArgList args, void* self) const; - void ReportInvokeStart(void* object, unsigned long nary, + CPPINTEROP_API void ReportInvokeStart(void* result, ArgList args, void* self) const; + CPPINTEROP_API void ReportInvokeStart(void* object, unsigned long nary, int withFree) const; void ReportInvokeEnd() const; public: diff --git a/lib/Interpreter/CMakeLists.txt b/lib/Interpreter/CMakeLists.txt index cd2f46571..bc7cd2957 100644 --- a/lib/Interpreter/CMakeLists.txt +++ b/lib/Interpreter/CMakeLists.txt @@ -129,10 +129,15 @@ if(EMSCRIPTEN) #FIXME: A patch is needed to llvm to remove -Wl,-z,defs since it is now recognised on emscripten. What needs to be removed is here # https://github.com/llvm/llvm-project/blob/128e2e446e90c3b1827cfc7d4d19e3c0976beff3/llvm/cmake/modules/HandleLLVMOptions.cmake#L318 . The PR to do try to do this is here # https://github.com/llvm/llvm-project/pull/123396 - set_target_properties(clangCppInterOp PROPERTIES - NO_SONAME 1 - LINK_FLAGS "-s WASM_BIGINT -s SIDE_MODULE=1 ${SYMBOLS_LIST}" + set_target_properties(clangCppInterOp + PROPERTIES NO_SONAME 1 ) + target_link_options(clangCppInterOp + PRIVATE "SHELL: -s WASM_BIGINT" + PRIVATE "SHELL: -s SIDE_MODULE=1" + PRIVATE "SHELL: ${SYMBOLS_LIST}" + ) + if (CPPINTEROP_ENABLE_TESTING) # When compiling Emscripten tests the shared library it links to is expected to be in the same folder as the compiled Javascript add_custom_command(TARGET clangCppInterOp POST_BUILD diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index e4ef404a7..b5a00231d 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -100,7 +100,7 @@ namespace Cpp { static clang::ASTContext& getASTContext() { return getSema().getASTContext(); } #define DEBUG_TYPE "jitcall" - bool JitCall::AreArgumentsValid(void* result, ArgList args, + CPPINTEROP_API bool JitCall::AreArgumentsValid(void* result, ArgList args, void* self) const { bool Valid = true; if (Cpp::IsConstructor(m_FD)) { @@ -130,7 +130,7 @@ namespace Cpp { return Valid; } - void JitCall::ReportInvokeStart(void* result, ArgList args, void* self) const{ + CPPINTEROP_API void JitCall::ReportInvokeStart(void* result, ArgList args, void* self) const{ std::string Name; llvm::raw_string_ostream OS(Name); auto FD = (const FunctionDecl*) m_FD; @@ -145,7 +145,7 @@ namespace Cpp { ); } - void JitCall::ReportInvokeStart(void* object, unsigned long nary, + CPPINTEROP_API void JitCall::ReportInvokeStart(void* object, unsigned long nary, int withFree) const { std::string Name; llvm::raw_string_ostream OS(Name); diff --git a/lib/Interpreter/exports.ld b/lib/Interpreter/exports.ld index 2e913dec1..b3192ddbf 100644 --- a/lib/Interpreter/exports.ld +++ b/lib/Interpreter/exports.ld @@ -37,3 +37,12 @@ -Wl,--export=_ZNK5clang7TagType7getDeclEv -Wl,--export=_ZNK5clang7VarDecl28isThisDeclarationADefinitionERNS_10ASTContextE -Wl,--export=_ZTVN4llvm18raw_string_ostreamE +-Wl,--export=_ZN4llvm13StringMapImpl11RehashTableEj +-Wl,--export=_ZN4llvm13StringMapImpl15LookupBucketForENS_9StringRefEj +-Wl,--export=_ZN4llvm13StringMapImpl4hashENS_9StringRefE +-Wl,--export=_ZNK5clang10ASTContext14getComplexTypeENS_8QualTypeE +-Wl,--export=_ZNK5clang10ASTContext19getTypeDeclTypeSlowEPKNS_8TypeDeclE +-Wl,--export=_ZNK5clang10RecordDecl19isInjectedClassNameEv +-Wl,--export=_ZNK5clang11DeclContext6lookupENS_15DeclarationNameE +-Wl,--export=_ZNK5clang17ClassTemplateDecl18getSpecializationsEv +-Wl,--export=_ZNK5clang4Sema15getStdNamespaceEv \ No newline at end of file diff --git a/unittests/CppInterOp/CMakeLists.txt b/unittests/CppInterOp/CMakeLists.txt index da02dbcef..e5c0c03ef 100644 --- a/unittests/CppInterOp/CMakeLists.txt +++ b/unittests/CppInterOp/CMakeLists.txt @@ -25,16 +25,21 @@ add_cppinterop_unittest(CppInterOpTests ) if(EMSCRIPTEN) - # Explaination of compile and link flags - # To dynamically link to a shared library for Emscripten builds you must use the MAIN_MODULE flag (see both https://github.com/emscripten-core/emscripten/issues/23543#issuecomment-2625334414 - # and https://emscripten.org/docs/compiling/Dynamic-Linking.html) - # Without WASM_BIGINT flag then you get fatal errors when trying to run compiled Javascript about trying to convert between BigInt and non BigInt types - # EXPORTED_RUNTIME_METHODS='[\"FS\",\"PATH\",\"LDSO\",\"loadDynamicLibrary\",\"ERRNO_CODES\"]' and --preload-file ${SYSROOT_PATH}/include@/include are not allow the Javascript that is - # compiled to have access to the standard library headers (approach taken from xeus-cpp) - # Without ALLOW_MEMORY_GROWTH=1 tests will fail with aborted(OOM). Approach to fix taken from answers to - # https://stackoverflow.com/questions/67222200/runtimeerror-abortoom-build-with-s-assertions-1-for-more-info - set_target_properties(CppInterOpTests PROPERTIES - LINK_FLAGS "-s MAIN_MODULE=1 -s WASM_BIGINT -s EXPORTED_RUNTIME_METHODS='[\"FS\",\"PATH\",\"LDSO\",\"loadDynamicLibrary\",\"ERRNO_CODES\"]' --preload-file ${SYSROOT_PATH}/include@/include -s ALLOW_MEMORY_GROWTH=1" + target_compile_options(CppInterOpTests + PUBLIC "SHELL: -fexceptions" + ) + + target_link_options(CppInterOpTests + PUBLIC "SHELL: -fexceptions" + PUBLIC "SHELL: -s MAIN_MODULE=1" + PUBLIC "SHELL: -s WASM=1" + PUBLIC "SHELL: -s WASM_BIGINT" + PUBLIC "SHELL: -s ASSERTIONS=0" + PUBLIC "SHELL: -s ALLOW_MEMORY_GROWTH=1" + PUBLIC "SHELL: -s EXIT_RUNTIME=1" + PUBLIC "SHELL: -s STACK_SIZE=32mb" + PUBLIC "SHELL: -s INITIAL_MEMORY=128mb" + PUBLIC "SHELL: --preload-file ${SYSROOT_PATH}/include@/include" ) endif() @@ -69,16 +74,21 @@ target_link_libraries(DynamicLibraryManagerTests ) if(EMSCRIPTEN) - # Explaination of compile and link flags - # To dynamically link to a shared library for Emscripten builds you must use the MAIN_MODULE flag (see both https://github.com/emscripten-core/emscripten/issues/23543#issuecomment-2625334414 - # and https://emscripten.org/docs/compiling/Dynamic-Linking.html) - # Without WASM_BIGINT flag then you get fatal errors when trying to run compiled Javascript about trying to convert between BigInt and non BigInt types - # EXPORTED_RUNTIME_METHODS='[\"FS\",\"PATH\",\"LDSO\",\"loadDynamicLibrary\",\"ERRNO_CODES\"]' and --preload-file ${SYSROOT_PATH}/include@/include are not allow the Javascript that is - # compiled to have access to the standard library headers (approach taken from xeus-cpp) - # Without ALLOW_MEMORY_GROWTH=1 tests will fail with aborted(OOM). Approach to fix taken from answers to - # https://stackoverflow.com/questions/67222200/runtimeerror-abortoom-build-with-s-assertions-1-for-more-info - set_target_properties(DynamicLibraryManagerTests PROPERTIES - LINK_FLAGS "-s MAIN_MODULE=1 -s WASM_BIGINT -s EXPORTED_RUNTIME_METHODS='[\"FS\",\"PATH\",\"LDSO\",\"loadDynamicLibrary\",\"ERRNO_CODES\"]' --preload-file ${SYSROOT_PATH}/include@/include -s ALLOW_MEMORY_GROWTH=1" + target_compile_options(DynamicLibraryManagerTests + PUBLIC "SHELL: -fexceptions" + ) + + target_link_options(DynamicLibraryManagerTests + PUBLIC "SHELL: -fexceptions" + PUBLIC "SHELL: -s MAIN_MODULE=1" + PUBLIC "SHELL: -s WASM=1" + PUBLIC "SHELL: -s WASM_BIGINT" + PUBLIC "SHELL: -s ASSERTIONS=0" + PUBLIC "SHELL: -s ALLOW_MEMORY_GROWTH=1" + PUBLIC "SHELL: -s EXIT_RUNTIME=1" + PUBLIC "SHELL: -s STACK_SIZE=32mb" + PUBLIC "SHELL: -s INITIAL_MEMORY=128mb" + PUBLIC "SHELL: --preload-file ${SYSROOT_PATH}/include@/include" ) endif() diff --git a/unittests/CppInterOp/FunctionReflectionTest.cpp b/unittests/CppInterOp/FunctionReflectionTest.cpp index 055115fd0..8862fbd72 100644 --- a/unittests/CppInterOp/FunctionReflectionTest.cpp +++ b/unittests/CppInterOp/FunctionReflectionTest.cpp @@ -547,9 +547,6 @@ TEST(FunctionReflectionTest, ExistsFunctionTemplate) { } TEST(FunctionReflectionTest, InstantiateTemplateFunctionFromString) { -#ifdef EMSCRIPTEN - GTEST_SKIP() << "Test crashes gtest on Emscipten"; -#endif if (llvm::sys::RunningOnValgrind()) GTEST_SKIP() << "XFAIL due to Valgrind report"; Cpp::CreateInterpreter(); @@ -1053,7 +1050,7 @@ TEST(FunctionReflectionTest, IsStaticMethod) { TEST(FunctionReflectionTest, GetFunctionAddress) { #ifdef EMSCRIPTEN - GTEST_SKIP() << "Test crashes gtest on Emscipten"; + GTEST_SKIP() << "Test fails for Emscipten builds"; #endif if (llvm::sys::RunningOnValgrind()) GTEST_SKIP() << "XFAIL due to Valgrind report"; @@ -1100,7 +1097,7 @@ TEST(FunctionReflectionTest, IsVirtualMethod) { TEST(FunctionReflectionTest, JitCallAdvanced) { #ifdef EMSCRIPTEN - GTEST_SKIP() << "Test crashes gtest on Emscipten"; + GTEST_SKIP() << "Test fails for Emscipten builds"; #endif if (llvm::sys::RunningOnValgrind()) GTEST_SKIP() << "XFAIL due to Valgrind report"; @@ -1127,7 +1124,7 @@ TEST(FunctionReflectionTest, JitCallAdvanced) { TEST(FunctionReflectionTest, GetFunctionCallWrapper) { #ifdef EMSCRIPTEN - GTEST_SKIP() << "Test crashes gtest on Emscipten"; + GTEST_SKIP() << "Test fails for Emscipten builds"; #endif if (llvm::sys::RunningOnValgrind()) GTEST_SKIP() << "XFAIL due to Valgrind report"; diff --git a/unittests/CppInterOp/ScopeReflectionTest.cpp b/unittests/CppInterOp/ScopeReflectionTest.cpp index f8b4a27cb..b3edf5b00 100644 --- a/unittests/CppInterOp/ScopeReflectionTest.cpp +++ b/unittests/CppInterOp/ScopeReflectionTest.cpp @@ -161,9 +161,6 @@ TEST(ScopeReflectionTest, SizeOf) { TEST(ScopeReflectionTest, IsBuiltin) { -#ifdef EMSCRIPTEN - GTEST_SKIP() << "Test crashes gtest on Emscipten"; -#endif // static std::set g_builtins = // {"bool", "char", "signed char", "unsigned char", "wchar_t", "short", "unsigned short", // "int", "unsigned int", "long", "unsigned long", "long long", "unsigned long long", @@ -494,9 +491,6 @@ TEST(ScopeReflectionTest, GetScopefromCompleteName) { } TEST(ScopeReflectionTest, GetNamed) { -#ifdef EMSCRIPTEN - GTEST_SKIP() << "Test crashes gtest on Emscipten"; -#endif std::string code = R"(namespace N1 { namespace N2 { class C { @@ -882,9 +876,6 @@ template T TrivialFnTemplate() { return T(); } } TEST(ScopeReflectionTest, InstantiateTemplateFunctionFromString) { -#ifdef EMSCRIPTEN - GTEST_SKIP() << "Test crashes gtest on Emscipten"; -#endif if (llvm::sys::RunningOnValgrind()) GTEST_SKIP() << "XFAIL due to Valgrind report"; Cpp::CreateInterpreter(); @@ -1025,9 +1016,6 @@ TEST(ScopeReflectionTest, GetClassTemplateInstantiationArgs) { TEST(ScopeReflectionTest, IncludeVector) { -#ifdef EMSCRIPTEN - GTEST_SKIP() << "Test crashes gtest on Emscipten"; -#endif if (llvm::sys::RunningOnValgrind()) GTEST_SKIP() << "XFAIL due to Valgrind report"; std::string code = R"( diff --git a/unittests/CppInterOp/TypeReflectionTest.cpp b/unittests/CppInterOp/TypeReflectionTest.cpp index 7f9b0977f..4f47c0229 100644 --- a/unittests/CppInterOp/TypeReflectionTest.cpp +++ b/unittests/CppInterOp/TypeReflectionTest.cpp @@ -546,9 +546,6 @@ TEST(TypeReflectionTest, IsPODType) { } TEST(TypeReflectionTest, IsSmartPtrType) { -#ifdef EMSCRIPTEN - GTEST_SKIP() << "Test crashes gtest on Emscipten"; -#endif if (llvm::sys::RunningOnValgrind()) GTEST_SKIP() << "XFAIL due to Valgrind report"; Cpp::CreateInterpreter(); diff --git a/unittests/CppInterOp/VariableReflectionTest.cpp b/unittests/CppInterOp/VariableReflectionTest.cpp index b58931a51..0122a156f 100644 --- a/unittests/CppInterOp/VariableReflectionTest.cpp +++ b/unittests/CppInterOp/VariableReflectionTest.cpp @@ -264,7 +264,7 @@ CODE TEST(VariableReflectionTest, GetVariableOffset) { #ifdef EMSCRIPTEN - GTEST_SKIP() << "Test crashes gtest on Emscipten"; + GTEST_SKIP() << "Test fails for Emscipten builds"; #endif std::vector Decls; #define Stringify(s) Stringifyx(s) @@ -331,9 +331,6 @@ TEST(VariableReflectionTest, GetVariableOffset) { CODE TEST(VariableReflectionTest, VariableOffsetsWithInheritance) { -#ifdef EMSCRIPTEN - GTEST_SKIP() << "Test crashes gtest on Emscipten"; -#endif if (llvm::sys::RunningOnValgrind()) GTEST_SKIP() << "XFAIL due to Valgrind report"; From ac4f7dddcb1defc4216d4a3f5e6f5d8e37b41080 Mon Sep 17 00:00:00 2001 From: anutosh491 Date: Mon, 17 Mar 2025 12:09:39 +0530 Subject: [PATCH 2/4] clang format reviews --- include/clang/Interpreter/CppInterOp.h | 8 +++++--- lib/Interpreter/CppInterOp.cpp | 6 +++--- unittests/CppInterOp/CMakeLists.txt | 6 ------ 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/include/clang/Interpreter/CppInterOp.h b/include/clang/Interpreter/CppInterOp.h index 987d97f47..4f4f05c64 100644 --- a/include/clang/Interpreter/CppInterOp.h +++ b/include/clang/Interpreter/CppInterOp.h @@ -127,13 +127,15 @@ namespace Cpp { : m_Kind(K), m_DestructorCall(C), m_FD(Dtor) {} /// Checks if the passed arguments are valid for the given function. - CPPINTEROP_API bool AreArgumentsValid(void* result, ArgList args, void* self) const; + CPPINTEROP_API bool AreArgumentsValid(void* result, ArgList args, + void* self) const; /// This function is used for debugging, it reports when the function was /// called. - CPPINTEROP_API void ReportInvokeStart(void* result, ArgList args, void* self) const; + CPPINTEROP_API void ReportInvokeStart(void* result, ArgList args, + void* self) const; CPPINTEROP_API void ReportInvokeStart(void* object, unsigned long nary, - int withFree) const; + int withFree) const; void ReportInvokeEnd() const; public: Kind getKind() const { return m_Kind; } diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index b5a00231d..e4ef404a7 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -100,7 +100,7 @@ namespace Cpp { static clang::ASTContext& getASTContext() { return getSema().getASTContext(); } #define DEBUG_TYPE "jitcall" - CPPINTEROP_API bool JitCall::AreArgumentsValid(void* result, ArgList args, + bool JitCall::AreArgumentsValid(void* result, ArgList args, void* self) const { bool Valid = true; if (Cpp::IsConstructor(m_FD)) { @@ -130,7 +130,7 @@ namespace Cpp { return Valid; } - CPPINTEROP_API void JitCall::ReportInvokeStart(void* result, ArgList args, void* self) const{ + void JitCall::ReportInvokeStart(void* result, ArgList args, void* self) const{ std::string Name; llvm::raw_string_ostream OS(Name); auto FD = (const FunctionDecl*) m_FD; @@ -145,7 +145,7 @@ namespace Cpp { ); } - CPPINTEROP_API void JitCall::ReportInvokeStart(void* object, unsigned long nary, + void JitCall::ReportInvokeStart(void* object, unsigned long nary, int withFree) const { std::string Name; llvm::raw_string_ostream OS(Name); diff --git a/unittests/CppInterOp/CMakeLists.txt b/unittests/CppInterOp/CMakeLists.txt index e5c0c03ef..124fe1e58 100644 --- a/unittests/CppInterOp/CMakeLists.txt +++ b/unittests/CppInterOp/CMakeLists.txt @@ -25,9 +25,6 @@ add_cppinterop_unittest(CppInterOpTests ) if(EMSCRIPTEN) - target_compile_options(CppInterOpTests - PUBLIC "SHELL: -fexceptions" - ) target_link_options(CppInterOpTests PUBLIC "SHELL: -fexceptions" @@ -74,9 +71,6 @@ target_link_libraries(DynamicLibraryManagerTests ) if(EMSCRIPTEN) - target_compile_options(DynamicLibraryManagerTests - PUBLIC "SHELL: -fexceptions" - ) target_link_options(DynamicLibraryManagerTests PUBLIC "SHELL: -fexceptions" From b6e20f5819f7a260b939f55c441706ceae07ded1 Mon Sep 17 00:00:00 2001 From: anutosh491 Date: Mon, 17 Mar 2025 14:59:04 +0530 Subject: [PATCH 3/4] some reviews --- lib/Interpreter/CMakeLists.txt | 11 +++-------- unittests/CppInterOp/CMakeLists.txt | 4 ---- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/lib/Interpreter/CMakeLists.txt b/lib/Interpreter/CMakeLists.txt index bc7cd2957..a16b77373 100644 --- a/lib/Interpreter/CMakeLists.txt +++ b/lib/Interpreter/CMakeLists.txt @@ -129,15 +129,10 @@ if(EMSCRIPTEN) #FIXME: A patch is needed to llvm to remove -Wl,-z,defs since it is now recognised on emscripten. What needs to be removed is here # https://github.com/llvm/llvm-project/blob/128e2e446e90c3b1827cfc7d4d19e3c0976beff3/llvm/cmake/modules/HandleLLVMOptions.cmake#L318 . The PR to do try to do this is here # https://github.com/llvm/llvm-project/pull/123396 - set_target_properties(clangCppInterOp - PROPERTIES NO_SONAME 1 + set_target_properties(clangCppInterOp PROPERTIES + NO_SONAME 1 + LINK_FLAGS "-s WASM_BIGINT -s SIDE_MODULE=1 ${SYMBOLS_LIST}" ) - target_link_options(clangCppInterOp - PRIVATE "SHELL: -s WASM_BIGINT" - PRIVATE "SHELL: -s SIDE_MODULE=1" - PRIVATE "SHELL: ${SYMBOLS_LIST}" - ) - if (CPPINTEROP_ENABLE_TESTING) # When compiling Emscripten tests the shared library it links to is expected to be in the same folder as the compiled Javascript add_custom_command(TARGET clangCppInterOp POST_BUILD diff --git a/unittests/CppInterOp/CMakeLists.txt b/unittests/CppInterOp/CMakeLists.txt index 124fe1e58..2ffa2035d 100644 --- a/unittests/CppInterOp/CMakeLists.txt +++ b/unittests/CppInterOp/CMakeLists.txt @@ -29,11 +29,9 @@ if(EMSCRIPTEN) target_link_options(CppInterOpTests PUBLIC "SHELL: -fexceptions" PUBLIC "SHELL: -s MAIN_MODULE=1" - PUBLIC "SHELL: -s WASM=1" PUBLIC "SHELL: -s WASM_BIGINT" PUBLIC "SHELL: -s ASSERTIONS=0" PUBLIC "SHELL: -s ALLOW_MEMORY_GROWTH=1" - PUBLIC "SHELL: -s EXIT_RUNTIME=1" PUBLIC "SHELL: -s STACK_SIZE=32mb" PUBLIC "SHELL: -s INITIAL_MEMORY=128mb" PUBLIC "SHELL: --preload-file ${SYSROOT_PATH}/include@/include" @@ -75,11 +73,9 @@ if(EMSCRIPTEN) target_link_options(DynamicLibraryManagerTests PUBLIC "SHELL: -fexceptions" PUBLIC "SHELL: -s MAIN_MODULE=1" - PUBLIC "SHELL: -s WASM=1" PUBLIC "SHELL: -s WASM_BIGINT" PUBLIC "SHELL: -s ASSERTIONS=0" PUBLIC "SHELL: -s ALLOW_MEMORY_GROWTH=1" - PUBLIC "SHELL: -s EXIT_RUNTIME=1" PUBLIC "SHELL: -s STACK_SIZE=32mb" PUBLIC "SHELL: -s INITIAL_MEMORY=128mb" PUBLIC "SHELL: --preload-file ${SYSROOT_PATH}/include@/include" From 05b6c463187f101f1a14f64a902e8740eb468255 Mon Sep 17 00:00:00 2001 From: anutosh491 Date: Mon, 17 Mar 2025 18:08:38 +0530 Subject: [PATCH 4/4] Apply some suggestions --- unittests/CppInterOp/CMakeLists.txt | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/unittests/CppInterOp/CMakeLists.txt b/unittests/CppInterOp/CMakeLists.txt index 2ffa2035d..23a3ef041 100644 --- a/unittests/CppInterOp/CMakeLists.txt +++ b/unittests/CppInterOp/CMakeLists.txt @@ -25,12 +25,29 @@ add_cppinterop_unittest(CppInterOpTests ) if(EMSCRIPTEN) - + # Explanation of Emscripten-specific link flags for CppInterOpTests: + # + # MAIN_MODULE=1: + # Enables building CppInterOpTests.js as the main WebAssembly module, allowing dynamic linking of side modules. + # + # WASM_BIGINT: + # Ensures support for 64-bit integer types by enabling JavaScript BigInt integration in WASM. + # + # ALLOW_MEMORY_GROWTH=1: + # Allows the WebAssembly memory to grow dynamically at runtime to accommodate increasing memory needs. + # Would lead to an abortOnCannotGrowMemory error if memory cannot be grown while running the tests. + # + # STACK_SIZE=32mb: Allocates 32MB of stack space to handle deep recursion or large stack-allocated objects safely. + # INITIAL_MEMORY=128mb: Sets the initial linear memory size to 128MB to reduce the likelihood of early memory expansion and improve performance. + # The STACK_SIZE and INITIAL_MEMORY values are chosen based on what has been put to use for running xeus-cpp-lite. + # Check https://github.com/jupyter-xeus/xeus/blob/main/cmake/WasmBuildOptions.cmake#L35-L36 for more details. + # Not setting these flags would lead to a memory access out of bounds error while running the tests. + # + # --preload-file ${SYSROOT_PATH}/include@/include: + # Preloads the system include directory into the Emscripten virtual filesystem to make headers accessible at runtime. target_link_options(CppInterOpTests - PUBLIC "SHELL: -fexceptions" PUBLIC "SHELL: -s MAIN_MODULE=1" PUBLIC "SHELL: -s WASM_BIGINT" - PUBLIC "SHELL: -s ASSERTIONS=0" PUBLIC "SHELL: -s ALLOW_MEMORY_GROWTH=1" PUBLIC "SHELL: -s STACK_SIZE=32mb" PUBLIC "SHELL: -s INITIAL_MEMORY=128mb" @@ -69,12 +86,10 @@ target_link_libraries(DynamicLibraryManagerTests ) if(EMSCRIPTEN) - + # Check explanation of Emscripten-specific link flags for CppInterOpTests above for DynamicLibraryManagerTests as well. target_link_options(DynamicLibraryManagerTests - PUBLIC "SHELL: -fexceptions" PUBLIC "SHELL: -s MAIN_MODULE=1" PUBLIC "SHELL: -s WASM_BIGINT" - PUBLIC "SHELL: -s ASSERTIONS=0" PUBLIC "SHELL: -s ALLOW_MEMORY_GROWTH=1" PUBLIC "SHELL: -s STACK_SIZE=32mb" PUBLIC "SHELL: -s INITIAL_MEMORY=128mb"