[wasm][debugger] Improvements in debugging in async methods by thaystg · Pull Request #78651 · dotnet/runtime

thaystg · 2022-11-21T19:55:18Z

Variables with same name in different scopes in async method (C# and VB) (Before this PR it was always showing the value of the last variable declared)
Variables in async methods in VB language
Method names of async methods in VB language

Fixes#77031
Fixes#77481

…hod.

ghost · 2022-11-21T19:55:36Z

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

Issue Details

Fixing 2 variables with same names in different scopes in an async method.

Before this PR it was always showing the value of the last variable declared.

Fixes #77031

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

…de, and also fixing 2 variables with same name in differente scopes, and also fixing method name for async methods in VB.

radical · 2022-11-21T23:32:58Z


 internal readonly ILogger logger;
- private static readonly Regex regexForAsyncLocals = new (@"\<([^)]*)\>", RegexOptions.Singleline);
+ private static readonly Regex regexForAsyncLocals = new (@"\<([^)]*)\>([^)]*)([_][_])([0-9]*)", RegexOptions.Singleline);


Can you add some example strings that we are trying to match?

radical · 2022-11-29T09:31:13Z

 public static bool operator !=(SourceId a, SourceId b) => !a.Equals(b);
 }

+ internal sealed record Scope (int Id, int StartOffset, int EndOffset);


Suggested change
internalsealedrecordScope(intId,intStartOffset,intEndOffset);
internalsealedrecordScope(intId,intStartOffset,intEndOffset);

Rename to AsyncScope, or AsyncScopeDebugInformation. And it can be a private nested type of MethodInfo.

radical · 2022-11-29T12:00:22Z

I pushed the changes that I suggested to https://github.com/radical/runtime/commits/aj-thays_fix_77031 .

radical · 2022-11-29T16:56:18Z

+ }
 else
 {
 asyncLocalsFull.Add(asyncLocal);


Should this be unconditional? It seems to be common, for the cases we don't continue earlier.

radical · 2022-11-29T16:56:34Z

+ var match = regexForVBAsyncMethodName.Match(klassName);
+ ret = ret.Insert(0, match.Groups[2].Value);


Check match.Success .

radical

Other than the comments, LGTM 👍

thaystg · 2022-12-12T15:00:35Z

/backport to release/7.0

github-actions · 2022-12-12T15:00:54Z

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3676987035

github-actions · 2022-12-12T15:02:49Z

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

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fixing 2 variables with same names in different scopes in a async method.
Using index info to reconstruct a base tree...
M	src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
M	src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
M	src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
CONFLICT (content): Merge conflict in src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
Auto-merging src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
Auto-merging src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fixing 2 variables with same names in different scopes in a async method.
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!

github-actions · 2022-12-12T15:02:51Z

@thaystg an error occurred while backporting to release/7.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

Fixing 2 variables with same names in different scopes in a async met…
c37ec32
…hod.

thaystg requested review from ilonatommy and lewing November 21, 2022 19:55

thaystg requested a review from radical as a code owner November 21, 2022 19:55

ghost added the area-Debugger-mono label Nov 21, 2022

ghost assigned thaystg Nov 21, 2022

This was referenced Nov 21, 2022
[wasm] add support for retrying debugger tests when chrome launch times out #76528
Closed
[wasm][debugger] DebuggerTests.AssignmentTests.InspectVariableBeforeAndAfterAssignment failing #64188
Closed

Implementing support to see local variables in async methods in VB co…
15a53ec
…de, and also fixing 2 variables with same name in differente scopes, and also fixing method name for async methods in VB.

thaystg changed the title ~~[wasm][debugger] Variables with same name in different scopes in async method~~[wasm][debugger] Various fix debugging in async methodsNov 22, 2022

thaystg changed the title ~~[wasm][debugger] Various fix debugging in async methods~~[wasm][debugger] Improviments in debugging in async methodsNov 22, 2022

Adding Test case as suggested by@radical
3c704f0

build-analysisBot mentioned this pull request Nov 22, 2022
[wasm] S.R.IS.JavaScript.Tests.JSImportExportTest.JsImportTaskTypes#77334
Closed

thaystg changed the title ~~[wasm][debugger] Improviments in debugging in async methods~~[wasm][debugger] Improvements in debugging in async methodsNov 23, 2022

ilonatommy approved these changes Nov 23, 2022
View reviewed changes

radical requested changes Nov 29, 2022
View reviewed changes

ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label Nov 29, 2022

radical added 4 commits November 29, 2022 10:45

Refactor Scope class
7830b1c

slight test changes
83ca78d

Split the tests, to be more readable in code, and in test results
6914711

cleanup
c23bcd2

ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Nov 29, 2022

thaystg added 3 commits November 29, 2022 10:47

Merge branch 'dotnet:main' into thays_fix_77031
6b87c08

addressing radical comments
0dd3980

addressing radical comments, fix test case
614db8f

radical reviewed Nov 29, 2022
View reviewed changes

radical approved these changes Nov 29, 2022
View reviewed changes

thaystg added 2 commits November 29, 2022 14:10

addressing radical comments
716b598

addressing@radicalcomments
568e617

radical approved these changes Nov 29, 2022
View reviewed changes

radical mentioned this pull request Nov 29, 2022
[wasm][debugger] Improve some regex patterns in MonoSDBHelper.cs#78979
Closed

thaystg merged commit 26e1ca5 into dotnet:mainNov 29, 2022

thaystg mentioned this pull request Dec 12, 2022
[release/7.0][wasm][debugger] Improvements in debugging in async methods #79542
Merged

ghost locked as resolved and limited conversation to collaborators Jan 11, 2023

	internalsealedrecordScope(intId,intStartOffset,intEndOffset);
	internalsealedrecordScope(intId,intStartOffset,intEndOffset);

		var match = regexForVBAsyncMethodName.Match(klassName);
		ret = ret.Insert(0, match.Groups[2].Value);

Uh oh!

Conversation

thaystg commented Nov 21, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Nov 21, 2022

Uh oh!

radicalNov 21, 2022

Choose a reason for hiding this comment

Uh oh!

radicalNov 29, 2022

Choose a reason for hiding this comment

Uh oh!

radicalNov 29, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

radical commented Nov 29, 2022

Uh oh!

radicalNov 29, 2022

Choose a reason for hiding this comment

Uh oh!

radicalNov 29, 2022

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 12, 2022

Uh oh!

github-actionsBot commented Dec 12, 2022

Uh oh!

github-actionsBot commented Dec 12, 2022

Uh oh!

github-actionsBot commented Dec 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thaystg commented Nov 21, 2022•
edited
Loading