[wasm][debugger] View multidimensional array when debugging by thaystg · Pull Request #60983 · dotnet/runtime

thaystg · 2021-10-28T19:22:16Z

Unfortunatelly as JS doesn't have mutidimensional arrays, I have to threat mutidimensional array as an object, as far as I understood for the user experience there isn't any difference in the behavior.

Before PR:

After PR on VS:

After PR on Chrome:

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1417072

…se it uses callFunctionOn

ghost · 2021-10-28T19:22:22Z

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

It's working when debug from chrome but not when debug from VS, because it uses callFunctionOn

Author:	thaystg
Assignees:	-
Labels:	`area-Debugger-mono`
Milestone:	-

ghost · 2021-10-28T20:52:13Z

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

It's working when debug from chrome but not when debug from VS, because it uses callFunctionOn

Author:	thaystg
Assignees:	-
Labels:	`arch-wasm`, `area-Debugger-mono`
Milestone:	-

lewing · 2021-10-28T22:33:32Z

I really wonder why vscode-js-debug uses callFunctionOn rather than the protocol bits.

lewing · 2021-11-14T01:08:56Z

 var length = retDebuggerCmdReader.ReadInt32();
- length = retDebuggerCmdReader.ReadInt32();
- return length;
+ var arrDimensions = new ArrayDimensions(length);


should this be var rank = new int[length] then new ArrayDimensions(rank) and forgo the resizing?

And populate rank, before new ArrayDimensions, so TotalLength can be precomputed too. And ArrayDimensions can be a record.

…radical

…radical.

radical · 2021-11-15T02:00:10Z

 return null;
 }
- public async Task<int> GetArrayLength(SessionId sessionId, int object_id, CancellationToken token)
+ public async Task<ArrayDimensions> GetArrayLength(SessionId sessionId, int object_id, CancellationToken token)


rename to GetArrayDimensions

radical · 2021-11-16T06:46:53Z

+ for (int i = 0 ; i < length; i++)
+ {
+ rank[i] = retDebuggerCmdReader.ReadInt32();
+ retDebuggerCmdReader.ReadInt32();


What is this reading? comment would be helpful

radical · 2021-11-16T06:47:58Z

 commandParamsWriter.Write(arrayId);
 commandParamsWriter.Write(0);
- commandParamsWriter.Write(length);
+ commandParamsWriter.Write(length.TotalLength);


nit: length -> dimensions?

radical · 2021-11-16T06:48:53Z

+ for (int i = 0 ; i < length.TotalLength; i++)
 {
- var var_json = await CreateJObjectForVariableValue(sessionId, retDebuggerCmdReader, i.ToString(), false, -1, false, token);
+ var var_json = await CreateJObjectForVariableValue(sessionId, retDebuggerCmdReader, length.GetArrayIndexString(i), false, -1, false, token);


nit: param name for the bools at least

radical · 2021-11-16T06:53:24Z

+ arrayType = arrayType.Insert(arrayType.LastIndexOf('[')+1, length.ToString());
+ if (className.LastIndexOf('[') > 0)
+ className = className.Insert(arrayType.LastIndexOf('[')+1, new string(',', length.Rank-1));
+ return CreateJObject<string>(null, "object", arrayType, false, className.ToString(), "dotnet:array:" + objectId, null, length.Rank == 1 ? "array" : null);


nit: param names, a bit confusing to read otherwise

radical · 2021-11-16T07:01:52Z

+ var length = await GetArrayLength(sessionId, arrayId, token);
+ var arrayProxy = JObject.FromObject(new
+ {
+ items = await GetArrayValues(sessionId, arrayId, token),


GetArrayValues also calls GetArrayLength. Instead maybe GetArrayValues can return (JArray, ArrayDimensions), or it can take a `ArrayDimensions argument?

radical · 2021-11-16T07:10:21Z

+ int boundLimit = 1;
+ int lastBoundLimit = 1;
+ int[] arrayStr = new int[Rank];
+ int rankStart = 0;


Check for idx < 0, and idx >= TotalLength

radical

LGTM! thank you for your patience :)

thaystg · 2021-12-01T20:35:25Z

/backport to release/6.0

github-actions · 2021-12-01T20:35:40Z

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1527691382

github-actions · 2021-12-01T20:37:18Z

@thaystg backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: It's working when debug from chrome but not when debug from VS, because it uses callFunctionOnApplying: Remove unrelated change.Applying: Working also on VS.Applying: Working also on VS.Using index info to reconstruct a base tree...A	src/mono/wasm/runtime/debug.tsFalling back to patching base and 3-way merge...CONFLICT (modify/delete): src/mono/wasm/runtime/debug.ts deleted in HEAD and modified in Working also on VS.. Version Working also on VS. of src/mono/wasm/runtime/debug.ts left in tree.error: Failed to merge in the changes.hint: Use 'git am --show-current-patch=diff' to see the failed patchPatch failed at 0004 Working also on VS.When you have resolved this problem, run "git am --continue".If you prefer to skip this patch, run "git am --skip" instead.To restore the original branch and stop patching, run "git am --abort".Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

thaystg added 2 commits October 27, 2021 18:13

It's working when debug from chrome but not when debug from VS, becau…
38368ca
…se it uses callFunctionOn

Remove unrelated change.
24a3cbe

ghost added the area-Debugger-mono label Oct 28, 2021

radical added the arch-wasm WebAssembly architecture label Oct 28, 2021

thaystg added 4 commits November 3, 2021 11:42

Working also on VS.
8b59dab

Merge branch 'dotnet:main' into thays_multidimensional_array
e46fe72

Working also on VS.
79255ae

merge main
19ef4a8

thaystg marked this pull request as ready for review November 3, 2021 15:06

thaystg requested a review from marek-safar as a code owner November 3, 2021 15:06

thaystg requested review from lewing and radical November 3, 2021 15:07

lewing reviewed Nov 14, 2021
View reviewed changes

thaystg added 2 commits November 15, 2021 12:57

Addressing@lewingand@radicalcomments
ec07dd1

Change ArrayDimensions to be a record and not a class as suggested by@…
3a80364
…radical.

radical requested changes Nov 16, 2021
View reviewed changes

Addressing@radicalcomments.
1d9162a

radical approved these changes Nov 16, 2021
View reviewed changes

lewing approved these changes Nov 16, 2021
View reviewed changes

thaystg merged commit 293e5ed into dotnet:mainNov 16, 2021

thaystg mentioned this pull request Dec 2, 2021
[release/6.0] [wasm][debugger] View multidimensional array when debugging #62283
Merged

ghost locked as resolved and limited conversation to collaborators Jan 1, 2022

Uh oh!

Conversation

thaystg commented Oct 28, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Oct 28, 2021

Uh oh!

ghost commented Oct 28, 2021

Uh oh!

lewing commented Oct 28, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

radical left a comment

Choose a reason for hiding this comment

Uh oh!

thaystg commented Dec 1, 2021

Uh oh!

github-actionsBot commented Dec 1, 2021

Uh oh!

github-actionsBot commented Dec 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thaystg commented Oct 28, 2021•
edited
Loading