[IDE debugging] Introduce DebugValue for structured value representation for debugging#19791
[IDE debugging] Introduce DebugValue for structured value representation for debugging#19791mkurnikov wants to merge 2 commits into
DebugValue for structured value representation for debugging#19791Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
mkurnikov
force-pushed
the
debug-value
branch
2 times, most recently
from
0f313fd to
327627d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 4 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 327627d. Configure here.
mkurnikov
force-pushed
the
debug-value
branch
2 times, most recently
from
179c698 to
dc8bf8e
Compare
| }; | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub enum DebugValue { |
There was a problem hiding this comment.
design question: why is this needed? In general, adding a new value type makes maintenance harder.
There was a problem hiding this comment.
Local values in the debugger are presented as a trees, think "struct fields", "vector elements", etc.
This struct is a representation of said tree.
It's hard to understand it's usefulness without the general DAP context, I agree. In the original PR (#19580), this data then serialized into JSON and passed to VSCode (or any DAP client) to show to the user.
The alternative would be to add a bunch of pub modifiers in values_impl.rs, and serialize Value straight into JSON, without intermediate DebugValue struct.
There was a problem hiding this comment.
Actually, there's a cross-thread communication happens.
Currently whole DebugContext is Send, if we'd try to pass Value straight to DAP server for serialization, DebugContext would stop being Send which creates a bunch of hard to resolve issues.
So serialization must happen in move-vm-types crate.
What I can do is to make DebugValue an untyped tree type instead, i.e. DapNode { item: String, children: Vec<DapNode> } which would get rid of the typed DebugValue. But implementation would mostly stay the same, there's still going to be recursive walk through the values to collect a value tree.
| } | ||
| } | ||
|
|
||
| fn is_type( |
There was a problem hiding this comment.
from this name, it is a general function but it seems only used in serialize_adt? Will this be used anywhere else? If not, maybe we don't this abstraction?
There was a problem hiding this comment.
I was thinking of special-casing Object<Type> in the future. But for now it can be removed, you're right.
| pub type FieldInfo = (String, Option<Type>); | ||
|
|
||
| #[derive(Clone)] | ||
| pub enum AdtInfo { |
There was a problem hiding this comment.
nit: Adt is not explained anywhere and it is actually structs or enum. Maybe just use StructTypeInfo?
There was a problem hiding this comment.
Yes, it's for the Abstract Data Type.
| .collect(); | ||
| DebugValue::Struct(children) | ||
| }, | ||
| Container::Locals(_) => DebugValue::Error("...".into()), |
There was a problem hiding this comment.
what is "..." here? Maybe add a meaningful error message instead?
|
Superseded by #19899 |
2 participants
Extracted from #19766
Note
Low Risk
Changes are confined to debugging/diagnostic paths (stack traces, enriched locals) with broad unit tests; normal transaction execution is unaffected unless debugging is enabled.
Overview
Introduces a
DebugValuetree and typed serialization path for Move VM debugging, replacing the large inline debug printer invalues_impl.rswith a dedicatedvalues_impl/debug.rsmodule and aTypeResolverhook for ADT/enum/struct names.Runtime wiring:
source_locator::print_locals_enrichednow builds per-slotLocalInfo(name +Type), usesLocatorTypeResolver(source maps +RuntimeEnvironment) withserialize_value_for_debug, and falls back tolocal[idx]when names are missing. The interpreter exposesget_stack_depth()onInterpreterDebugInterfacefor stack traces and VM error dumps.Cleanup: Operand-stack traces still call
values::debug::print_value(untyped resolver).debug_write!/debug_writeln!macros use fully qualifiedPartialVMErrorpaths.Reviewed by Cursor Bugbot for commit 502545a. Bugbot is set up for automated code reviews on this repo. Configure here.