-
Notifications
You must be signed in to change notification settings - Fork 36
implement sub-interpreters #380
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
base: main
Are you sure you want to change the base?
implement sub-interpreters #380
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There were too many comments to post at once. Showing the first 10 out of 12. Check the log or trigger a new build to see more.
#include "xsystem.hpp" | ||
|
||
#include "xmagics/multi_interpreter.hpp" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: included header multi_interpreter.hpp is not used directly [misc-include-cleaner]
#include "multi_interpreter.hpp" | ||
|
||
#include <iostream> | ||
#include <iterator> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: included header iostream is not used directly [misc-include-cleaner]
#include <iterator> | |
#include <iterator> |
|
||
#include <iostream> | ||
#include <iterator> | ||
#include <sstream> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: included header iterator is not used directly [misc-include-cleaner]
#include <sstream> | |
#include <sstream> |
#include <sstream> | ||
#include <string> | ||
#include <vector> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: included header vector is not used directly [misc-include-cleaner]
|
||
static std::vector<std::string> split(const std::string& input) | ||
{ | ||
std::istringstream buffer(input); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: unused local variable 'buffer' of type 'std::istringstream' (aka 'basic_istringstream') [bugprone-unused-local-non-trivial-variable]
std::istringstream buffer(input);
^
return ret; | ||
} | ||
|
||
static constexpr size_t len(std::string_view n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: no header providing "size_t" is directly included [misc-include-cleaner]
src/xmagics/multi_interpreter.cpp:10:
- #include <iostream>
+ #include <cstddef>
+ #include <iostream>
return ret; | ||
} | ||
|
||
static constexpr size_t len(std::string_view n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: no header providing "std::string_view" is directly included [misc-include-cleaner]
src/xmagics/multi_interpreter.cpp:14:
- #include <vector>
+ #include <string_view>
+ #include <vector>
************************************************************************************/ | ||
|
||
#ifndef XEUS_CPP_MULTI_INTERPRETER_MAGIC_HPP | ||
#define XEUS_CPP_MULTI_INTERPRETER_MAGIC_HPP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: header guard does not follow preferred style [llvm-header-guard]
#define XEUS_CPP_MULTI_INTERPRETER_MAGIC_HPP | |
#ifndef GITHUB_WORKSPACE_SRC_XMAGICS_MULTI_INTERPRETER_HPP | |
#define GITHUB_WORKSPACE_SRC_XMAGICS_MULTI_INTERPRETER_HPP |
src/xmagics/multi_interpreter.hpp:32:
- #endif // XEUS_CPP_MULTI_INTERPRETER_MAGIC_HPP
+ #endif // GITHUB_WORKSPACE_SRC_XMAGICS_MULTI_INTERPRETER_HPP
|
||
#include <string> | ||
#include <unordered_map> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: included header unordered_map is not used directly [misc-include-cleaner]
#include <string> | ||
#include <unordered_map> | ||
|
||
#include "CppInterOp/CppInterOp.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'CppInterOp/CppInterOp.h' file not found [clang-diagnostic-error]
#include "CppInterOp/CppInterOp.h"
^
b20deed
to
63271f0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
{ | ||
public: | ||
|
||
XEUS_CPP_API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: no header providing "XEUS_CPP_API" is directly included [misc-include-cleaner]
src/xmagics/multi_interpreter.hpp:15:
- #include "xeus-cpp/xmagics.hpp"
+ #include "xeus-cpp/xeus_cpp_config.hpp"
+ #include "xeus-cpp/xmagics.hpp"
|
||
private: | ||
|
||
std::unordered_map<std::string, Cpp::TInterp_t> interpreters; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for private member 'interpreters' [readability-identifier-naming]
std::unordered_map<std::string, Cpp::TInterp_t> interpreters; | |
std::unordered_map<std::string, Cpp::TInterp_t> m_interpreters; |
Please add some tests to this PR |
Description
Implements a way to create sub-interpreter and use them with in a notebook session.
Fixes # (issue)
Type of change
Please tick all options which are relevant.