Skip to content

[wip] Implementing debugger in xeus-cpp(Prototype)#354

Draft
kr-2003 wants to merge 23 commits intocompiler-research:mainfrom
kr-2003:jit-oop-exec
Draft

[wip] Implementing debugger in xeus-cpp(Prototype)#354
kr-2003 wants to merge 23 commits intocompiler-research:mainfrom
kr-2003:jit-oop-exec

Conversation

@kr-2003
Copy link
Copy Markdown
Contributor

@kr-2003 kr-2003 commented Jun 19, 2025

Description

Please include a summary of changes, motivation and context for this PR.

Fixes # (issue)

Type of change

Please tick all options which are relevant.

  • Bug fix
  • New feature
  • Added/removed dependencies
  • Required documentation updates

@kr-2003 kr-2003 marked this pull request as draft June 19, 2025 15:33
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a 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 122. Check the log or trigger a new build to see more.

Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a 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 103. Check the log or trigger a new build to see more.

Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp Outdated
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xdebugger.hpp
Comment thread include/xeus-cpp/xinterpreter.hpp
Comment thread src/main.cpp
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 22.33010% with 80 lines in your changes missing coverage. Please review.

Project coverage is 77.15%. Comparing base (951e670) to head (3c6be4e).

Files with missing lines Patch % Lines
src/xinterpreter.cpp 24.44% 68 Missing ⚠️
src/main.cpp 7.69% 12 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #354      +/-   ##
==========================================
- Coverage   81.78%   77.15%   -4.64%     
==========================================
  Files          20       20              
  Lines         950     1011      +61     
  Branches       87       94       +7     
==========================================
+ Hits          777      780       +3     
- Misses        173      231      +58     
Files with missing lines Coverage Δ
src/main.cpp 47.05% <7.69%> (+6.38%) ⬆️
src/xinterpreter.cpp 66.53% <24.44%> (-23.58%) ⬇️
Files with missing lines Coverage Δ
src/main.cpp 47.05% <7.69%> (+6.38%) ⬆️
src/xinterpreter.cpp 66.53% <24.44%> (-23.58%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a 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 93. Check the log or trigger a new build to see more.

Comment thread src/main.cpp
Comment thread src/main.cpp
Comment thread src/main.cpp
Comment thread src/main.cpp
Comment thread src/xdebugger.cpp
Comment thread src/xdebugger.cpp
Comment thread src/xdebugger.cpp
Comment thread src/xdebugger.cpp
Comment thread src/xdebugger.cpp
Comment thread src/xdebugger.cpp
Copy link
Copy Markdown
Collaborator

@Vipul-Cariappa Vipul-Cariappa left a comment

Choose a reason for hiding this comment

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

I have put some comments. We can discuss this in detail during the meeting today.

Comment thread CMakeLists.txt Outdated
Comment on lines +568 to +572
if(XEUS_CPP_USE_DEBUGGER)
add_definitions(
-DXEUS_CPP_USE_DEBUGGER
)
endif() No newline at end of file
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not sure if this is a good idea. I need to know the purpose.

Comment thread include/xeus-cpp/xdebugger.hpp Outdated
Comment on lines +66 to +67
std::string m_lldb_port;
std::string m_lldbdap_port;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is the difference between m_lldb_port & m_lldbdap_port?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks like m_lldbdap_port is unused?

Comment thread src/xdebugger.cpp
Comment thread src/main.cpp Outdated
Comment on lines +63 to +68
#ifdef XEUS_CPP_USE_DEBUGGER
nl::json debugger_config;
debugger_config["lldb"]["initCommands"] = {
"settings set plugin.jit-loader.gdb.enable on"
};
#endif
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This does not look correct. The use of ifdef. Let's discuss it in the meeting.
We need to do this based on the metadata of the kernel.json.

Comment thread src/xdebugger.cpp
Comment on lines +96 to +102
int fd = open(log_file.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0644);
if (fd != -1)
{
dup2(fd, STDOUT_FILENO);
dup2(fd, STDERR_FILENO);
close(fd);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Need some explanation here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This code is used to run lldb-dap and redirect its output to a file. might be useful for debugging

Comment thread src/xdebugger.cpp Outdated
<< std::endl;
nl::json reply = {
{"type", "response"},
{"request_seq", message["seq"]},
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is request_seq and message["seq"]?

Comment thread src/xdebugger.cpp Outdated
Comment on lines +194 to +201
{{"variables",
{{{"name", "a"}, {"value", "100"}, {"type", "int"}, {"evaluateName", "a"}, {"variablesReference", 0}},
{{"name", "b"},
{"value", "1000"},
{"type", "int"},
{"evaluateName", "b"},
{"variablesReference", 0}}}}}}
};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see that this is not yet implemented.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, its the major feature to discuss and work upon

Comment thread src/xdebugger.cpp
Comment on lines +262 to +270
std::string debugger::get_cell_temporary_file(const std::string& code) const
{
// Placeholder: Return a dummy temporary file path
std::string tmp_file = get_tmp_prefix() + "/cell_tmp.cpp";
std::ofstream out(tmp_file);
out << code;
out.close();
return tmp_file;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hopefully, we don't need this.

Comment thread src/xinternal_utils.cpp Outdated
Comment on lines +56 to +61
std::string highlight(const std::string& code)
{
// Placeholder: No syntax highlighting implemented
// In a real implementation, use a C++ library (e.g., libclang or a custom highlighter)
return code;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How and where do you plan to use this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

highlighting lines when breakpoint hit, step ins and outs

Comment thread src/xinterpreter.cpp Outdated
restore_output();
}

nl::json interpreter::internal_request_impl(const nl::json& content)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do we have this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah, i forgot to remove it. i was using previously. no need of this now.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a 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 99. Check the log or trigger a new build to see more.

#include "nlohmann/json.hpp"
#include "xeus-zmq/xdebugger_base.hpp"
#include "xeus_cpp_config.hpp"
#include "xeus-cpp/xinterpreter.hpp"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: #includes are not sorted properly [llvm-include-order]

Suggested change
#include "xeus-cpp/xinterpreter.hpp"
#include "xeus-cpp/xinterpreter.hpp"
#include "xeus-zmq/xdebugger_base.hpp"
#include "xeus_cpp_config.hpp"

bool start() override;
void stop() override;
xeus::xdebugger_info get_debugger_info() const override;
virtual std::string get_cell_temporary_file(const std::string& code) const override;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: 'virtual' is redundant since the function is already declared 'override' [cppcoreguidelines-explicit-virtual-functions]

Suggested change
virtual std::string get_cell_temporary_file(const std::string& code) const override;
std::string get_cell_temporary_file(const std::string& code) const override;

bool m_tcp_connected;
pid_t jit_process_pid;

using breakpoint_list_t = std::map<std::string, std::vector<nl::json>>;
Copy link
Copy Markdown
Contributor

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::vector" is directly included [misc-include-cleaner]

include/xeus-cpp/xdebugger.hpp:12:

- #ifdef __GNUC__
+ #include <vector>
+ #ifdef __GNUC__


xcpp::interpreter* m_interpreter;

std::unordered_map<std::string, std::vector<std::string>> m_hash_to_sequential;
Copy link
Copy Markdown
Contributor

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::unordered_map" is directly included [misc-include-cleaner]

include/xeus-cpp/xdebugger.hpp:12:

- #ifdef __GNUC__
+ #include <unordered_map>
+ #ifdef __GNUC__

const std::string& user_name,
const std::string& session_id,
const nl::json& debugger_config);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: namespace 'xcpp' not terminated with a closing comment [llvm-namespace-comment]

Suggested change
}
} // namespace xcpp
Additional context

include/xeus-cpp/xdebugger.hpp:27: namespace 'xcpp' starts here

namespace xcpp
          ^


static pid_t get_current_pid();

std::vector<int> get_execution_count(const std::string& code) const
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: function 'get_execution_count' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
std::vector<int> get_execution_count(const std::string& code) const
[[nodiscard]] std::vector<int> get_execution_count(const std::string& code) const

xoutput_buffer m_cout_buffer;
xoutput_buffer m_cerr_buffer;

std::map<std::string, std::vector<int>> m_code_to_execution_count_map;
Copy link
Copy Markdown
Contributor

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::map" is directly included [misc-include-cleaner]

include/xeus-cpp/xinterpreter.hpp:13:

- #include <memory>
+ #include <map>
+ #include <memory>

Comment thread src/main.cpp
#include "xeus-cpp/xeus_cpp_config.hpp"
#include "xeus-cpp/xinterpreter.hpp"
#include "xeus-cpp/xutils.hpp"
#include "xeus-cpp/xdebugger.hpp"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: #includes are not sorted properly [llvm-include-order]

Suggested change
#include "xeus-cpp/xdebugger.hpp"
#include "xeus-cpp/xdebugger.hpp"
#include "xeus-cpp/xeus_cpp_config.hpp"
#include "xeus-cpp/xinterpreter.hpp"
#include "xeus-cpp/xutils.hpp"
#include "xeus-zmq/xzmq_context.hpp"
#include <xeus-zmq/xserver_zmq.hpp>
#include <xeus/xhelper.hpp>
#include <xeus/xkernel.hpp>
#include <xeus/xkernel_configuration.hpp>

Comment thread src/main.cpp
auto interpreter = std::make_unique<xcpp::interpreter>(argc, argv);
std::unique_ptr<xeus::xcontext> context = xeus::make_zmq_context();

nl::json debugger_config;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "nlohmann::json" is directly included [misc-include-cleaner]

src/main.cpp:9:

- #include <string>
+ #include <nlohmann/json_fwd.hpp>
+ #include <string>

Comment thread src/main.cpp
debugger_config["lldb"]["initCommands"] = {
"settings set plugin.jit-loader.gdb.enable on"
};
debugger_config["interpreter_ptr"] = reinterpret_cast<std::uintptr_t>(interpreter.get());
Copy link
Copy Markdown
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]

    debugger_config["interpreter_ptr"] = reinterpret_cast<std::uintptr_t>(interpreter.get());
                                         ^

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a 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 103. Check the log or trigger a new build to see more.

Comment thread src/main.cpp
debugger_config["lldb"]["initCommands"] = {
"settings set plugin.jit-loader.gdb.enable on"
};
debugger_config["interpreter_ptr"] = reinterpret_cast<std::uintptr_t>(interpreter.get());
Copy link
Copy Markdown
Contributor

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::uintptr_t" is directly included [misc-include-cleaner]

src/main.cpp:8:

- #include <memory>
+ #include <cstdint>
+ #include <memory>

Comment thread src/xdebugger.cpp Outdated
using namespace std::placeholders;

// ***** ONLY FOR DEBUGGING PURPOSES. NOT TO BE COMMITTED *****
static std::ofstream log_stream("/Users/abhinavkumar/Desktop/Coding/CERN_HSF_COMPILER_RESEARCH/xeus-cpp/build/xeus-cpp-logs.log", std::ios::app);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: initializing non-local variable with non-const expression depending on uninitialized non-local variable 'app' [cppcoreguidelines-interfaces-global-init]

static std::ofstream log_stream("/Users/abhinavkumar/Desktop/Coding/CERN_HSF_COMPILER_RESEARCH/xeus-cpp/build/xeus-cpp-logs.log", std::ios::app);
                     ^

Comment thread src/xdebugger.cpp Outdated
using namespace std::placeholders;

// ***** ONLY FOR DEBUGGING PURPOSES. NOT TO BE COMMITTED *****
static std::ofstream log_stream("/Users/abhinavkumar/Desktop/Coding/CERN_HSF_COMPILER_RESEARCH/xeus-cpp/build/xeus-cpp-logs.log", std::ios::app);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'log_stream' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

static std::ofstream log_stream("/Users/abhinavkumar/Desktop/Coding/CERN_HSF_COMPILER_RESEARCH/xeus-cpp/build/xeus-cpp-logs.log", std::ios::app);
                     ^

Comment thread src/xdebugger.cpp
// ***** ONLY FOR DEBUGGING PURPOSES. NOT TO BE COMMITTED *****
static std::ofstream log_stream("/Users/abhinavkumar/Desktop/Coding/CERN_HSF_COMPILER_RESEARCH/xeus-cpp/build/xeus-cpp-logs.log", std::ios::app);

void log_debug(const std::string& msg) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: function 'log_debug' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage]

Suggested change
void log_debug(const std::string& msg) {
static void log_debug(const std::string& msg) {

Comment thread src/xdebugger.cpp
// ***** ONLY FOR DEBUGGING PURPOSES. NOT TO BE COMMITTED *****
static std::ofstream log_stream("/Users/abhinavkumar/Desktop/Coding/CERN_HSF_COMPILER_RESEARCH/xeus-cpp/build/xeus-cpp-logs.log", std::ios::app);

void log_debug(const std::string& msg) {
Copy link
Copy Markdown
Contributor

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" is directly included [misc-include-cleaner]

src/xdebugger.cpp:9:

- #include <sys/socket.h>
+ #include <string>
+ #include <sys/socket.h>

Comment thread src/xdebugger.cpp
static std::ofstream log_stream("/Users/abhinavkumar/Desktop/Coding/CERN_HSF_COMPILER_RESEARCH/xeus-cpp/build/xeus-cpp-logs.log", std::ios::app);

void log_debug(const std::string& msg) {
log_stream << "[xeus-cpp] " << msg << std::endl;
Copy link
Copy Markdown
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 'std::endl' with streams; use '\n' instead [performance-avoid-endl]

Suggested change
log_stream << "[xeus-cpp] " << msg << std::endl;
log_stream << "[xeus-cpp] " << msg << '\n';

Comment thread src/xdebugger.cpp

namespace xcpp
{
debugger::debugger(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: constructor does not initialize these fields: m_is_running, m_tcp_socket, m_tcp_connected, jit_process_pid, m_copy_to_globals_available [cppcoreguidelines-pro-type-member-init]

include/xeus-cpp/xdebugger.hpp:72:

-         bool m_is_running;
-         int m_tcp_socket;
-         bool m_tcp_connected;
-         pid_t jit_process_pid;
+         bool m_is_running{};
+         int m_tcp_socket{};
+         bool m_tcp_connected{};
+         pid_t jit_process_pid{};

include/xeus-cpp/xdebugger.hpp:85:

-         bool m_copy_to_globals_available;
+         bool m_copy_to_globals_available{};

Comment thread src/xdebugger.cpp
namespace xcpp
{
debugger::debugger(
xeus::xcontext& context,
Copy link
Copy Markdown
Contributor

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::xcontext" is directly included [misc-include-cleaner]

src/xdebugger.cpp:14:

+ #include <xeus/xeus_context.hpp>

Comment thread src/xdebugger.cpp
{
debugger::debugger(
xeus::xcontext& context,
const xeus::xconfiguration& config,
Copy link
Copy Markdown
Contributor

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::xconfiguration" is directly included [misc-include-cleaner]

src/xdebugger.cpp:14:

+ #include <xeus/xkernel_configuration.hpp>

Comment thread src/xdebugger.cpp
const xeus::xconfiguration& config,
const std::string& user_name,
const std::string& session_id,
const nl::json& debugger_config
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "nlohmann::json" is directly included [misc-include-cleaner]

src/xdebugger.cpp:9:

- #include <sys/socket.h>
+ #include <nlohmann/json_fwd.hpp>
+ #include <sys/socket.h>

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a 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 93. Check the log or trigger a new build to see more.

Comment thread src/xdebugger.cpp
const std::string& session_id,
const nl::json& debugger_config
)
: xdebugger_base(context)
Copy link
Copy Markdown
Contributor

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::xdebugger_base" is directly included [misc-include-cleaner]

src/xdebugger.cpp:14:

+ #include <xeus-zmq/xdebugger_base.hpp>

Comment thread src/xdebugger.cpp
context,
config,
xeus::get_socket_linger(),
xdap_tcp_configuration(xeus::dap_tcp_type::client, xeus::dap_init_type::parallel, user_name, session_id),
Copy link
Copy Markdown
Contributor

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::dap_init_type" is directly included [misc-include-cleaner]

              xdap_tcp_configuration(xeus::dap_tcp_type::client, xeus::dap_init_type::parallel, user_name, session_id),
                                                                       ^

Comment thread src/xdebugger.cpp
context,
config,
xeus::get_socket_linger(),
xdap_tcp_configuration(xeus::dap_tcp_type::client, xeus::dap_init_type::parallel, user_name, session_id),
Copy link
Copy Markdown
Contributor

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::dap_tcp_type" is directly included [misc-include-cleaner]

src/xdebugger.cpp:14:

+ #include <xeus-zmq/xdap_tcp_client.hpp>

Comment thread src/xdebugger.cpp
get_event_callback()
))
, m_lldb_host("127.0.0.1")
, m_lldb_port("")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: redundant string initialization [readability-redundant-string-init]

Suggested change
, m_lldb_port("")
,

Comment thread src/xdebugger.cpp
// Register request handlers
register_request_handler(
"inspectVariables",
std::bind(&debugger::inspect_variables_request, this, _1),
Copy link
Copy Markdown
Contributor

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::bind" is directly included [misc-include-cleaner]

src/xdebugger.cpp:7:

- #include <iostream>
+ #include <functional>
+ #include <iostream>

Comment thread src/xdebugger.cpp
// Register request handlers
register_request_handler(
"inspectVariables",
std::bind(&debugger::inspect_variables_request, this, _1),
Copy link
Copy Markdown
Contributor

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::placeholders::_1" is directly included [misc-include-cleaner]

            std::bind(&debugger::inspect_variables_request, this, _1),
                                                                  ^

Comment thread src/xdebugger.cpp
// Register request handlers
register_request_handler(
"inspectVariables",
std::bind(&debugger::inspect_variables_request, this, _1),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: prefer a lambda to std::bind [modernize-avoid-bind]

Suggested change
std::bind(&debugger::inspect_variables_request, this, _1),
[this](auto && PH1) { return inspect_variables_request(std::forward<decltype(PH1)>(PH1)); },

Comment thread src/xdebugger.cpp
std::bind(&debugger::inspect_variables_request, this, _1),
false
);
register_request_handler("attach", std::bind(&debugger::attach_request, this, _1), true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: prefer a lambda to std::bind [modernize-avoid-bind]

Suggested change
register_request_handler("attach", std::bind(&debugger::attach_request, this, _1), true);
register_request_handler("attach", [this](auto && PH1) { return attach_request(std::forward<decltype(PH1)>(PH1)); }, true);

Comment thread src/xdebugger.cpp
register_request_handler("attach", std::bind(&debugger::attach_request, this, _1), true);
register_request_handler(
"configurationDone",
std::bind(&debugger::configuration_done_request, this, _1),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: prefer a lambda to std::bind [modernize-avoid-bind]

Suggested change
std::bind(&debugger::configuration_done_request, this, _1),
[this](auto && PH1) { return configuration_done_request(std::forward<decltype(PH1)>(PH1)); },

Comment thread src/xdebugger.cpp
std::bind(&debugger::configuration_done_request, this, _1),
true
);
register_request_handler("richInspectVariables", std::bind(&debugger::rich_inspect_variables_request, this, _1), true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: prefer a lambda to std::bind [modernize-avoid-bind]

Suggested change
register_request_handler("richInspectVariables", std::bind(&debugger::rich_inspect_variables_request, this, _1), true);
register_request_handler("richInspectVariables", [this](auto && PH1) { return rich_inspect_variables_request(std::forward<decltype(PH1)>(PH1)); }, true);

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a 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 102. Check the log or trigger a new build to see more.

#include "xeus_cpp_config.hpp"
#include "xmanager.hpp"
#include <thread>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: included header thread is not used directly [misc-include-cleaner]

Suggested change

return {};
}

std::string get_code_from_execution_count(int execution_count) const
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: function 'get_code_from_execution_count' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
std::string get_code_from_execution_count(int execution_count) const
[[nodiscard]] std::string get_code_from_execution_count(int execution_count) const

Comment thread src/main.cpp
#include "xeus-cpp/xeus_cpp_config.hpp"
#include "xeus-cpp/xinterpreter.hpp"
#include "xeus-cpp/xutils.hpp"
#include "xeus-cpp/xdebugger.hpp"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: #includes are not sorted properly [llvm-include-order]

Suggested change
#include "xeus-cpp/xdebugger.hpp"
#include "xeus-cpp/xdebugger.hpp"
#include "xeus-cpp/xeus_cpp_config.hpp"
#include "xeus-cpp/xinterpreter.hpp"
#include "xeus-cpp/xutils.hpp"
#include "xeus-zmq/xserver_zmq_split.hpp"
#include "xeus-zmq/xzmq_context.hpp"
#include <xeus-zmq/xserver_zmq.hpp>
#include <xeus/xhelper.hpp>
#include <xeus/xkernel.hpp>
#include <xeus/xkernel_configuration.hpp>

Comment thread src/xdebugger.cpp

namespace xcpp
{
debugger::debugger(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: constructor does not initialize these fields: m_is_running, m_tcp_socket, m_tcp_connected, jit_process_pid, m_copy_to_globals_available [cppcoreguidelines-pro-type-member-init]

include/xeus-cpp/xdebugger.hpp:76:

-         bool m_is_running;
-         int m_tcp_socket;
-         bool m_tcp_connected;
-         pid_t jit_process_pid;
+         bool m_is_running{};
+         int m_tcp_socket{};
+         bool m_tcp_connected{};
+         pid_t jit_process_pid{};

include/xeus-cpp/xdebugger.hpp:89:

-         bool m_copy_to_globals_available;
+         bool m_copy_to_globals_available{};

Comment thread src/xdebugger.cpp
true
);
register_request_handler("richInspectVariables", std::bind(&debugger::rich_inspect_variables_request, this, _1), true);
register_request_handler("setBreakpoints", std::bind(&debugger::set_breakpoints_request, this, _1), true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: prefer a lambda to std::bind [modernize-avoid-bind]

Suggested change
register_request_handler("setBreakpoints", std::bind(&debugger::set_breakpoints_request, this, _1), true);
register_request_handler("setBreakpoints", [this](auto && PH1) { return set_breakpoints_request(std::forward<decltype(PH1)>(PH1)); }, true);

Comment thread src/xdebugger.cpp
);
register_request_handler("richInspectVariables", std::bind(&debugger::rich_inspect_variables_request, this, _1), true);
register_request_handler("setBreakpoints", std::bind(&debugger::set_breakpoints_request, this, _1), true);
register_request_handler("dumpCell", std::bind(&debugger::dump_cell_request, this, _1), true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: prefer a lambda to std::bind [modernize-avoid-bind]

Suggested change
register_request_handler("dumpCell", std::bind(&debugger::dump_cell_request, this, _1), true);
register_request_handler("dumpCell", [this](auto && PH1) { return dump_cell_request(std::forward<decltype(PH1)>(PH1)); }, true);

Comment thread src/xdebugger.cpp
register_request_handler("richInspectVariables", std::bind(&debugger::rich_inspect_variables_request, this, _1), true);
register_request_handler("setBreakpoints", std::bind(&debugger::set_breakpoints_request, this, _1), true);
register_request_handler("dumpCell", std::bind(&debugger::dump_cell_request, this, _1), true);
register_request_handler("copyToGlobals", std::bind(&debugger::copy_to_globals_request, this, _1), true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: prefer a lambda to std::bind [modernize-avoid-bind]

Suggested change
register_request_handler("copyToGlobals", std::bind(&debugger::copy_to_globals_request, this, _1), true);
register_request_handler("copyToGlobals", [this](auto && PH1) { return copy_to_globals_request(std::forward<decltype(PH1)>(PH1)); }, true);

Comment thread src/xdebugger.cpp
register_request_handler("setBreakpoints", std::bind(&debugger::set_breakpoints_request, this, _1), true);
register_request_handler("dumpCell", std::bind(&debugger::dump_cell_request, this, _1), true);
register_request_handler("copyToGlobals", std::bind(&debugger::copy_to_globals_request, this, _1), true);
register_request_handler("stackTrace", std::bind(&debugger::stack_trace_request, this, _1), true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: prefer a lambda to std::bind [modernize-avoid-bind]

Suggested change
register_request_handler("stackTrace", std::bind(&debugger::stack_trace_request, this, _1), true);
register_request_handler("stackTrace", [this](auto && PH1) { return stack_trace_request(std::forward<decltype(PH1)>(PH1)); }, true);

Comment thread src/xdebugger.cpp
register_request_handler("dumpCell", std::bind(&debugger::dump_cell_request, this, _1), true);
register_request_handler("copyToGlobals", std::bind(&debugger::copy_to_globals_request, this, _1), true);
register_request_handler("stackTrace", std::bind(&debugger::stack_trace_request, this, _1), true);
register_request_handler("source", std::bind(&debugger::source_request, this, _1), true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: prefer a lambda to std::bind [modernize-avoid-bind]

Suggested change
register_request_handler("source", std::bind(&debugger::source_request, this, _1), true);
register_request_handler("source", [this](auto && PH1) { return source_request(std::forward<decltype(PH1)>(PH1)); }, true);

Comment thread src/xdebugger.cpp
register_request_handler("stackTrace", std::bind(&debugger::stack_trace_request, this, _1), true);
register_request_handler("source", std::bind(&debugger::source_request, this, _1), true);

m_interpreter = reinterpret_cast<xcpp::interpreter*>(
Copy link
Copy Markdown
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]

        m_interpreter = reinterpret_cast<xcpp::interpreter*>(
                        ^

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a 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 118. Check the log or trigger a new build to see more.

Comment thread src/xdebugger.cpp
register_request_handler("stackTrace", std::bind(&debugger::stack_trace_request, this, _1), true);
register_request_handler("source", std::bind(&debugger::source_request, this, _1), true);

m_interpreter = reinterpret_cast<xcpp::interpreter*>(
Copy link
Copy Markdown
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]

        m_interpreter = reinterpret_cast<xcpp::interpreter*>(
                        ^

Comment thread src/xdebugger.cpp
register_request_handler("source", std::bind(&debugger::source_request, this, _1), true);

m_interpreter = reinterpret_cast<xcpp::interpreter*>(
debugger_config["interpreter_ptr"].get<std::uintptr_t>()
Copy link
Copy Markdown
Contributor

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::uintptr_t" is directly included [misc-include-cleaner]

src/xdebugger.cpp:4:

- #include <cstdlib>
+ #include <cstdint>
+ #include <cstdlib>

Comment thread src/xdebugger.cpp

m_interpreter = reinterpret_cast<xcpp::interpreter*>(
debugger_config["interpreter_ptr"].get<std::uintptr_t>()
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: 'm_interpreter' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]

src/xdebugger.cpp:52:

-         , m_debugger_config(debugger_config)
+         , m_debugger_config(debugger_config), m_interpreter(reinterpret_cast<xcpp::interpreter*>(
+             debugger_config["interpreter_ptr"].get<std::uintptr_t>()
+         ))
Suggested change
);

Comment thread src/xdebugger.cpp
m_lldb_port = xeus::find_free_port(100, 9999, 10099);
if (m_lldb_port.empty())
{
std::cerr << "Failed to find a free port for LLDB-DAP" << std::endl;
Copy link
Copy Markdown
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 'std::endl' with streams; use '\n' instead [performance-avoid-endl]

Suggested change
std::cerr << "Failed to find a free port for LLDB-DAP" << std::endl;
std::cerr << "Failed to find a free port for LLDB-DAP" << '\n';

Comment thread src/xdebugger.cpp
}

// std::vector<std::string> lldb_args = {"/Users/abhinavkumar/Desktop/Coding/CERN_HSF_COMPILER_RESEARCH/llvm-project-lldb/build/bin/lldb-dap", "--connection", "listen://localhost:" + m_lldb_port};
std::vector<std::string> lldb_args = {
Copy link
Copy Markdown
Contributor

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::vector" is directly included [misc-include-cleaner]

src/xdebugger.cpp:14:

+ #include <vector>

Comment thread src/xdebugger.cpp
};

std::string log_dir = xeus::get_temp_directory_path() + "/xcpp_debug_logs_"
+ std::to_string(xeus::get_current_pid());
Copy link
Copy Markdown
Contributor

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::to_string" is directly included [misc-include-cleaner]

                              + std::to_string(xeus::get_current_pid());
                                     ^

Comment thread src/xdebugger.cpp

// std::cout << "Log file for LLDB-DAP: " << log_file << std::endl;

pid_t pid = fork();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "pid_t" is directly included [misc-include-cleaner]

src/xdebugger.cpp:13:

- #include <unistd.h>
+ #include <time.h>
+ #include <unistd.h>

Comment thread src/xdebugger.cpp
if (pid == 0)
{
// ***** ONLY FOR DEBUGGING PURPOSES. NOT TO BE COMMITTED *****
int fd = open(log_file.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0644);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: do not call c-style vararg functions [cppcoreguidelines-pro-type-vararg]

            int fd = open(log_file.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0644);
                     ^

Comment thread src/xdebugger.cpp
}
// ******* (END) ONLY FOR DEBUGGING PURPOSES. NOT TO BE COMMITTED ******

int null_fd = open("/dev/null", O_RDONLY);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: do not call c-style vararg functions [cppcoreguidelines-pro-type-vararg]

            int null_fd = open("/dev/null", O_RDONLY);
                          ^

Comment thread src/xdebugger.cpp
std::vector<char*> argv;
for (auto& arg : lldb_args)
{
argv.push_back(const_cast<char*>(arg.c_str()));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: 'push_back' is called inside a loop; consider pre-allocating the container capacity before the loop [performance-inefficient-vector-operation]

src/xdebugger.cpp:137:

-             for (auto& arg : lldb_args)
+             argv.reserve(lldb_args.size());
+ for (auto& arg : lldb_args)

@kr-2003 kr-2003 changed the title [wip] Implementing debugger in xeus-cpp [wip] Implementing debugger in xeus-cpp(Prototype) Jul 8, 2025
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a 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 82. Check the log or trigger a new build to see more.

Comment thread src/xdebugger.cpp
std::vector<char*> argv;
for (auto& arg : lldb_args)
{
argv.push_back(const_cast<char*>(arg.c_str()));
Copy link
Copy Markdown
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 const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast]

                argv.push_back(const_cast<char*>(arg.c_str()));
                               ^

Comment thread src/xdebugger.cpp

execvp("lldb-dap", argv.data());

std::cerr << "Failed to execute lldb-dap" << std::endl;
Copy link
Copy Markdown
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 'std::endl' with streams; use '\n' instead [performance-avoid-endl]

Suggested change
std::cerr << "Failed to execute lldb-dap" << std::endl;
std::cerr << "Failed to execute lldb-dap" << '\n';

Comment thread src/xdebugger.cpp
}
else if (pid > 0)
{
int status;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Suggested change
int status;
int status = 0;

Comment thread src/xdebugger.cpp Outdated
else if (pid > 0)
{
int status;
if (waitpid(pid, &status, WNOHANG) != 0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: no header providing "WNOHANG" is directly included [misc-include-cleaner]

src/xdebugger.cpp:9:

- #include <sys/socket.h>
+ #include <stdlib.h>
+ #include <sys/socket.h>

Comment thread src/xdebugger.cpp
int status;
if (waitpid(pid, &status, WNOHANG) != 0)
{
std::cerr << "LLDB-DAP process exited prematurely." << std::endl;
Copy link
Copy Markdown
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 'std::endl' with streams; use '\n' instead [performance-avoid-endl]

Suggested change
std::cerr << "LLDB-DAP process exited prematurely." << std::endl;
std::cerr << "LLDB-DAP process exited prematurely." << '\n';

Comment thread src/xdebugger.cpp
}
else
{
std::cerr << "fork() failed" << std::endl;
Copy link
Copy Markdown
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 'std::endl' with streams; use '\n' instead [performance-avoid-endl]

Suggested change
std::cerr << "fork() failed" << std::endl;
std::cerr << "fork() failed" << '\n';

Comment thread src/xdebugger.cpp
bool lldb_started = start_lldb();
if (!lldb_started)
{
std::cerr << "Failed to start LLDB-DAP" << std::endl;
Copy link
Copy Markdown
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 'std::endl' with streams; use '\n' instead [performance-avoid-endl]

Suggested change
std::cerr << "Failed to start LLDB-DAP" << std::endl;
std::cerr << "Failed to start LLDB-DAP" << '\n';

Comment thread src/xdebugger.cpp
nl::json bp_json = message["arguments"]["breakpoints"];
std::vector<nl::json> bp_list(bp_json.begin(), bp_json.end());
std::string sequential_source = m_hash_to_sequential[source][0];
m_breakpoint_list.insert(std::make_pair(std::move(source), std::move(bp_list)));
Copy link
Copy Markdown
Contributor

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::make_pair" is directly included [misc-include-cleaner]

src/xdebugger.cpp:14:

+ #include <utility>

Comment thread src/xdebugger.cpp
nl::json bp_json = message["arguments"]["breakpoints"];
std::vector<nl::json> bp_list(bp_json.begin(), bp_json.end());
std::string sequential_source = m_hash_to_sequential[source][0];
m_breakpoint_list.insert(std::make_pair(std::move(source), std::move(bp_list)));
Copy link
Copy Markdown
Contributor

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::move" is directly included [misc-include-cleaner]

        m_breakpoint_list.insert(std::make_pair(std::move(source), std::move(bp_list)));
                                                                        ^

Comment thread src/xdebugger.cpp
nl::json bp_json = message["arguments"]["breakpoints"];
std::vector<nl::json> bp_list(bp_json.begin(), bp_json.end());
std::string sequential_source = m_hash_to_sequential[source][0];
m_breakpoint_list.insert(std::make_pair(std::move(source), std::move(bp_list)));
Copy link
Copy Markdown
Contributor

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::move" is directly included [misc-include-cleaner]

        m_breakpoint_list.insert(std::make_pair(std::move(source), std::move(bp_list)));
                                                     ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants