Skip to content
Merged
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
8 changes: 5 additions & 3 deletions lldb/tools/lldb-dap/DAP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -526,12 +526,14 @@ ExceptionBreakpoint *DAP::GetExceptionBPFromStopReason(lldb::SBThread &thread) {
}

lldb::SBThread DAP::GetLLDBThread(const llvm::json::Object &arguments) {
auto tid = GetSigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
auto tid = GetInteger<int64_t>(arguments, "threadId")
.value_or(LLDB_INVALID_THREAD_ID);
return target.GetProcess().GetThreadByID(tid);
}

lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object &arguments) {
const uint64_t frame_id = GetUnsigned(arguments, "frameId", UINT64_MAX);
const uint64_t frame_id =
GetInteger<uint64_t>(arguments, "frameId").value_or(UINT64_MAX);
lldb::SBProcess process = target.GetProcess();
// Upper 32 bits is the thread index ID
lldb::SBThread thread =
Expand Down Expand Up @@ -771,7 +773,7 @@ bool DAP::HandleObject(const llvm::json::Object &object) {
}

if (packet_type == "response") {
auto id = GetSigned(object, "request_seq", 0);
auto id = GetInteger<int64_t>(object, "request_seq").value_or(0);

std::unique_ptr<ResponseHandler> response_handler;
{
Expand Down
7 changes: 4 additions & 3 deletions lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
const int invalid_port = 0;
const auto *arguments = request.getObject("arguments");
const lldb::pid_t pid =
GetUnsigned(arguments, "pid", LLDB_INVALID_PROCESS_ID);
GetInteger<uint64_t>(arguments, "pid").value_or(LLDB_INVALID_PROCESS_ID);
const auto gdb_remote_port =
GetUnsigned(arguments, "gdb-remote-port", invalid_port);
GetInteger<uint64_t>(arguments, "gdb-remote-port").value_or(invalid_port);
const auto gdb_remote_hostname =
GetString(arguments, "gdb-remote-hostname", "localhost");
if (pid != LLDB_INVALID_PROCESS_ID)
Expand All @@ -70,7 +70,8 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
dap.terminate_commands = GetStrings(arguments, "terminateCommands");
auto attachCommands = GetStrings(arguments, "attachCommands");
llvm::StringRef core_file = GetString(arguments, "coreFile");
const uint64_t timeout_seconds = GetUnsigned(arguments, "timeout", 30);
const auto timeout_seconds =
GetInteger<uint64_t>(arguments, "timeout").value_or(30);
dap.stop_at_entry = core_file.empty()
? GetBoolean(arguments, "stopOnEntry").value_or(false)
: true;
Expand Down
12 changes: 7 additions & 5 deletions lldb/tools/lldb-dap/Handler/BreakpointLocationsHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,13 @@ void BreakpointLocationsRequestHandler::operator()(
auto *arguments = request.getObject("arguments");
auto *source = arguments->getObject("source");
std::string path = GetString(source, "path").str();
uint64_t start_line = GetUnsigned(arguments, "line", 0);
uint64_t start_column = GetUnsigned(arguments, "column", 0);
uint64_t end_line = GetUnsigned(arguments, "endLine", start_line);
uint64_t end_column =
GetUnsigned(arguments, "endColumn", std::numeric_limits<uint64_t>::max());
const auto start_line = GetInteger<uint64_t>(arguments, "line").value_or(0);
const auto start_column =
GetInteger<uint64_t>(arguments, "column").value_or(0);
const auto end_line =
GetInteger<uint64_t>(arguments, "endLine").value_or(start_line);
const auto end_column = GetInteger<uint64_t>(arguments, "endColumn")
.value_or(std::numeric_limits<uint64_t>::max());
Comment on lines +134 to +140
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use LLDB_INVALID_LINE_NUMBER as the default for any of the 'line' values?

And for the 'column' should default to LLDB_INVALID_COLUMN_NUMBER, which is also 0, but the value makes it more explicit that we're using it as a fallback.

I suppose that may result in a change of behavior, so we may not want to but it seems like some like we may want to have more specific fallbacks. For example, I haven't actually tried if I can set a breakpoint on line 0 (or 1 depending on how you count) of a file.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's a good idea, especially since we have code comparing against LLDB_INVALID_LINE_NUMBER. I'll do that in a follow-up though to keep this one NFC.


lldb::SBFileSpec file_spec(path.c_str(), true);
lldb::SBSymbolContextList compile_units =
Expand Down
5 changes: 3 additions & 2 deletions lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,9 @@ void CompletionsRequestHandler::operator()(
}

std::string text = GetString(arguments, "text").str();
auto original_column = GetSigned(arguments, "column", text.size());
auto original_line = GetSigned(arguments, "line", 1);
auto original_column =
GetInteger<int64_t>(arguments, "column").value_or(text.size());
auto original_line = GetInteger<int64_t>(arguments, "line").value_or(1);
auto offset = original_column - 1;
if (original_line > 1) {
llvm::SmallVector<::llvm::StringRef, 2> lines;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ void DataBreakpointInfoRequestHandler::operator()(
llvm::json::Array accessTypes{"read", "write", "readWrite"};
const auto *arguments = request.getObject("arguments");
const auto variablesReference =
GetUnsigned(arguments, "variablesReference", 0);
GetInteger<uint64_t>(arguments, "variablesReference").value_or(0);
llvm::StringRef name = GetString(arguments, "name");
lldb::SBFrame frame = dap.GetLLDBFrame(*arguments);
lldb::SBValue variable = dap.variables.FindVariable(variablesReference, name);
Expand Down
5 changes: 3 additions & 2 deletions lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ void DisassembleRequestHandler::operator()(
}
lldb::addr_t addr_ptr = *addr_opt;

addr_ptr += GetSigned(arguments, "instructionOffset", 0);
addr_ptr += GetInteger<int64_t>(arguments, "instructionOffset").value_or(0);
lldb::SBAddress addr(addr_ptr, dap.target);
if (!addr.IsValid()) {
response["success"] = false;
Expand All @@ -113,7 +113,8 @@ void DisassembleRequestHandler::operator()(
return;
}

const auto inst_count = GetUnsigned(arguments, "instructionCount", 0);
const auto inst_count =
GetInteger<int64_t>(arguments, "instructionCount").value_or(0);
lldb::SBInstructionList insts = dap.target.ReadInstructions(addr, inst_count);

if (!insts.IsValid()) {
Expand Down
3 changes: 2 additions & 1 deletion lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ void LocationsRequestHandler::operator()(
FillResponse(request, response);
auto *arguments = request.getObject("arguments");

uint64_t location_id = GetUnsigned(arguments, "locationReference", 0);
const auto location_id =
GetInteger<uint64_t>(arguments, "locationReference").value_or(0);
// We use the lowest bit to distinguish between value location and declaration
// location
auto [var_ref, is_value_location] = UnpackLocation(location_id);
Expand Down
5 changes: 3 additions & 2 deletions lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,9 @@ void ReadMemoryRequestHandler::operator()(
return;
}
lldb::addr_t addr_int = *addr_opt;
addr_int += GetSigned(arguments, "offset", 0);
const uint64_t count_requested = GetUnsigned(arguments, "count", 0);
addr_int += GetInteger<uint64_t>(arguments, "offset").value_or(0);
const uint64_t count_requested =
GetInteger<uint64_t>(arguments, "count").value_or(0);

// We also need support reading 0 bytes
// VS Code sends those requests to check if a `memoryReference`
Expand Down
3 changes: 2 additions & 1 deletion lldb/tools/lldb-dap/Handler/RequestHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ RequestHandler::LaunchProcess(const llvm::json::Object &request) const {
launch_info.SetDetachOnError(detachOnError);
launch_info.SetLaunchFlags(flags | lldb::eLaunchFlagDebug |
lldb::eLaunchFlagStopAtEntry);
const uint64_t timeout_seconds = GetUnsigned(arguments, "timeout", 30);
const auto timeout_seconds =
GetInteger<uint64_t>(arguments, "timeout").value_or(30);

if (GetBoolean(arguments, "runInTerminal").value_or(false)) {
if (llvm::Error err = RunInTerminal(dap, request, timeout_seconds))
Expand Down
5 changes: 3 additions & 2 deletions lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ void SetVariableRequestHandler::operator()(
const auto *arguments = request.getObject("arguments");
// This is a reference to the containing variable/scope
const auto variablesReference =
GetUnsigned(arguments, "variablesReference", 0);
GetInteger<uint64_t>(arguments, "variablesReference").value_or(0);
llvm::StringRef name = GetString(arguments, "name");

const auto value = GetString(arguments, "value");
Expand All @@ -133,7 +133,8 @@ void SetVariableRequestHandler::operator()(
// the name of the variable. We could have two shadowed variables with the
// same name in "Locals" or "Globals". In our case the "id" absolute index
// of the variable within the dap.variables list.
const auto id_value = GetUnsigned(arguments, "id", UINT64_MAX);
const auto id_value =
GetInteger<uint64_t>(arguments, "id").value_or(UINT64_MAX);
if (id_value != UINT64_MAX) {
variable = dap.variables.GetVariable(id_value);
} else {
Expand Down
6 changes: 4 additions & 2 deletions lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,10 @@ void SourceRequestHandler::operator()(const llvm::json::Object &request) const {
const auto *arguments = request.getObject("arguments");
const auto *source = arguments->getObject("source");
llvm::json::Object body;
int64_t source_ref = GetUnsigned(
source, "sourceReference", GetUnsigned(arguments, "sourceReference", 0));
const auto source_ref =
GetInteger<uint64_t>(source, "sourceReference")
.value_or(
GetInteger<uint64_t>(arguments, "sourceReference").value_or(0));

if (source_ref) {
lldb::SBProcess process = dap.target.GetProcess();
Expand Down
5 changes: 3 additions & 2 deletions lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,9 @@ void StackTraceRequestHandler::operator()(
llvm::json::Object body;

if (thread.IsValid()) {
const auto start_frame = GetUnsigned(arguments, "startFrame", 0);
const auto levels = GetUnsigned(arguments, "levels", 0);
const auto start_frame =
GetInteger<uint64_t>(arguments, "startFrame").value_or(0);
const auto levels = GetInteger<uint64_t>(arguments, "levels").value_or(0);
int64_t offset = 0;
bool reached_end_of_stack =
FillStackFrames(dap, thread, stack_frames, offset, start_frame,
Expand Down
3 changes: 2 additions & 1 deletion lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ void StepInRequestHandler::operator()(const llvm::json::Object &request) const {
const auto *arguments = request.getObject("arguments");

std::string step_in_target;
uint64_t target_id = GetUnsigned(arguments, "targetId", 0);
const auto target_id =
GetInteger<uint64_t>(arguments, "targetId").value_or(0);
auto it = dap.step_in_targets.find(target_id);
if (it != dap.step_in_targets.end())
step_in_target = it->second;
Expand Down
6 changes: 3 additions & 3 deletions lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ void VariablesRequestHandler::operator()(
llvm::json::Array variables;
const auto *arguments = request.getObject("arguments");
const auto variablesReference =
GetUnsigned(arguments, "variablesReference", 0);
const int64_t start = GetSigned(arguments, "start", 0);
const int64_t count = GetSigned(arguments, "count", 0);
GetInteger<uint64_t>(arguments, "variablesReference").value_or(0);
const auto start = GetInteger<int64_t>(arguments, "start").value_or(0);
const auto count = GetInteger<int64_t>(arguments, "count").value_or(0);
bool hex = false;
const auto *format = arguments->getObject("format");
if (format)
Expand Down
2 changes: 1 addition & 1 deletion lldb/tools/lldb-dap/InstructionBreakpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace lldb_dap {
InstructionBreakpoint::InstructionBreakpoint(DAP &d,
const llvm::json::Object &obj)
: Breakpoint(d, obj), instructionAddressReference(LLDB_INVALID_ADDRESS),
offset(GetSigned(obj, "offset", 0)) {
offset(GetInteger<int64_t>(obj, "offset").value_or(0)) {
GetString(obj, "instructionReference")
.getAsInteger(0, instructionAddressReference);
instructionAddressReference += offset;
Expand Down
16 changes: 1 addition & 15 deletions lldb/tools/lldb-dap/JSONUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,20 +116,6 @@ std::optional<bool> GetBoolean(const llvm::json::Object *obj,
return std::nullopt;
}

int64_t GetSigned(const llvm::json::Object &obj, llvm::StringRef key,
int64_t fail_value) {
if (auto value = obj.getInteger(key))
return *value;
return fail_value;
}

int64_t GetSigned(const llvm::json::Object *obj, llvm::StringRef key,
int64_t fail_value) {
if (obj == nullptr)
return fail_value;
return GetSigned(*obj, key, fail_value);
}

bool ObjectContainsKey(const llvm::json::Object &obj, llvm::StringRef key) {
return obj.find(key) != obj.end();
}
Expand Down Expand Up @@ -280,7 +266,7 @@ void FillResponse(const llvm::json::Object &request,
response.try_emplace("type", "response");
response.try_emplace("seq", (int64_t)0);
EmplaceSafeString(response, "command", GetString(request, "command"));
const int64_t seq = GetSigned(request, "seq", 0);
const auto seq = GetInteger<int64_t>(request, "seq").value_or(0);
response.try_emplace("request_seq", seq);
response.try_emplace("success", true);
}
Expand Down
46 changes: 19 additions & 27 deletions lldb/tools/lldb-dap/JSONUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ llvm::StringRef GetString(const llvm::json::Object &obj, llvm::StringRef key,
llvm::StringRef GetString(const llvm::json::Object *obj, llvm::StringRef key,
llvm::StringRef defaultValue = {});

/// Extract the unsigned integer value for the specified key from
/// the specified object.
/// Extract the integer value for the specified key from the specified object
/// and return it as the specified integer type T.
///
/// \param[in] obj
/// A JSON object that we will attempt to extract the value from
Expand All @@ -84,13 +84,23 @@ llvm::StringRef GetString(const llvm::json::Object *obj, llvm::StringRef key,
/// The key to use when extracting the value
///
/// \return
/// The unsigned integer value for the specified \a key, or
/// \a fail_value if there is no key that matches or if the
/// value is not an integer.
uint64_t GetUnsigned(const llvm::json::Object &obj, llvm::StringRef key,
uint64_t fail_value);
uint64_t GetUnsigned(const llvm::json::Object *obj, llvm::StringRef key,
uint64_t fail_value);
/// The integer value for the specified \a key, or std::nullopt if there is
/// no key that matches or if the value is not an integer.
/// @{
template <typename T>
std::optional<T> GetInteger(const llvm::json::Object &obj,
llvm::StringRef key) {
return obj.getInteger(key);
}

template <typename T>
std::optional<T> GetInteger(const llvm::json::Object *obj,
llvm::StringRef key) {
if (obj != nullptr)
return GetInteger<T>(*obj, key);
return std::nullopt;
}
/// @}

/// Extract the boolean value for the specified key from the
/// specified object.
Expand All @@ -112,24 +122,6 @@ std::optional<bool> GetBoolean(const llvm::json::Object *obj,
llvm::StringRef key);
/// @}

/// Extract the signed integer for the specified key from the
/// specified object.
///
/// \param[in] obj
/// A JSON object that we will attempt to extract the value from
///
/// \param[in] key
/// The key to use when extracting the value
///
/// \return
/// The signed integer value for the specified \a key, or
/// \a fail_value if there is no key that matches or if the
/// value is not an integer.
int64_t GetSigned(const llvm::json::Object &obj, llvm::StringRef key,
int64_t fail_value);
int64_t GetSigned(const llvm::json::Object *obj, llvm::StringRef key,
int64_t fail_value);

/// Check if the specified key exists in the specified object.
///
/// \param[in] obj
Expand Down
4 changes: 2 additions & 2 deletions lldb/tools/lldb-dap/SourceBreakpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ namespace lldb_dap {
SourceBreakpoint::SourceBreakpoint(DAP &dap, const llvm::json::Object &obj)
: Breakpoint(dap, obj),
logMessage(std::string(GetString(obj, "logMessage"))),
line(GetUnsigned(obj, "line", 0)), column(GetUnsigned(obj, "column", 0)) {
}
line(GetInteger<uint64_t>(obj, "line").value_or(0)),
column(GetInteger<uint64_t>(obj, "column").value_or(0)) {}

void SourceBreakpoint::SetBreakpoint(const llvm::StringRef source_path) {
lldb::SBFileSpecList module_list;
Expand Down
Loading