Skip to content

[wasm] [debugger] Eval fixes for static class eval - #61660

Merged
ilonatommy merged 88 commits into
dotnet:mainfrom
ilonatommy:fix-static-attribute-support
Nov 24, 2021
Merged

[wasm] [debugger] Eval fixes for static class eval#61660
ilonatommy merged 88 commits into
dotnet:mainfrom
ilonatommy:fix-static-attribute-support

Conversation

@ilonatommy

@ilonatommyilonatommy commented Nov 16, 2021

Copy link
Copy Markdown
Member

Covering skipped evaluation cases from #61252. To be merged after #61252.

It fixes static classes evaluation and nested static classes evaluation. It changed nested classes access concatenation character: from "/" to ".".
Requires a follow-up Issue to fix the test: EvaluateStaticClassFromStaticMethod with parameters without the full namespace.

ilonatommyand others added 30 commits November 5, 2021 11:57
…chance it will be located out of the current assembly.

@radicalradical left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great. It does cause some test failures though, which need to be fixed.

And there are some additional cases that we should look at, but not needed in this PR.

Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs Outdated
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs Outdated
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs Outdated
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs Outdated
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
@ilonatommy

Copy link
Copy Markdown
MemberAuthor

@radical, as discussed I merged nested static evaluation PR with this one. Only 2 more tests fail on this PR in comparison to main and they are connected with lack of method evaluation without namespace (EvaluateStaticClassFromStaticMethod cannot be found in the same namespace it is declared). It will have to be solved in a separate PR.

@radicalradical left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your patience, LGTM! 👍

@ilonatommy
ilonatommy merged commit 35815de into dotnet:mainNov 24, 2021
@ilonatommy

Copy link
Copy Markdown
MemberAuthor

Fixes #61462.

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasmWebAssembly architecturearea-Debugger-mono

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@ilonatommy@radical@lewing@thaystg