Merge branch 'main' into tagfiles-lite

Author: kr-2003

CMakeLists.txt

@@ -148,6 +148,7 @@ function(configure_native_kernel kernel)
 endfunction()
 
 function(configure_wasm_kernel kernel)
+  set(XEUS_CPP_RESOURCE_DIR "/lib/clang/${CPPINTEROP_LLVM_VERSION_MAJOR}" PARENT_SCOPE)
 
   configure_file (
     "${CMAKE_CURRENT_SOURCE_DIR}${kernel}wasm_kernel.json.in"
@@ -169,17 +170,9 @@ endfunction()
 
 message("Configure kernels: ...")
 if(EMSCRIPTEN)
-    # TODO: Currently jupyterlite-xeus and xeus-lite do not provide
-    # methods to fetch information from the arguments present in the
-    # generated emscripten kernel.
-    # The following needs to be done here :
-    # 1) We need to configure the kernel properly
-    #    Check issue https://github.com/compiler-research/xeus-cpp/issues/185.
-    # 2) Once the above is done we need to add support in jupyterlite-xeus & xeus-lite
-    #    to be able to deal with arguments present in kernel.json
-    # 3) Finally we should fetch the C++ version from the kernel.json file and
-    #    be able to pass it to our wasm interpreter rather than forcing a version.
+    configure_wasm_kernel("/share/jupyter/kernels/xcpp17/")
     configure_wasm_kernel("/share/jupyter/kernels/xcpp20/")
+    configure_wasm_kernel("/share/jupyter/kernels/xcpp23/")
 else()
     configure_native_kernel("/share/jupyter/kernels/xcpp17/")
     configure_native_kernel("/share/jupyter/kernels/xcpp20/")
@@ -448,16 +441,8 @@ if(EMSCRIPTEN)
         PUBLIC "SHELL: --preload-file ${XCPP_TAGCONFS_DIR}@/etc/xeus-cpp/tags.d"
         PUBLIC "SHELL: --post-js ${CMAKE_CURRENT_SOURCE_DIR}/wasm_patches/post.js"
     )
-    # TODO: Emscripten supports preloading files just once before it generates
-    # the xcpp.data file (containing the binary representation of the file(s) we
-    # want to include in our application).
-    # Hence although we are adding support for Standard Headers, Libraries etc
-    # through emscripten's sysroot for now, we need to do the following:
-    # 1) Enable CppInterOp to provide us with a resource dir.
-    # 2) If the above cannot be done, we can use the resource dir provided
-    #    by llvm on emscripten-forge but would involve adding a dependency.
-    # 3) Shift the resource dir and the sysroot to a common location.
-    # 4) Preload everything required together.
+    # TODO: Uncomment the above line regarding preloading clang's resource dir
+    # once has been supported through cppinterop.
 endif()
 # Tests
 # =====

include/xeus-cpp/xinterpreter_wasm.hpp

@@ -19,7 +19,7 @@ namespace xcpp
     {
     public:
 
-        wasm_interpreter();
+        wasm_interpreter(int argc = 0, char** argv = nullptr);
         virtual ~wasm_interpreter() = default;
 
     };

include/xeus-cpp/xutils.hpp

@@ -24,9 +24,6 @@ namespace xcpp
     XEUS_CPP_API
     void stop_handler(int sig);
 
-    XEUS_CPP_API
-    interpreter_ptr build_interpreter(int argc, char** argv);
-
     XEUS_CPP_API
     std::string retrieve_tagconf_dir();
 

share/jupyter/kernels/xcpp17/wasm_kernel.json.in

@@ -0,0 +1,15 @@
+{
+  "display_name": "C++17",
+  "argv": [
+      "@XEUS_CPP_KERNELSPEC_PATH@xcpp",
+      "-resource-dir", "@XEUS_CPP_RESOURCE_DIR@",
+      "-std=c++17"
+  ],
+  "language": "cpp",
+  "metadata": {
+    "debugger": false,
+    "shared": {
+      "libclangCppInterOp.so": "lib/libclangCppInterOp.so"
+    }
+  }
+}

share/jupyter/kernels/xcpp20/wasm_kernel.json.in

@@ -2,8 +2,7 @@
   "display_name": "C++20",
   "argv": [
       "@XEUS_CPP_KERNELSPEC_PATH@xcpp",
-      "-f",
-      "{connection_file}",
+      "-resource-dir", "@XEUS_CPP_RESOURCE_DIR@",
       "-std=c++20"
   ],
   "language": "cpp",

share/jupyter/kernels/xcpp23/wasm_kernel.json.in

@@ -0,0 +1,15 @@
+{
+  "display_name": "C++23",
+  "argv": [
+      "@XEUS_CPP_KERNELSPEC_PATH@xcpp",
+      "-resource-dir", "@XEUS_CPP_RESOURCE_DIR@",
+      "-std=c++23"
+  ],
+  "language": "cpp",
+  "metadata": {
+    "debugger": false,
+    "shared": {
+      "libclangCppInterOp.so": "lib/libclangCppInterOp.so"
+    }
+  }
+}

src/main.cpp

@@ -59,7 +59,7 @@ int main(int argc, char* argv[])
     signal(SIGINT, xcpp::stop_handler);
 
     std::string file_name = xeus::extract_filename(argc, argv);
-    interpreter_ptr interpreter = xcpp::build_interpreter(argc, argv);
+    auto interpreter = std::make_unique<xcpp::interpreter>(argc, argv);
     std::unique_ptr<xeus::xcontext> context = xeus::make_zmq_context();
 
     if (!file_name.empty())

src/main_emscripten_kernel.cpp

@@ -15,9 +15,30 @@
 
 #include "xeus-cpp/xinterpreter_wasm.hpp"
 
+template <class interpreter_type>
+static interpreter_type* builder_with_args(emscripten::val js_args)
+{
+    // TODO: Refactor interpreter constructor to avoid static args-to-argv conversion.
+    static std::vector<std::string> args = emscripten::vecFromJSArray<std::string>(js_args);
+    static std::vector<const char*> argv_vec;
+
+    for (const auto& s : args)
+    {
+        argv_vec.push_back(s.c_str());
+    }
+
+    int argc = static_cast<int>(argv_vec.size());
+    char** argv = const_cast<char**>(argv_vec.data());
+
+    auto* res = new interpreter_type(argc, argv);
+    argv_vec.clear();
+    args.clear();
+    return res;
+}
+
 EMSCRIPTEN_BINDINGS(my_module)
 {
     xeus::export_core();
     using interpreter_type = xcpp::wasm_interpreter;
-    xeus::export_kernel<interpreter_type>("xkernel");
+    xeus::export_kernel<interpreter_type, &builder_with_args<interpreter_type>>("xkernel");
 }

src/xinterpreter.cpp

@@ -18,6 +18,7 @@
 #include "xinput.hpp"
 #include "xinspect.hpp"
 #include "xmagics/os.hpp"
+#include <iostream>
 #ifndef EMSCRIPTEN
 #include "xmagics/xassist.hpp"
 #endif
@@ -27,25 +28,23 @@
 using Args = std::vector<const char*>;
 
 void* createInterpreter(const Args &ExtraArgs = {}) {
-  Args ClangArgs = {/*"-xc++"*/"-v"}; // ? {"-Xclang", "-emit-llvm-only", "-Xclang", "-diagnostic-log-file", "-Xclang", "-", "-xc++"};
-#ifdef EMSCRIPTEN
-  ClangArgs.push_back("-std=c++20");
-#else
+  Args ClangArgs = {/*"-xc++"*/"-v"};
   if (std::find_if(ExtraArgs.begin(), ExtraArgs.end(), [](const std::string& s) {
     return s == "-resource-dir";}) == ExtraArgs.end()) {
     std::string resource_dir = Cpp::DetectResourceDir();
-    if (resource_dir.empty())
-      std::cerr << "Failed to detect the resource-dir\n";
-    ClangArgs.push_back("-resource-dir");
-    ClangArgs.push_back(resource_dir.c_str());
+    if (!resource_dir.empty()) {
+        ClangArgs.push_back("-resource-dir");
+        ClangArgs.push_back(resource_dir.c_str());
+    } else {
+        std::cerr << "Failed to detect the resource-dir\n";
+    }
   }
   std::vector<std::string> CxxSystemIncludes;
   Cpp::DetectSystemCompilerIncludePaths(CxxSystemIncludes);
   for (const std::string& CxxInclude : CxxSystemIncludes) {
     ClangArgs.push_back("-isystem");
     ClangArgs.push_back(CxxInclude.c_str());
   }
-#endif
   ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end());
   // FIXME: We should process the kernel input options and conditionally pass
   // the gpu args here.

src/xinterpreter_wasm.cpp

@@ -15,9 +15,8 @@
 
 namespace xcpp
 {
-
-    wasm_interpreter::wasm_interpreter()
-        : interpreter(0, nullptr)
+    wasm_interpreter::wasm_interpreter(int argc, char** argv)
+        : interpreter(argc, argv)
     {
     }
 }

src/xutils.cpp

@@ -50,21 +50,6 @@ namespace xcpp
         exit(0);
     }
 
-    interpreter_ptr build_interpreter(int argc, char** argv)
-    {
-        std::vector<const char*> interpreter_argv(argc);
-        interpreter_argv[0] = "xeus-cpp";
-
-        // Copy all arguments in the new vector excepting the process name.
-        for (int i = 1; i < argc; i++)
-        {
-            interpreter_argv[i] = argv[i];
-        }
-
-        interpreter_ptr interp_ptr = std::make_unique<interpreter>(argc, interpreter_argv.data());
-        return interp_ptr;
-    }
-
     std::string retrieve_tagconf_dir()
     {
         const char* tagconf_dir_env = std::getenv("XCPP_TAGCONFS_DIR");

test/test_interpreter.cpp

@@ -344,24 +344,6 @@ TEST_SUITE("trim"){
 
 }
 
-TEST_SUITE("build_interpreter")
-{
-    // This test case checks if the function `build_interpreter` returns a non-null pointer
-    // when valid arguments are passed. It sets up a scenario with valid command line arguments
-    // and checks if the function returns a non-null pointer.
-    TEST_CASE("build_interpreter_pointer_not_null")
-    {
-        char arg1[] = "program_name";
-        char arg2[] = "-option1";
-        char* argv[] = {arg1, arg2};
-        int argc = 2;
-
-        interpreter_ptr interp_ptr = xcpp::build_interpreter(argc, argv);
-
-        REQUIRE(interp_ptr != nullptr);
-    }
-}
-
 TEST_SUITE("is_match_magics_manager")
 {
     // This test case checks if the function `is_match` correctly identifies strings that match