Skip to content

Commit f8adf66

Browse files
committed
Improve variables view
We used to have two scopes of variables, "locals" and "arguments". However, VM somehow returns incorrect results for LocalVariable#isArgument(), so we presented confusing views to users. Since it's not very important to distinguish arguments and local variables, we now include them in the same "locals" scope. Instead, we introduce the new "fields" scope containing fields of the current object (aka "this"). And finally, we now support inspecting the hierarchy of objects.
1 parent ca503f1 commit f8adf66

File tree

1 file changed

+133
-18
lines changed

1 file changed

+133
-18
lines changed

src/main/java/org/javacs/debug/JavaDebugServer.java

Lines changed: 133 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.nio.file.Path;
1414
import java.nio.file.Paths;
1515
import java.util.ArrayList;
16+
import java.util.HashMap;
1617
import java.util.HashSet;
1718
import java.util.List;
1819
import java.util.Objects;
@@ -440,6 +441,7 @@ public void terminate(TerminateArguments req) {
440441

441442
@Override
442443
public void continue_(ContinueArguments req) {
444+
valueIdTracker.clear();
443445
vm.resume();
444446
}
445447

@@ -454,6 +456,7 @@ public void next(NextArguments req) {
454456
var step = vm.eventRequestManager().createStepRequest(thread, StepRequest.STEP_LINE, StepRequest.STEP_OVER);
455457
step.addCountFilter(1);
456458
step.enable();
459+
valueIdTracker.clear();
457460
vm.resume();
458461
}
459462

@@ -468,6 +471,7 @@ public void stepIn(StepInArguments req) {
468471
var step = vm.eventRequestManager().createStepRequest(thread, StepRequest.STEP_LINE, StepRequest.STEP_INTO);
469472
step.addCountFilter(1);
470473
step.enable();
474+
valueIdTracker.clear();
471475
vm.resume();
472476
}
473477

@@ -482,6 +486,7 @@ public void stepOut(StepOutArguments req) {
482486
var step = vm.eventRequestManager().createStepRequest(thread, StepRequest.STEP_LINE, StepRequest.STEP_OUT);
483487
step.addCountFilter(1);
484488
step.enable();
489+
valueIdTracker.clear();
485490
vm.resume();
486491
}
487492

@@ -632,47 +637,157 @@ private com.sun.jdi.StackFrame findFrame(long id) {
632637
@Override
633638
public ScopesResponseBody scopes(ScopesArguments req) {
634639
var resp = new ScopesResponseBody();
640+
var fields = new Scope();
641+
fields.name = "Fields";
642+
fields.presentationHint = "locals";
643+
fields.expensive = true; // do not expand by default
644+
fields.variablesReference = req.frameId * 2;
635645
var locals = new Scope();
636646
locals.name = "Locals";
637647
locals.presentationHint = "locals";
638-
locals.variablesReference = req.frameId * 2;
639-
var arguments = new Scope();
640-
arguments.name = "Arguments";
641-
arguments.presentationHint = "arguments";
642-
arguments.variablesReference = req.frameId * 2 + 1;
643-
resp.scopes = new Scope[] {locals, arguments};
648+
locals.variablesReference = req.frameId * 2 + 1;
649+
resp.scopes = new Scope[] {fields, locals};
644650
return resp;
645651
}
646652

653+
private static final long VALUE_ID_START = 1000000000;
654+
655+
private static class ValueIdTracker {
656+
private final HashMap<Long, Value> values = new HashMap<>();
657+
private long nextId = VALUE_ID_START;
658+
659+
public void clear() {
660+
values.clear();
661+
// Keep nextId to avoid accidentally accessing wrong Values.
662+
}
663+
664+
public Value get(long id) {
665+
return values.get(id);
666+
}
667+
668+
public long put(Value value) {
669+
long id = nextId++;
670+
values.put(id, value);
671+
return id;
672+
}
673+
}
674+
675+
private final ValueIdTracker valueIdTracker = new ValueIdTracker();
676+
677+
private static boolean hasInterestingChildren(Value value) {
678+
return value instanceof ObjectReference && !(value instanceof StringReference);
679+
}
680+
647681
@Override
648682
public VariablesResponseBody variables(VariablesArguments req) {
649-
var frameId = req.variablesReference / 2;
650-
var scopeId = (int) (req.variablesReference % 2);
651-
var argumentScope = scopeId == 1;
683+
if (req.variablesReference < VALUE_ID_START) {
684+
var frameId = req.variablesReference / 2;
685+
var scopeId = (int)(req.variablesReference % 2);
686+
return frameVariables(frameId, scopeId);
687+
}
688+
Value value = valueIdTracker.get(req.variablesReference);
689+
return valueChildren(value);
690+
}
691+
692+
private VariablesResponseBody frameVariables(long frameId, int scopeId) {
652693
var frame = findFrame(frameId);
694+
var thread = frame.thread();
695+
var variables = new ArrayList<Variable>();
696+
697+
if (scopeId == 0) {
698+
var thisValue = frame.thisObject();
699+
if (thisValue != null) {
700+
variables.addAll(objectFieldsAsVariables(thisValue, thread));
701+
}
702+
} else {
703+
variables.addAll(frameLocalsAsVariables(frame, thread));
704+
}
705+
706+
var resp = new VariablesResponseBody();
707+
resp.variables = variables.toArray(Variable[]::new);
708+
return resp;
709+
}
710+
711+
private VariablesResponseBody valueChildren(Value parentValue) {
712+
// TODO: Use an actual owner thread.
713+
ThreadReference mainThread = vm.allThreads().get(0);
714+
var variables = new ArrayList<Variable>();
715+
716+
if (parentValue instanceof ArrayReference array) {
717+
variables.addAll(arrayElementsAsVariables(array, mainThread));
718+
} else if (parentValue instanceof ObjectReference object) {
719+
variables.addAll(objectFieldsAsVariables(object, mainThread));
720+
}
721+
722+
var resp = new VariablesResponseBody();
723+
resp.variables = variables.toArray(Variable[]::new);
724+
return resp;
725+
}
726+
727+
private List<Variable> frameLocalsAsVariables(com.sun.jdi.StackFrame frame, ThreadReference thread) {
653728
List<LocalVariable> visible;
654729
try {
655730
visible = frame.visibleVariables();
656731
} catch (AbsentInformationException __) {
657732
LOG.warning(String.format("No visible variable information in %s", frame.location()));
658-
return new VariablesResponseBody();
733+
return List.of();
659734
}
660-
var values = frame.getValues(visible);
661-
var thread = frame.thread();
735+
662736
var variables = new ArrayList<Variable>();
737+
var values = frame.getValues(visible);
663738
for (var v : visible) {
664-
if (v.isArgument() != argumentScope) continue;
739+
var value = values.get(v);
665740
var w = new Variable();
666741
w.name = v.name();
667-
w.value = print(values.get(v), thread);
742+
w.value = print(value, thread);
668743
w.type = v.typeName();
669-
// TODO set variablesReference and allow inspecting structure of collections and POJOs
744+
if (hasInterestingChildren(value)) {
745+
w.variablesReference = valueIdTracker.put(value);
746+
}
747+
if (value instanceof ArrayReference array) {
748+
w.indexedVariables = array.length();
749+
}
670750
// TODO set variablePresentationHint
671751
variables.add(w);
672752
}
673-
var resp = new VariablesResponseBody();
674-
resp.variables = variables.toArray(Variable[]::new);
675-
return resp;
753+
return variables;
754+
}
755+
756+
private List<Variable> arrayElementsAsVariables(ArrayReference array, ThreadReference thread) {
757+
var variables = new ArrayList<Variable>();
758+
var arrayType = (ArrayType) array.type();
759+
var values = array.getValues();
760+
var length = values.size();
761+
for (int i = 0; i < length; i++) {
762+
var value = values.get(i);
763+
var w = new Variable();
764+
w.name = Integer.toString(i, 10);
765+
w.value = print(value, thread);
766+
w.type = arrayType.componentTypeName();
767+
if (hasInterestingChildren(value)) {
768+
w.variablesReference = valueIdTracker.put(value);
769+
}
770+
variables.add(w);
771+
}
772+
return variables;
773+
}
774+
775+
private List<Variable> objectFieldsAsVariables(ObjectReference object, ThreadReference thread) {
776+
var variables = new ArrayList<Variable>();
777+
var classType = (ClassType) object.type();
778+
var values = object.getValues(classType.allFields());
779+
for (var field : values.keySet()) {
780+
var value = values.get(field);
781+
var w = new Variable();
782+
w.name = field.name();
783+
w.value = print(value, thread);
784+
w.type = field.typeName();
785+
if (hasInterestingChildren(value)) {
786+
w.variablesReference = valueIdTracker.put(value);
787+
}
788+
variables.add(w);
789+
}
790+
return variables;
676791
}
677792

678793
private String print(Value value, ThreadReference t) {

0 commit comments

Comments
 (0)