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

Attempt to process cell without semicolon #152

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
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
79 changes: 43 additions & 36 deletions src/xinterpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "xinput.hpp"
#include "xinspect.hpp"
#include "xmagics/os.hpp"
#include "xmime_internal.hpp"
#include "xparser.hpp"
#include "xsystem.hpp"

Expand Down Expand Up @@ -128,17 +129,14 @@ __get_cxx_version ()

void interpreter::execute_request_impl(
send_reply_callback cb,
int /*execution_count*/,
int execution_counter,
const std::string& code,
xeus::execute_request_config config,
nl::json /*user_expressions*/
)
{
nl::json kernel_res;


auto input_guard = input_redirection(config.allow_stdin);

// Check for magics
for (auto& pre : preamble_manager.preamble)
{
Expand All @@ -150,10 +148,15 @@ __get_cxx_version ()
}
}

// Split code from includes
auto blocks = split_from_includes(code);

auto errorlevel = 0;

std::string ename;
std::string evalue;
bool compilation_result = false;
intptr_t output_value = 0;
bool hadError = false;

// If silent is set to true, temporarily dismiss all std::cerr and
// std::cout outputs resulting from `process_code`.
Expand All @@ -170,30 +173,36 @@ __get_cxx_version ()

std::string err;

// Attempt normal evaluation
try
{
StreamRedirectRAII R(err);
compilation_result = Cpp::Process(code.c_str());
}
catch (std::exception& e)
{
errorlevel = 1;
ename = "Standard Exception: ";
evalue = e.what();
}
catch (...)
{
errorlevel = 1;
ename = "Error: ";
}
// Scope guard performing the temporary redirection of input requests.
auto input_guard = input_redirection(config.allow_stdin);

if (compilation_result)
for (const auto& block : blocks)
{
errorlevel = 1;
ename = "Error: ";
evalue = "Compilation error! " + err;
std::cerr << err;
// Attempt normal evaluation
try
{
StreamRedirectRAII R(err);
output_value = Cpp::Evaluate(block.c_str(), &hadError);
}
catch (std::exception& e)
{
errorlevel = 1;
ename = "Standard Exception: ";
evalue = e.what();
}
catch (...)
{
errorlevel = 1;
ename = "Error: ";
}

if (hadError)
{
errorlevel = 1;
ename = "Error: ";
evalue = "Compilation error! " + err;
std::cerr << err;
}
}

// Flush streams
Expand Down Expand Up @@ -233,15 +242,13 @@ __get_cxx_version ()
}
else
{
/*
// Publish a mime bundle for the last return value if
// the semicolon was omitted.
if (!config.silent && output.hasValue() && trim(code).back() != ';')
{
nl::json pub_data = mime_repr(output);
publish_execution_result(execution_counter, std::move(pub_data), nl::json::object());
}
*/
// Publish a mime bundle for the last return value if
// the semicolon was omitted.
if (!config.silent && output_value != 0 && trim(blocks.back()).back() != ';')
{
nl::json pub_data = mime_repr(output_value);
anutosh491 marked this conversation as resolved.
Show resolved Hide resolved
publish_execution_result(execution_counter, std::move(pub_data), nl::json::object());
}
// Compose execute_reply message.
kernel_res["status"] = "ok";
kernel_res["payload"] = nl::json::array();
Expand Down
67 changes: 67 additions & 0 deletions src/xmime_internal.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/************************************************************************************
* Copyright (c) 2016, Johan Mabille, Loic Gouarin, Sylvain Corlay, Wolf Vollprecht *
* Copyright (c) 2016, QuantStack *
* *
* Distributed under the terms of the BSD 3-Clause License. *
* *
* The full license is in the file LICENSE, distributed with this software. *
************************************************************************************/

#ifndef XCPP_MIME_INTERNAL_HPP
#define XCPP_MIME_INTERNAL_HPP
Copy link
Contributor

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]

Suggested change
#define XCPP_MIME_INTERNAL_HPP
#ifndef GITHUB_WORKSPACE_SRC_XMIME_INTERNAL_HPP
#define GITHUB_WORKSPACE_SRC_XMIME_INTERNAL_HPP


#include <cstddef>
#include <locale>
#include <string>

#include "clang/Interpreter/CppInterOp.h" // from CppInterOp package

#include <nlohmann/json.hpp>

namespace nl = nlohmann;

namespace xcpp
{
inline nl::json mime_repr(intptr_t V)
{
// Return a JSON mime bundle representing the specified value.
void* value_ptr = reinterpret_cast<void*>(V);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]

        void* value_ptr = reinterpret_cast<void*>(V);
                          ^

Copy link
Contributor

Choose a reason for hiding this comment

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

warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]

        void* value_ptr = reinterpret_cast<void*>(V);
                          ^


// Include "xcpp/xmime.hpp" only on the first time a variable is displayed.
static bool xmime_included = false;

if (!xmime_included)
{
Cpp::Declare("#include \"xcpp/xmime.hpp\"");
xmime_included = true;
}

intptr_t mimeReprV;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable 'mimeReprV' is not initialized [cppcoreguidelines-init-variables]

Suggested change
intptr_t mimeReprV;
intptr_t mimeReprV = 0;

bool hadError = false;
{
std::ostringstream code;
code << "using xcpp::mime_bundle_repr;";
code << "mime_bundle_repr(";
// code << "*(" << getTypeAsString(V);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Commented out.

code << &value_ptr;
code << "));";

std::string codeString = code.str();

mimeReprV = Cpp::Evaluate(codeString.c_str(), &hadError);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: Value stored to 'mimeReprV' is never read [clang-analyzer-deadcode.DeadStores]

            mimeReprV = Cpp::Evaluate(codeString.c_str(), &hadError);
            ^
Additional context

src/xmime_internal.hpp:50: Value stored to 'mimeReprV' is never read

            mimeReprV = Cpp::Evaluate(codeString.c_str(), &hadError);
            ^

}

void* mimeReprV_ptr = reinterpret_cast<void*>(V);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]

        void* mimeReprV_ptr = reinterpret_cast<void*>(V);
                              ^

Copy link
Contributor

Choose a reason for hiding this comment

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

warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]

        void* mimeReprV_ptr = reinterpret_cast<void*>(V);
                              ^


if (!hadError && mimeReprV_ptr)
{
return *(nl::json*) mimeReprV_ptr;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

            return *(nl::json*) mimeReprV_ptr;
                    ^

}
else
{
return nl::json::object();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use 'else' after 'return' [llvm-else-after-return]

Suggested change
}
} return nl::json::object();

Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use 'else' after 'return' [llvm-else-after-return]

Suggested change
}
return nl::json::object();

}
}
anutosh491 marked this conversation as resolved.
Show resolved Hide resolved

#endif
76 changes: 76 additions & 0 deletions src/xparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,80 @@ namespace xcpp

return result;
}

std::vector<std::string> get_lines(const std::string& input)
{
std::vector<std::string> lines;
std::regex re("\\n");

std::copy(
std::sregex_token_iterator(input.begin(), input.end(), re, -1),
std::sregex_token_iterator(),
std::back_inserter(lines)
);
return lines;
}

std::vector<std::string> split_from_includes(const std::string& input)
{
// this function split the input into part where we have only #include.

// split input into lines
std::vector<std::string> lines = get_lines(input);

// check if each line contains #include and concatenate the result in the good part of the result
std::regex incl_re("\\#include.*");
std::regex magic_re("^\\%\\w+");
std::vector<std::string> result;
result.push_back("");
anutosh491 marked this conversation as resolved.
Show resolved Hide resolved
std::size_t current = 0; // 0 include, 1 other
std::size_t rindex = 0; // current index of result vector
for (std::size_t i = 0; i < lines.size(); ++i)
{
if (!lines[i].empty())
{
if (std::regex_search(lines[i], magic_re))
{
result.push_back(lines[i] + "\n");
result.push_back("");
anutosh491 marked this conversation as resolved.
Show resolved Hide resolved
rindex += 2;
}
else
{
if (std::regex_match(lines[i], incl_re))
{
// if we have #include in this line
// but the current item of result vector contains
// other things
if (current != 0)
{
current = 0;
result.push_back("");
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use emplace_back instead of push_back [modernize-use-emplace]

Suggested change
result.push_back("");
result.emplace_back("");

rindex++;
}
}
else
{
// if we don't have #include in this line
// but the current item of result vector contains
// the include parts
if (current != 1)
{
current = 1;
result.push_back("");
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: use emplace_back instead of push_back [modernize-use-emplace]

Suggested change
result.push_back("");
result.emplace_back("");

rindex++;
}
}
// if we have multiple lines, we add a semicolon at the end of the lines that not contain
// #include keyword (except for the last line)
result[rindex] += lines[i];
if (i != lines.size() - 1)
{
result[rindex] += "\n";
}
}
}
}
return result;
}
}
6 changes: 6 additions & 0 deletions src/xparser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,11 @@ namespace xcpp

XEUS_CPP_API std::vector<std::string>
split_line(const std::string& input, const std::string& delims, std::size_t cursor_pos);

XEUS_CPP_API
std::vector<std::string> get_lines(const std::string& input);

XEUS_CPP_API
std::vector<std::string> split_from_includes(const std::string& input);
}
#endif