From 2a6dacc603238fc509df577511511f735601c539 Mon Sep 17 00:00:00 2001 From: AhmedFatthy1040 Date: Thu, 28 Nov 2024 18:30:59 +0200 Subject: [PATCH 1/6] Enhance init_includes to handle dynamic include paths --- src/xinterpreter.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/xinterpreter.cpp b/src/xinterpreter.cpp index f099b688..abcec349 100644 --- a/src/xinterpreter.cpp +++ b/src/xinterpreter.cpp @@ -357,7 +357,25 @@ __get_cxx_version () void interpreter::init_includes() { + // Add the standard include path Cpp::AddIncludePath((xeus::prefix_path() + "/include/").c_str()); + + // Add non-standard include paths from XEUS_SEARCH_PATH + const char* non_standard_paths = XEUS_SEARCH_PATH; + + if (non_standard_paths && std::strlen(non_standard_paths) > 0) + { + // Split the paths by colon ':' and add each one + std::istringstream stream(non_standard_paths); + std::string path; + while (std::getline(stream, path, ':')) + { + if (!path.empty()) + { + Cpp::AddIncludePath(path.c_str()); + } + } + } } void interpreter::init_preamble() From 30504714c4121fcb2f0c23805015bc40dea1db71 Mon Sep 17 00:00:00 2001 From: AhmedFatthy1040 Date: Thu, 28 Nov 2024 18:31:10 +0200 Subject: [PATCH 2/6] Update CMakeLists to support dynamic inclusion of directories --- test/CMakeLists.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8ef68992..c3185063 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -67,3 +67,11 @@ target_link_libraries(test_xeus_cpp xeus-cpp doctest::doctest ${CMAKE_THREAD_LIB target_include_directories(test_xeus_cpp PRIVATE ${XEUS_CPP_INCLUDE_DIR}) add_custom_target(xtest COMMAND test_xeus_cpp DEPENDS test_xeus_cpp) + +set(XEUS_SEARCH_PATH $,:>) + +if (NOT EMSCRIPTEN) + set(XEUS_SEARCH_PATH "${XEUS_SEARCH_PATH}:${CMAKE_CURRENT_SOURCE_DIR}/src") +endif() + +target_compile_definitions(xeus-cpp PRIVATE "XEUS_SEARCH_PATH=\"${XEUS_SEARCH_PATH}\"") \ No newline at end of file From 71cd1ec0f0f2c873ef7c1b68be1a1dc316232d8f Mon Sep 17 00:00:00 2001 From: AhmedFatthy1040 Date: Tue, 3 Dec 2024 23:32:37 +0200 Subject: [PATCH 3/6] Switch XEUS_SEARCH_PATH from compile-time to runtime This change: 1. Replaces compile-time XEUS_SEARCH_PATH define with runtime environment variable 2. Removes CMake definition to eliminate redefinition warnings 3. Adds test to verify non-standard include path functionality 4. Makes include path configuration more flexible (can be changed without recompiling) Resolves #175 --- CMakeLists.txt | 16 ++++++++++++++++ src/xinterpreter.cpp | 9 ++++++--- test/custom_includes/test_header.hpp | 8 ++++++++ test/test_include_paths.cpp | 8 ++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 test/custom_includes/test_header.hpp create mode 100644 test/test_include_paths.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index be5d3928..bf898e82 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -50,6 +50,8 @@ message(STATUS "Building xeus-cpp v${${PROJECT_NAME}_VERSION}") # Build options # ============= +# Note: XEUS_SEARCH_PATH is now handled at runtime through environment variables + option(XEUS_CPP_BUILD_STATIC "Build xeus-cpp static library" ON) option(XEUS_CPP_BUILD_SHARED "Split xcpp build into executable and library" ON) option(XEUS_CPP_BUILD_EXECUTABLE "Build the xcpp executable" ON) @@ -404,7 +406,21 @@ endif() # ===== if(XEUS_CPP_BUILD_TESTS) + enable_testing() add_subdirectory(test) + + # Find doctest package + find_package(doctest REQUIRED) + + # Test for non-standard include paths + add_executable(test_include_paths test/test_include_paths.cpp) + target_link_libraries(test_include_paths PRIVATE doctest::doctest) + target_include_directories(test_include_paths PRIVATE + ${CMAKE_SOURCE_DIR}/test/custom_includes + ${DOCTEST_INCLUDE_DIR} + ) + target_compile_definitions(test_include_paths PRIVATE DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN) + add_test(NAME test_include_paths COMMAND test_include_paths) endif() # Installation diff --git a/src/xinterpreter.cpp b/src/xinterpreter.cpp index abcec349..8a8018f1 100644 --- a/src/xinterpreter.cpp +++ b/src/xinterpreter.cpp @@ -360,10 +360,13 @@ __get_cxx_version () // Add the standard include path Cpp::AddIncludePath((xeus::prefix_path() + "/include/").c_str()); - // Add non-standard include paths from XEUS_SEARCH_PATH - const char* non_standard_paths = XEUS_SEARCH_PATH; + // Get include paths from environment variable + const char* non_standard_paths = std::getenv("XEUS_SEARCH_PATH"); + if (!non_standard_paths) { + non_standard_paths = ""; + } - if (non_standard_paths && std::strlen(non_standard_paths) > 0) + if (std::strlen(non_standard_paths) > 0) { // Split the paths by colon ':' and add each one std::istringstream stream(non_standard_paths); diff --git a/test/custom_includes/test_header.hpp b/test/custom_includes/test_header.hpp new file mode 100644 index 00000000..ad3354b1 --- /dev/null +++ b/test/custom_includes/test_header.hpp @@ -0,0 +1,8 @@ +#ifndef TEST_HEADER_HPP +#define TEST_HEADER_HPP + +namespace test_ns { + constexpr int test_value = 42; +} + +#endif diff --git a/test/test_include_paths.cpp b/test/test_include_paths.cpp new file mode 100644 index 00000000..b27bf670 --- /dev/null +++ b/test/test_include_paths.cpp @@ -0,0 +1,8 @@ +#include +#include +#include "test_header.hpp" + +TEST_CASE("Test non-standard include paths") +{ + CHECK(test_ns::test_value == 42); +} From 5c40221e394ae8cf6aa8120123ab6d503b08dfed Mon Sep 17 00:00:00 2001 From: AhmedFatthy1040 Date: Mon, 9 Dec 2024 02:41:03 +0200 Subject: [PATCH 4/6] fix: add missing standard library includes in xinterpreter.cpp - Add for std::strlen - Add for std::istringstream - Add for std::getline Resolves include-cleaner warnings by explicitly including headers for all used standard library symbols. This improves code maintainability and prevents reliance on indirect includes. --- src/xinterpreter.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/xinterpreter.cpp b/src/xinterpreter.cpp index 8a8018f1..e5912345 100644 --- a/src/xinterpreter.cpp +++ b/src/xinterpreter.cpp @@ -15,6 +15,10 @@ #include "xeus-cpp/xinterpreter.hpp" #include "xeus-cpp/xmagics.hpp" +#include // for std::strlen +#include // for std::istringstream +#include // for std::getline + #include "xinput.hpp" #include "xinspect.hpp" #ifndef EMSCRIPTEN From bdb66eb31d86368157c51655650e362c6f73ae76 Mon Sep 17 00:00:00 2001 From: AhmedFatthy1040 Date: Mon, 9 Dec 2024 15:36:45 +0200 Subject: [PATCH 5/6] Move test_include_paths configuration to the test folder - Relocated the test_include_paths executable setup from the root CMakeLists.txt to the test folder's CMakeLists.txt. - Included the necessary doctest package configuration and non-standard include paths. - Ensured proper target linking and definitions for the test_include_paths target. --- CMakeLists.txt | 13 ------------- test/CMakeLists.txt | 10 ++++++++++ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bf898e82..41cd0da0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -408,19 +408,6 @@ endif() if(XEUS_CPP_BUILD_TESTS) enable_testing() add_subdirectory(test) - - # Find doctest package - find_package(doctest REQUIRED) - - # Test for non-standard include paths - add_executable(test_include_paths test/test_include_paths.cpp) - target_link_libraries(test_include_paths PRIVATE doctest::doctest) - target_include_directories(test_include_paths PRIVATE - ${CMAKE_SOURCE_DIR}/test/custom_includes - ${DOCTEST_INCLUDE_DIR} - ) - target_compile_definitions(test_include_paths PRIVATE DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN) - add_test(NAME test_include_paths COMMAND test_include_paths) endif() # Installation diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c3185063..e323a468 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -68,6 +68,16 @@ target_include_directories(test_xeus_cpp PRIVATE ${XEUS_CPP_INCLUDE_DIR}) add_custom_target(xtest COMMAND test_xeus_cpp DEPENDS test_xeus_cpp) +# Test for non-standard include paths +add_executable(test_include_paths test_include_paths.cpp) +target_link_libraries(test_include_paths PRIVATE doctest::doctest) +target_include_directories(test_include_paths PRIVATE + ${CMAKE_SOURCE_DIR}/test/custom_includes + ${DOCTEST_INCLUDE_DIR} +) +target_compile_definitions(test_include_paths PRIVATE DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN) +add_test(NAME test_include_paths COMMAND test_include_paths) + set(XEUS_SEARCH_PATH $,:>) if (NOT EMSCRIPTEN) From 944c962746e1ed904aff588eef53e5985ace31bc Mon Sep 17 00:00:00 2001 From: AhmedFatthy1040 Date: Thu, 19 Dec 2024 22:12:00 +0200 Subject: [PATCH 6/6] Formated the code using clang-format-18 --- src/xinterpreter.cpp | 119 ++++++++++++++++++++++++++----------------- 1 file changed, 72 insertions(+), 47 deletions(-) diff --git a/src/xinterpreter.cpp b/src/xinterpreter.cpp index e5912345..93aee3a0 100644 --- a/src/xinterpreter.cpp +++ b/src/xinterpreter.cpp @@ -15,9 +15,9 @@ #include "xeus-cpp/xinterpreter.hpp" #include "xeus-cpp/xmagics.hpp" -#include // for std::strlen -#include // for std::istringstream -#include // for std::getline +#include // for std::strlen +#include // for std::istringstream +#include // for std::getline #include "xinput.hpp" #include "xinspect.hpp" @@ -30,43 +30,62 @@ using Args = std::vector; -void* createInterpreter(const Args &ExtraArgs = {}) { - Args ClangArgs = {/*"-xc++"*/"-v"}; // ? {"-Xclang", "-emit-llvm-only", "-Xclang", "-diagnostic-log-file", "-Xclang", "-", "-xc++"}; - 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()); - } - std::vector CxxSystemIncludes; - Cpp::DetectSystemCompilerIncludePaths(CxxSystemIncludes); - for (const std::string& CxxInclude : CxxSystemIncludes) { - ClangArgs.push_back("-isystem"); - ClangArgs.push_back(CxxInclude.c_str()); - } - ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end()); - // FIXME: We should process the kernel input options and conditionally pass - // the gpu args here. - return Cpp::CreateInterpreter(ClangArgs/*, {"-cuda"}*/); +void* createInterpreter(const Args& ExtraArgs = {}) +{ + Args ClangArgs = {/*"-xc++"*/ "-v"}; // ? {"-Xclang", "-emit-llvm-only", "-Xclang", + // "-diagnostic-log-file", "-Xclang", "-", "-xc++"}; + 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()); + } + std::vector CxxSystemIncludes; + Cpp::DetectSystemCompilerIncludePaths(CxxSystemIncludes); + for (const std::string& CxxInclude : CxxSystemIncludes) + { + ClangArgs.push_back("-isystem"); + ClangArgs.push_back(CxxInclude.c_str()); + } + ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end()); + // FIXME: We should process the kernel input options and conditionally pass + // the gpu args here. + return Cpp::CreateInterpreter(ClangArgs /*, {"-cuda"}*/); } using namespace std::placeholders; namespace xcpp { - struct StreamRedirectRAII { - std::string &err; - StreamRedirectRAII(std::string &e) : err(e) { - Cpp::BeginStdStreamCapture(Cpp::kStdErr); - Cpp::BeginStdStreamCapture(Cpp::kStdOut); - } - ~StreamRedirectRAII() { - std::string out = Cpp::EndStdStreamCapture(); - err = Cpp::EndStdStreamCapture(); - std::cout << out; - } + struct StreamRedirectRAII + { + std::string& err; + + StreamRedirectRAII(std::string& e) + : err(e) + { + Cpp::BeginStdStreamCapture(Cpp::kStdErr); + Cpp::BeginStdStreamCapture(Cpp::kStdOut); + } + + ~StreamRedirectRAII() + { + std::string out = Cpp::EndStdStreamCapture(); + err = Cpp::EndStdStreamCapture(); + std::cout << out; + } }; void interpreter::configure_impl() @@ -102,15 +121,14 @@ __get_cxx_version () return std::to_string(cxx_version); } - - interpreter::interpreter(int argc, const char* const* argv) : - xmagics() + interpreter::interpreter(int argc, const char* const* argv) + : xmagics() , p_cout_strbuf(nullptr) , p_cerr_strbuf(nullptr) , m_cout_buffer(std::bind(&interpreter::publish_stdout, this, _1)) , m_cerr_buffer(std::bind(&interpreter::publish_stderr, this, _1)) { - //NOLINTNEXTLINE (cppcoreguidelines-pro-bounds-pointer-arithmetic) + // NOLINTNEXTLINE (cppcoreguidelines-pro-bounds-pointer-arithmetic) createInterpreter(Args(argv ? argv + 1 : argv, argv + argc)); m_version = get_stdopt(); redirect_output(); @@ -214,10 +232,11 @@ __get_cxx_version () // // JupyterLab displays the "{ename}: {evalue}" if the traceback is // empty. - if (evalue.size() < 4) { + if (evalue.size() < 4) + { ename = " "; } - std::vector traceback({ename + evalue}); + std::vector traceback({ename + evalue}); if (!config.silent) { publish_execution_error(ename, evalue, traceback); @@ -260,7 +279,8 @@ __get_cxx_version () Cpp::CodeComplete(results, code.c_str(), 1, _cursor_pos + 1); - return xeus::create_complete_reply(results /*matches*/, + return xeus::create_complete_reply( + results /*matches*/, cursor_pos - to_complete.length() /*cursor_start*/, cursor_pos /*cursor_end*/ ); @@ -281,13 +301,17 @@ __get_cxx_version () nl::json interpreter::is_complete_request_impl(const std::string& code) { - if (!code.empty() && code[code.size() - 1] == '\\') { + if (!code.empty() && code[code.size() - 1] == '\\') + { auto found = code.rfind('\n'); if (found == std::string::npos) + { found = -1; + } auto found1 = found++; - while (isspace(code[++found1])) ; - return xeus::create_is_complete_reply("incomplete", code.substr(found, found1-found)); + while (isspace(code[++found1])) + ; + return xeus::create_is_complete_reply("incomplete", code.substr(found, found1 - found)); } return xeus::create_is_complete_reply("complete"); @@ -366,7 +390,8 @@ __get_cxx_version () // Get include paths from environment variable const char* non_standard_paths = std::getenv("XEUS_SEARCH_PATH"); - if (!non_standard_paths) { + if (!non_standard_paths) + { non_standard_paths = ""; } @@ -387,11 +412,11 @@ __get_cxx_version () void interpreter::init_preamble() { - //NOLINTBEGIN(cppcoreguidelines-owning-memory) + // NOLINTBEGIN(cppcoreguidelines-owning-memory) preamble_manager.register_preamble("introspection", std::make_unique()); preamble_manager.register_preamble("magics", std::make_unique()); preamble_manager.register_preamble("shell", std::make_unique()); - //NOLINTEND(cppcoreguidelines-owning-memory) + // NOLINTEND(cppcoreguidelines-owning-memory) } void interpreter::init_magic()