Skip to content

Commit

Permalink
[trace-view] Fixed displaying of compound reference values (#20752)
Browse files Browse the repository at this point in the history 
## Description 

When displaying references to structs/enums, we need to store the index
path to a specific portion of the struct/enum so that we can display
only the "innermost" portion of the whole referenced values. Consider
the following code:
```move
public struct SomeStruct has drop {
    struct_field: VecStruct,
}

public struct VecStruct has drop, copy {
    vec_field: vector<u64>,
}

fun bar(vec_ref: &mut vector<u64>): u64 {
    let e = vector::borrow_mut(vec_ref, 0);
    *e = 42;
    vec_ref[0]
}

fun foo(some_struct_ref: &mut SomeStruct): u64 {
    let res = bar(&mut some_struct_ref.struct_field.vec_field);
    res + some_struct_ref.struct_field.vec_field[0]
}

fun some_struct(): SomeStruct {
    SomeStruct {
        struct_field: VecStruct { vec_field: vector::singleton(0) }
    }
}

#[test]
fun test() {
    let mut some_struct = some_struct();
    some_struct.struct_field.vec_field.push_back(7);
    foo(&mut some_struct);
}
```

In `bar`, we want to display just the vector and its values even though
this vector is part of a larger nested struct. Previously, without index
paths being available, we would see the entire struct:

![image](https://github.com/user-attachments/assets/3eb58ee7-9780-4bf8-814d-aa7e6c50323a)

After changes in this PR, we will correctly only show the content of the
vector:

![image](https://github.com/user-attachments/assets/9024012b-8eb9-47f8-a892-9e44bf09dca2)


## Test plan 

All tests (including the updated ones) must pass
  • Loading branch information
@awelc
awelc authored Jan 8, 2025
1 parent 4a9e509 commit ba84a1b
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 40 deletions.
34 changes: 29 additions & 5 deletions external-crates/move/crates/move-analyzer/trace-adapter/src/adapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,31 +320,45 @@ export class MoveDebugSession extends LoggingDebugSession {
+ ' when converting ref value for frame id '
+ frameID);
}

return this.convertRuntimeValue(local.value, name, type);
const indexPath = [...value.loc.indexPath];
return this.convertRuntimeValue(local.value, name, indexPath, type);
}

/**
* Converts a runtime value to a DAP variable.
*
* @param value variable value
* @param name variable name
* @param indexPath a path to actual value for compound types (e.g, [1, 7] means
* first field/vector element and then seventh field/vector element)
* @param type optional variable type
* @returns a DAP variable.
* @throws Error with a descriptive error message if conversion has failed.
*/
private convertRuntimeValue(
value: RuntimeValueType,
name: string,
indexPath: number[],
type?: string
): DebugProtocol.Variable {
if (typeof value === 'string') {
if (indexPath.length > 0) {
throw new Error('Cannot index into a string');
}
return {
name,
type,
value,
variablesReference: 0
};
} else if (Array.isArray(value)) {
if (indexPath.length > 0) {
const index = indexPath.pop();
if (index === undefined || index >= value.length) {
throw new Error('Index path for an array is invalid');
}
return this.convertRuntimeValue(value[index], name, indexPath, type);
}
const compoundValueReference = this.variableHandles.create(value);
return {
name,
Expand All @@ -353,6 +367,13 @@ export class MoveDebugSession extends LoggingDebugSession {
variablesReference: compoundValueReference
};
} else if ('fields' in value) {
if (indexPath.length > 0) {
const index = indexPath.pop();
if (index === undefined || index >= value.fields.length) {
throw new Error('Index path for a compound type is invalid');
}
return this.convertRuntimeValue(value.fields[index][1], name, indexPath, type);
}
const compoundValueReference = this.variableHandles.create(value);
// use type if available as it will have information about whether
// it's a reference or not (e.g., `&mut 0x42::mod::SomeStruct`),
Expand All @@ -373,6 +394,9 @@ export class MoveDebugSession extends LoggingDebugSession {
variablesReference: compoundValueReference
};
} else {
if (indexPath.length > 0) {
throw new Error('Cannot index into a reference value');
}
return this.convertRefValue(value, name, type);
}
}
Expand All @@ -388,7 +412,7 @@ export class MoveDebugSession extends LoggingDebugSession {
const runtimeVariables = runtimeScope.locals;
runtimeVariables.forEach(v => {
if (v) {
variables.push(this.convertRuntimeValue(v.value, v.name, v.type));
variables.push(this.convertRuntimeValue(v.value, v.name, [], v.type));
}
});
return variables;
Expand All @@ -410,11 +434,11 @@ export class MoveDebugSession extends LoggingDebugSession {
if (Array.isArray(variableHandle)) {
for (let i = 0; i < variableHandle.length; i++) {
const v = variableHandle[i];
variables.push(this.convertRuntimeValue(v, String(i)));
variables.push(this.convertRuntimeValue(v, String(i), []));
}
} else {
variableHandle.fields.forEach(([fname, fvalue]) => {
variables.push(this.convertRuntimeValue(fvalue, fname));
variables.push(this.convertRuntimeValue(fvalue, fname, []));
});
}
}
Expand Down
46 changes: 32 additions & 14 deletions external-crates/move/crates/move-analyzer/trace-adapter/src/runtime.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export type RuntimeValueType = string | CompoundType | IRuntimeRefValue;
export interface IRuntimeVariableLoc {
frameID: number;
localIndex: number;
indexPath: number[];
}

/**
Expand Down Expand Up @@ -789,7 +790,7 @@ export class Runtime extends EventEmitter {
* @returns string representation of the variable.
*/
private varToString(tabs: string, variable: IRuntimeVariable): string {
return this.valueToString(tabs, variable.value, variable.name, variable.type);
return this.valueToString(tabs, variable.value, variable.name, [], variable.type);
}

/**
Expand All @@ -804,7 +805,7 @@ export class Runtime extends EventEmitter {
: compoundValue.type;
let res = '(' + type + ') {\n';
for (const [name, value] of compoundValue.fields) {
res += this.valueToString(tabs + this.singleTab, value, name);
res += this.valueToString(tabs + this.singleTab, value, name, []);
}
res += tabs + '}\n';
return res;
Expand Down Expand Up @@ -839,21 +840,25 @@ export class Runtime extends EventEmitter {
if (!local) {
return res;
}
return this.valueToString(tabs, local.value, name, type);
const indexPath = [...refValue.loc.indexPath];
return this.valueToString(tabs, local.value, name, indexPath, type);
}

/**
* Returns a string representation of a runtime value.
*
* @param value runtime value.
* @param name name of the variable containing the value.
* @param indexPath a path to actual value for compound types (e.g, [1, 7] means
* first field/vector element and then seventh field/vector element)
* @param type optional type of the variable containing the value.
* @returns string representation of the value.
*/
private valueToString(
tabs: string,
value: RuntimeValueType,
name: string,
indexPath: number[],
type?: string
): string {
let res = '';
Expand All @@ -863,19 +868,32 @@ export class Runtime extends EventEmitter {
res += tabs + 'type: ' + type + '\n';
}
} else if (Array.isArray(value)) {
res += tabs + name + ' : [\n';
for (let i = 0; i < value.length; i++) {
res += this.valueToString(tabs + this.singleTab, value[i], String(i));
}
res += tabs + ']\n';
if (type) {
res += tabs + 'type: ' + type + '\n';
if (indexPath.length > 0) {
const index = indexPath.pop();
if (index !== undefined) {
res += this.valueToString(tabs, value[index], name, indexPath, type);
}
} else {
res += tabs + name + ' : [\n';
for (let i = 0; i < value.length; i++) {
res += this.valueToString(tabs + this.singleTab, value[i], String(i), indexPath);
}
res += tabs + ']\n';
if (type) {
res += tabs + 'type: ' + type + '\n';
}
}
return res;
} else if ('fields' in value) {
res += tabs + name + ' : ' + this.compoundValueToString(tabs, value);
if (type) {
res += tabs + 'type: ' + type + '\n';
if (indexPath.length > 0) {
const index = indexPath.pop();
if (index !== undefined) {
res += this.valueToString(tabs, value.fields[index][1], name, indexPath, type);
}
} else {
res += tabs + name + ' : ' + this.compoundValueToString(tabs, value);
if (type) {
res += tabs + 'type: ' + type + '\n';
}
}
} else {
res += this.refValueToString(tabs, value, name, type);
Expand Down
14 changes: 9 additions & 5 deletions external-crates/move/crates/move-analyzer/trace-adapter/src/trace_utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ export function readTrace(
// if a local is read or written, set its end of lifetime
// to infinite (end of frame)
const location = effect.Write ? effect.Write.location : effect.Read!.location;
const loc = processJSONLocalLocation(location, localLifetimeEnds);
const loc = processJSONLocalLocation(location, [], localLifetimeEnds);
if (effect.Write) {
if (loc !== undefined) {
// Process a write only if the location is supported.
Expand Down Expand Up @@ -788,12 +788,15 @@ function JSONTraceAddressToHexString(address: string): string {
* Processes a location of a local variable in a JSON trace: sets the end of its lifetime
* when requested and returns its location
* @param traceLocation location in the trace.
* @param indexPath a path to actual value for compound types (e.g, [1, 7] means
* first field/vector element and then seventh field/vector element)
* @param localLifetimeEnds map of local variable lifetimes (defined if local variable
* lifetime should happen).
* @returns variable location.
*/
function processJSONLocalLocation(
traceLocation: JSONTraceLocation,
indexPath: number[],
localLifetimeEnds?: Map<number, number[]>,
): IRuntimeVariableLoc | undefined {
if ('Local' in traceLocation) {
Expand All @@ -804,9 +807,10 @@ function processJSONLocalLocation(
lifetimeEnds[localIndex] = FRAME_LIFETIME;
localLifetimeEnds.set(frameID, lifetimeEnds);
}
return { frameID, localIndex };
return { frameID, localIndex, indexPath };
} else if ('Indexed' in traceLocation) {
return processJSONLocalLocation(traceLocation.Indexed[0], localLifetimeEnds);
indexPath.push(traceLocation.Indexed[1]);
return processJSONLocalLocation(traceLocation.Indexed[0], indexPath, localLifetimeEnds);
} else {
// Currently, there is nothing that needs to be done for 'Global' locations,
// neither with respect to lifetime nor with respect to location itself.
Expand All @@ -829,14 +833,14 @@ function processJSONLocalLocation(
*/
function traceRefValueFromJSON(value: JSONTraceRefValue): RuntimeValueType {
if ('MutRef' in value) {
const loc = processJSONLocalLocation(value.MutRef.location);
const loc = processJSONLocalLocation(value.MutRef.location, []);
if (!loc) {
throw new Error('Unsupported location type in MutRef');
}
const ret: IRuntimeRefValue = { mutable: true, loc };
return ret;
} else {
const loc = processJSONLocalLocation(value.ImmRef.location);
const loc = processJSONLocalLocation(value.ImmRef.location, []);
if (!loc) {
throw new Error('Unsupported location type in ImmRef');
}
Expand Down
24 changes: 8 additions & 16 deletions external-crates/move/crates/move-analyzer/trace-adapter/tests/references_deep/test.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,10 @@ current frame stack:

function: bar (m.move:13)
scope 0 :
vec_ref : (0x0::m::SomeStruct) {
struct_field : (0x0::m::VecStruct) {
vec_field : [
0 : 0
1 : 7
]
}
}
vec_ref : [
0 : 0
1 : 7
]
type: &mut vector<u64>

current frame stack:
Expand Down Expand Up @@ -62,13 +58,9 @@ current frame stack:

function: bar (m.move:15)
scope 0 :
vec_ref : (0x0::m::SomeStruct) {
struct_field : (0x0::m::VecStruct) {
vec_field : [
0 : 42
1 : 7
]
}
}
vec_ref : [
0 : 42
1 : 7
]
type: &mut vector<u64>

0 comments on commit ba84a1b

Please sign in to comment.