[lldb-dap] Refactoring JSONUtils to not use g_dap and instead passing in required arguments.
#115561
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…g in required arguments. This is part of a larger refactor to remove the global `g_dap` variable.
ashgti
assigned
walter-erquinigo and
vogelsgesang and unassigned
walter-erquinigo and
vogelsgesang
|
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesThis is part of a larger refactor to remove the global Patch is 25.94 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/115561.diff 3 Files Affected:
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index a7300abae0eac8..6ca4dfb4711a13 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -154,7 +154,7 @@ DecodeMemoryReference(llvm::StringRef memoryReference) {
std::vector<std::string> GetStrings(const llvm::json::Object *obj,
llvm::StringRef key) {
std::vector<std::string> strs;
- auto json_array = obj->getArray(key);
+ const auto *json_array = obj->getArray(key);
if (!json_array)
return strs;
for (const auto &value : *json_array) {
@@ -210,12 +210,6 @@ static bool IsClassStructOrUnionType(lldb::SBType t) {
/// glance.
static std::optional<std::string>
TryCreateAutoSummaryForContainer(lldb::SBValue &v) {
- // We gate this feature because it performs GetNumChildren(), which can
- // cause performance issues because LLDB needs to complete possibly huge
- // types.
- if (!g_dap.enable_auto_variable_summaries)
- return std::nullopt;
-
if (!v.MightHaveChildren())
return std::nullopt;
/// As this operation can be potentially slow, we limit the total time spent
@@ -271,10 +265,7 @@ TryCreateAutoSummaryForContainer(lldb::SBValue &v) {
/// Try to create a summary string for the given value that doesn't have a
/// summary of its own.
-static std::optional<std::string> TryCreateAutoSummary(lldb::SBValue value) {
- if (!g_dap.enable_auto_variable_summaries)
- return std::nullopt;
-
+static std::optional<std::string> TryCreateAutoSummary(lldb::SBValue &value) {
// We use the dereferenced value for generating the summary.
if (value.GetType().IsPointerType() || value.GetType().IsReferenceType())
value = value.Dereference();
@@ -485,10 +476,12 @@ static std::string ConvertDebugInfoSizeToString(uint64_t debug_info) {
}
return oss.str();
}
-llvm::json::Value CreateModule(lldb::SBModule &module) {
+
+llvm::json::Value CreateModule(lldb::SBTarget &target, lldb::SBModule &module) {
llvm::json::Object object;
- if (!module.IsValid())
+ if (!target.IsValid() || !module.IsValid())
return llvm::json::Value(std::move(object));
+
const char *uuid = module.GetUUIDString();
object.try_emplace("id", uuid ? std::string(uuid) : std::string(""));
object.try_emplace("name", std::string(module.GetFileSpec().GetFilename()));
@@ -514,7 +507,7 @@ llvm::json::Value CreateModule(lldb::SBModule &module) {
object.try_emplace("symbolStatus", "Symbols not found.");
}
std::string loaded_addr = std::to_string(
- module.GetObjectFileHeaderAddress().GetLoadAddress(g_dap.target));
+ module.GetObjectFileHeaderAddress().GetLoadAddress(target));
object.try_emplace("addressRange", loaded_addr);
std::string version_str;
uint32_t version_nums[3];
@@ -705,7 +698,7 @@ llvm::json::Value CreateSource(llvm::StringRef source_path) {
return llvm::json::Value(std::move(source));
}
-std::optional<llvm::json::Value> CreateSource(lldb::SBFrame &frame) {
+static std::optional<llvm::json::Value> CreateSource(lldb::SBFrame &frame) {
auto line_entry = frame.GetLineEntry();
// A line entry of 0 indicates the line is compiler generated i.e. no source
// file is associated with the frame.
@@ -776,15 +769,15 @@ std::optional<llvm::json::Value> CreateSource(lldb::SBFrame &frame) {
// },
// "required": [ "id", "name", "line", "column" ]
// }
-llvm::json::Value CreateStackFrame(lldb::SBFrame &frame) {
+llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
+ lldb::SBFormat &format) {
llvm::json::Object object;
int64_t frame_id = MakeDAPFrameID(frame);
object.try_emplace("id", frame_id);
std::string frame_name;
lldb::SBStream stream;
- if (g_dap.frame_format &&
- frame.GetDescriptionWithFormat(g_dap.frame_format, stream).Success()) {
+ if (format && frame.GetDescriptionWithFormat(format, stream).Success()) {
frame_name = stream.GetData();
// `function_name` can be a nullptr, which throws an error when assigned to
@@ -801,7 +794,7 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame) {
}
// We only include `[opt]` if a custom frame format is not specified.
- if (!g_dap.frame_format && frame.GetFunction().GetIsOptimized())
+ if (!format && frame.GetFunction().GetIsOptimized())
frame_name += " [opt]";
EmplaceSafeString(object, "name", frame_name);
@@ -835,11 +828,11 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame) {
return llvm::json::Value(std::move(object));
}
-llvm::json::Value CreateExtendedStackFrameLabel(lldb::SBThread &thread) {
+llvm::json::Value CreateExtendedStackFrameLabel(lldb::SBThread &thread,
+ lldb::SBFormat &format) {
std::string name;
lldb::SBStream stream;
- if (g_dap.thread_format &&
- thread.GetDescriptionWithFormat(g_dap.thread_format, stream).Success()) {
+ if (format && thread.GetDescriptionWithFormat(format, stream).Success()) {
name = stream.GetData();
} else {
const uint32_t thread_idx = thread.GetExtendedBacktraceOriginatingIndexID();
@@ -872,13 +865,12 @@ llvm::json::Value CreateExtendedStackFrameLabel(lldb::SBThread &thread) {
// },
// "required": [ "id", "name" ]
// }
-llvm::json::Value CreateThread(lldb::SBThread &thread) {
+llvm::json::Value CreateThread(lldb::SBThread &thread, lldb::SBFormat &format) {
llvm::json::Object object;
object.try_emplace("id", (int64_t)thread.GetThreadID());
std::string thread_str;
lldb::SBStream stream;
- if (g_dap.thread_format &&
- thread.GetDescriptionWithFormat(g_dap.thread_format, stream).Success()) {
+ if (format && thread.GetDescriptionWithFormat(format, stream).Success()) {
thread_str = stream.GetData();
} else {
const char *thread_name = thread.GetName();
@@ -966,7 +958,7 @@ llvm::json::Value CreateThread(lldb::SBThread &thread) {
// "required": [ "event", "body" ]
// }]
// }
-llvm::json::Value CreateThreadStopped(lldb::SBThread &thread,
+llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread,
uint32_t stop_id) {
llvm::json::Object event(CreateEventObject("stopped"));
llvm::json::Object body;
@@ -976,13 +968,13 @@ llvm::json::Value CreateThreadStopped(lldb::SBThread &thread,
body.try_emplace("reason", "step");
break;
case lldb::eStopReasonBreakpoint: {
- ExceptionBreakpoint *exc_bp = g_dap.GetExceptionBPFromStopReason(thread);
+ ExceptionBreakpoint *exc_bp = dap.GetExceptionBPFromStopReason(thread);
if (exc_bp) {
body.try_emplace("reason", "exception");
EmplaceSafeString(body, "description", exc_bp->label);
} else {
InstructionBreakpoint *inst_bp =
- g_dap.GetInstructionBPFromStopReason(thread);
+ dap.GetInstructionBPFromStopReason(thread);
if (inst_bp) {
body.try_emplace("reason", "instruction breakpoint");
} else {
@@ -1042,21 +1034,21 @@ llvm::json::Value CreateThreadStopped(lldb::SBThread &thread,
}
}
// "threadCausedFocus" is used in tests to validate breaking behavior.
- if (tid == g_dap.focus_tid) {
+ if (tid == dap.focus_tid) {
body.try_emplace("threadCausedFocus", true);
}
- body.try_emplace("preserveFocusHint", tid != g_dap.focus_tid);
+ body.try_emplace("preserveFocusHint", tid != dap.focus_tid);
body.try_emplace("allThreadsStopped", true);
event.try_emplace("body", std::move(body));
return llvm::json::Value(std::move(event));
}
-const char *GetNonNullVariableName(lldb::SBValue v) {
+const char *GetNonNullVariableName(lldb::SBValue &v) {
const char *name = v.GetName();
return name ? name : "<null>";
}
-std::string CreateUniqueVariableNameForDisplay(lldb::SBValue v,
+std::string CreateUniqueVariableNameForDisplay(lldb::SBValue &v,
bool is_name_duplicated) {
lldb::SBStream name_builder;
name_builder.Print(GetNonNullVariableName(v));
@@ -1073,7 +1065,9 @@ std::string CreateUniqueVariableNameForDisplay(lldb::SBValue v,
return name_builder.GetData();
}
-VariableDescription::VariableDescription(lldb::SBValue v, bool format_hex,
+VariableDescription::VariableDescription(lldb::SBValue v,
+ bool auto_variable_summaries,
+ bool format_hex,
bool is_name_duplicated,
std::optional<std::string> custom_name)
: v(v) {
@@ -1104,7 +1098,7 @@ VariableDescription::VariableDescription(lldb::SBValue v, bool format_hex,
} else {
value = llvm::StringRef(v.GetValue()).str();
summary = llvm::StringRef(v.GetSummary()).str();
- if (summary.empty())
+ if (summary.empty() && auto_variable_summaries)
auto_summary = TryCreateAutoSummary(v);
std::optional<std::string> effective_summary =
@@ -1188,7 +1182,7 @@ bool ValuePointsToCode(lldb::SBValue v) {
lldb::addr_t addr = v.GetValueAsAddress();
lldb::SBLineEntry line_entry =
- g_dap.target.ResolveLoadAddress(addr).GetLineEntry();
+ v.GetTarget().ResolveLoadAddress(addr).GetLineEntry();
return line_entry.IsValid();
}
@@ -1349,9 +1343,12 @@ std::pair<int64_t, bool> UnpackLocation(int64_t location_id) {
// "required": [ "name", "value", "variablesReference" ]
// }
llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
- bool format_hex, bool is_name_duplicated,
+ bool format_hex, bool auto_variable_summaries,
+ bool synthetic_child_debugging,
+ bool is_name_duplicated,
std::optional<std::string> custom_name) {
- VariableDescription desc(v, format_hex, is_name_duplicated, custom_name);
+ VariableDescription desc(v, auto_variable_summaries, format_hex,
+ is_name_duplicated, custom_name);
llvm::json::Object object;
EmplaceSafeString(object, "name", desc.name);
EmplaceSafeString(object, "value", desc.display_value);
@@ -1387,7 +1384,7 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
size_t num_children = v.GetNumChildren();
// If we are creating a "[raw]" fake child for each synthetic type, we
// have to account for it when returning indexed variables.
- if (g_dap.enable_synthetic_child_debugging)
+ if (synthetic_child_debugging)
++num_children;
object.try_emplace("indexedVariables", num_children);
}
@@ -1418,7 +1415,7 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
return llvm::json::Value(std::move(object));
}
-llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit unit) {
+llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit) {
llvm::json::Object object;
char unit_path_arr[PATH_MAX];
unit.GetFileSpec().GetPath(unit_path_arr, sizeof(unit_path_arr));
@@ -1439,7 +1436,7 @@ CreateRunInTerminalReverseRequest(const llvm::json::Object &launch_request,
// the terminal in a new window.
run_in_terminal_args.try_emplace("kind", "integrated");
- auto launch_request_arguments = launch_request.getObject("arguments");
+ const auto *launch_request_arguments = launch_request.getObject("arguments");
// The program path must be the first entry in the "args" field
std::vector<std::string> args = {debug_adaptor_path.str(), "--comm-file",
comm_file.str()};
@@ -1465,11 +1462,7 @@ CreateRunInTerminalReverseRequest(const llvm::json::Object &launch_request,
llvm::StringRef key = envs.GetNameAtIndex(index);
llvm::StringRef value = envs.GetValueAtIndex(index);
- if (key.empty())
- g_dap.SendOutput(OutputType::Stderr,
- "empty environment variable for value: \"" +
- value.str() + '\"');
- else
+ if (!key.empty())
env_json.try_emplace(key, value);
}
run_in_terminal_args.try_emplace("env",
@@ -1481,8 +1474,8 @@ CreateRunInTerminalReverseRequest(const llvm::json::Object &launch_request,
// Keep all the top level items from the statistics dump, except for the
// "modules" array. It can be huge and cause delay
// Array and dictionary value will return as <key, JSON string> pairs
-void FilterAndGetValueForKey(const lldb::SBStructuredData data, const char *key,
- llvm::json::Object &out) {
+static void FilterAndGetValueForKey(const lldb::SBStructuredData data,
+ const char *key, llvm::json::Object &out) {
lldb::SBStructuredData value = data.GetValueForKey(key);
std::string key_utf8 = llvm::json::fixUTF8(key);
if (llvm::StringRef(key) == "modules")
@@ -1524,8 +1517,8 @@ void FilterAndGetValueForKey(const lldb::SBStructuredData data, const char *key,
}
}
-void addStatistic(llvm::json::Object &event) {
- lldb::SBStructuredData statistics = g_dap.target.GetStatistics();
+static void addStatistic(lldb::SBTarget &target, llvm::json::Object &event) {
+ lldb::SBStructuredData statistics = target.GetStatistics();
bool is_dictionary =
statistics.GetType() == lldb::eStructuredDataTypeDictionary;
if (!is_dictionary)
@@ -1542,9 +1535,9 @@ void addStatistic(llvm::json::Object &event) {
event.try_emplace("statistics", std::move(stats_body));
}
-llvm::json::Object CreateTerminatedEventObject() {
+llvm::json::Object CreateTerminatedEventObject(lldb::SBTarget &target) {
llvm::json::Object event(CreateEventObject("terminated"));
- addStatistic(event);
+ addStatistic(target, event);
return event;
}
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 2e13459c45556f..db56d987773476 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -12,6 +12,7 @@
#include "DAPForward.h"
#include "lldb/API/SBCompileUnit.h"
#include "lldb/API/SBFileSpec.h"
+#include "lldb/API/SBFormat.h"
#include "lldb/API/SBLineEntry.h"
#include "lldb/API/SBType.h"
#include "lldb/API/SBValue.h"
@@ -267,13 +268,16 @@ CreateBreakpoint(BreakpointBase *bp,
/// Converts a LLDB module to a VS Code DAP module for use in "modules" events.
///
+/// \param[in] target
+/// A LLDB target object to convert into a JSON value.
+///
/// \param[in] module
/// A LLDB module object to convert into a JSON value
///
/// \return
/// A "Module" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
-llvm::json::Value CreateModule(lldb::SBModule &module);
+llvm::json::Value CreateModule(lldb::SBTarget &target, lldb::SBModule &module);
/// Create a "Event" JSON object using \a event_name as the event name
///
@@ -363,10 +367,15 @@ llvm::json::Value CreateSource(llvm::StringRef source_path);
/// The LLDB stack frame to use when populating out the "StackFrame"
/// object.
///
+/// \param[in] format
+/// The LLDB format to use when populating out the "StackFrame"
+/// object.
+///
/// \return
/// A "StackFrame" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
-llvm::json::Value CreateStackFrame(lldb::SBFrame &frame);
+llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
+ lldb::SBFormat &format);
/// Create a "StackFrame" label object for a LLDB thread.
///
@@ -382,10 +391,14 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame);
/// The LLDB thread to use when populating out the "Thread"
/// object.
///
+/// \param[in] format
+/// The configured formatter for the DAP session.
+///
/// \return
/// A "StackFrame" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
-llvm::json::Value CreateExtendedStackFrameLabel(lldb::SBThread &thread);
+llvm::json::Value CreateExtendedStackFrameLabel(lldb::SBThread &thread,
+ lldb::SBFormat &format);
/// Create a "Thread" object for a LLDB thread object.
///
@@ -400,10 +413,14 @@ llvm::json::Value CreateExtendedStackFrameLabel(lldb::SBThread &thread);
/// The LLDB thread to use when populating out the "Thread"
/// object.
///
+/// \param[in] format
+/// The LLDB format to use when populating out the "Thread"
+/// object.
+///
/// \return
/// A "Thread" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
-llvm::json::Value CreateThread(lldb::SBThread &thread);
+llvm::json::Value CreateThread(lldb::SBThread &thread, lldb::SBFormat &format);
/// Create a "StoppedEvent" object for a LLDB thread object.
///
@@ -419,25 +436,32 @@ llvm::json::Value CreateThread(lldb::SBThread &thread);
/// "allThreadsStopped" - set to True to indicate that all threads
/// stop when any thread stops.
///
+/// \param[in] dap
+/// The DAP session associated with the stopped thread.
+///
/// \param[in] thread
/// The LLDB thread to use when populating out the "StoppedEvent"
/// object.
///
+/// \param[in] stop_id
+/// The stop id for this event.
+///
/// \return
/// A "StoppedEvent" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
-llvm::json::Value CreateThreadStopped(lldb::SBThread &thread, uint32_t stop_id);
+llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread,
+ uint32_t stop_id);
/// \return
/// The variable name of \a value or a default placeholder.
-const char *GetNonNullVariableName(lldb::SBValue value);
+const char *GetNonNullVariableName(lldb::SBValue &value);
/// VSCode can't display two variables with the same name, so we need to
/// distinguish them by using a suffix.
///
/// If the source and line information is present, we use it as the suffix.
/// Otherwise, we fallback to the variable address or register location.
-std::string CreateUniqueVariableNameForDisplay(lldb::SBValue v,
+std::string CreateUniqueVariableNameForDisplay(lldb::SBValue &v,
bool is_name_duplicated);
/// Helper struct that parses the metadata of an \a lldb::SBValue and produces
@@ -464,8 +488,8 @@ struct VariableDescription {
/// The SBValue for this variable.
lldb::SBValue v;
- VariableDescription(lldb::SBValue v, bool format_hex = false,
- bool is_name_duplicated = false,
+ VariableDescription(lldb::SBValue v, bool auto_variable_summaries,
+ bool format_hex = false, bool is_name_duplicated = false,
std::optional<std::string> custom_name = {});
/// Create a JSON object that represents these extensions to the DAP variable
@@ -514,9 +538,12 @@ std::pair<int64_t, bool> UnpackLocation(int64_t location_id);
/// properties.
///
/// \param[in] format_hex
-/// It set to true the variable will be formatted as hex in
+/// If set to true the variable will be formatted as hex in
/// the "value" key value pair for the value of the variable.
///
+/// \param[in] auto_variable_summaries
+/// IF set to true the variable will create an automatic variable summary.
+///
/// \param[in] is_name_duplicated
/// Whether the same variable name appears multiple times within the same
/// context (e.g. locals). This can happen due to shadowed variables in
@@ -533,11 +560,12 @@ std::pair<int64_t, bool> UnpackLocation(int64_t location_id);
/// A "Variable" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
- bool format_hex,
+ bool format_hex, bool auto_variable_summaries,
+ bool synthetic_child_debugging,
bool is_name_duplicated = false,
std::optional<std::string> custom_name = {});
-llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit unit);
+llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit);
/// Create a runInTerminal reverse request object
///
@@ -570,7 +598,7 @@ CreateRunInTerminalReverseRequest(const llvm...
[truncated]
|
Comment on lines
-1468
to
-1472
| if (key.empty()) | ||
| g_dap.SendOutput(OutputType::Stderr, | ||
| "empty environment variable for value: \"" + | ||
| value.str() + '\"'); | ||
| else |
There was a problem hiding this comment.
Why did you remove this?
There was a problem hiding this comment.
I guess I wasn't really sure why this is a warning.
In lldb I can use settings set target.env-vars FOO= and there isn't any warnings when I run a process and having an empty env isn't an issue on mac or linux as far as I can tell.
I also don't see any mentions of why this was added in #106919
I could move the warning into lldb-dap.cpp after we call this helper.
There was a problem hiding this comment.
In lldb I can use
settings set target.env-vars FOO=[...]
Note that this warning warns about key.empty(), not value.empty(), i.e. for =FOO and not FOO=.
That being said, I am also not sure how much value this warning provides
There was a problem hiding this comment.
Ah, your right, I mixed that up. Trying to set an empty key in lldb produces:
(lldb) settings set target.env-vars =1
error: empty dictionary key
However, if I have "env": ["=123"], in my launch.json I do end up with the empty "" being set at runtime, at least on macOS, which is kinda of wild that it works.
(vsocde-repl) `expr (char*)getenv("")
(lldb) expr (char*)getenv("")
(char *) $0 = 0x000000016fdff65f "123"
vogelsgesang
changed the title
g_dap and instead passing in required arguments.g_dap and instead passing in required arguments.
vogelsgesang
approved these changes
Nov 9, 2024
labath
approved these changes
Nov 11, 2024
walter-erquinigo
approved these changes
Nov 12, 2024
JDevlieghere
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Feb 25, 2025
…ng in required arguments. (llvm#115561) This is part of a larger refactor to remove the global `g_dap` variable. (cherry picked from commit faaf2db)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is part of a larger refactor to remove the global
g_dapvariable.