Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip computing resource dir and include paths for xeus-cpp-lite #183

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/xinterpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ 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++"};
#ifndef EMSCRIPTEN
if (std::find_if(ExtraArgs.begin(), ExtraArgs.end(), [](const std::string& s) {
return s == "-resource-dir";}) == ExtraArgs.end()) {
std::string resource_dir = Cpp::DetectResourceDir();
Expand All @@ -42,6 +43,7 @@ void* createInterpreter(const Args &ExtraArgs = {}) {
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.
Expand Down Expand Up @@ -357,7 +359,9 @@ __get_cxx_version ()

void interpreter::init_includes()
{
#ifndef EMSCRIPTEN
Cpp::AddIncludePath((xeus::prefix_path() + "/include/").c_str());
Copy link
Contributor

Choose a reason for hiding this comment

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

Does that hurt on emscripten? I'd think it's just some random non-existing folder?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah random non existing folder that won't contribute to our solution :)

If you look at Anubhab's solution (https://wasmdemo.argentite.me/) or any other solutions that interest us (https://binji.github.io/wasm-clang/) we are only interested in two folders is what I can see

  1. include ( for /include/c++/v1)
  2. include/wasm32-emscripten (for /include/wasm32-emscripten/c++/v1)
// 1st link
"" -cc1 -triple wasm32-unknown-emscripten -emit-obj -mrelax-all -disable-free -clear-ast-before-backend -main-file-name "<<< inputs >>>" -mrelocation-model static -mframe-pointer=none -ffp-contract=on -fno-rounding-math -mconstructor-aliases -target-cpu generic -fvisibility=hidden -debugger-tuning=gdb -v -fcoverage-compilation-dir=/ -resource-dir lib/clang/17 -internal-isystem /include/wasm32-emscripten/c++/v1 -internal-isystem /include/c++/v1 -internal-isystem lib/clang/17/include -internal-isystem /include/wasm32-emscripten -internal-isystem /include -std=c++17 -fdeprecated-macro -fdebug-compilation-dir=/ -ferror-limit 19 -fgnuc-version=4.2.1 -fcxx-exceptions -fexceptions -fincremental-extensions -o "<<< inputs >>>.o" -x c++ "<<< inputs >>>"
clang -cc1 version 17.0.0 based upon LLVM 17.0.0git default target wasm32-unknown-emscripten
ignoring nonexistent directory "/include/wasm32-emscripten/c++/v1"
ignoring nonexistent directory "lib/clang/17/include"
ignoring nonexistent directory "/include/wasm32-emscripten"
#include "..." search starts here:
#include <...> search starts here:
/include/c++/v1
/include

// 2nd link
clang -cc1 -emit-obj -disable-free -isysroot / -internal-isystem /include/c++/v1 -internal-isystem /include -internal-isystem /lib/clang/8.0.1/include -ferror-limit 19 -fmessage-length 80 -fcolor-diagnostics -O2 -o test.o -x c++ test.cc

We eventually need to fetch anything we are interested in from one of the two folders and that is what --preload-file would do for us !

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Anyways the emscripten Ci would fail for now cause I am playing around with how the shared build for clangCppInteroP would work on emscripten-forge. As I mentioned on discord I wrote my thought on what should be done for cppinterop's emscripten build but I haven't heard anything back on it. Obviously we need to decide on a static build vs a shared build.

Copy link
Contributor

Choose a reason for hiding this comment

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

The question is does not having ifdefs around that code cause a failure on emscripten?

#endif
}

void interpreter::init_preamble()
Expand Down