[wasm][debugger] Fix debugger behavior when the type has ToString method overridden #76780
Conversation
…the result of the ToString and not the type name.
|
Tagging subscribers to this area: @thaystg Issue DetailsThe description of an object that has ToString implemented should bethe result of the ToString and not the type name. Fixes #76748 Using this PR:
|
src/mono/wasm/debugger/BrowserDebugProxy/JObjectValueCreator.cs
Outdated
Show resolved
Hide resolved
radical
changed the title
Implement support for value types with ToString overriden. Fix Debugger* is more relevant than ToString
| if (typeInfo.Name == "object") | ||
| return null; | ||
| Microsoft.WebAssembly.Diagnostics.MethodInfo methodInfo = typeInfo.Info.Methods.FirstOrDefault(m => m.Name == "ToString"); | ||
| if (!IsEnum && methodInfo == null) |
There was a problem hiding this comment.
Can this method exit early in case of IsEnum==true?
There was a problem hiding this comment.
No should be &&, if it's a enum we want to call ToString even if we cannot find the method.
There was a problem hiding this comment.
We don't get the method in case of an enum? Is that the case for other valuetypes? record struct?
There was a problem hiding this comment.
Only for enum, I added tests for record also.
There was a problem hiding this comment.
Are no methods returned for enums at all? Is that a debugger-agent thing?
There was a problem hiding this comment.
No methods returned for enums at all, not sure if it's a debugger-agent thing or if they are really not implemented in the enum, they are somewhere like in a "base class".
src/mono/wasm/debugger/BrowserDebugProxy/JObjectValueCreator.cs
Outdated
Show resolved
Hide resolved
src/mono/wasm/debugger/BrowserDebugProxy/JObjectValueCreator.cs
Outdated
Show resolved
Hide resolved
src/mono/wasm/debugger/BrowserDebugProxy/JObjectValueCreator.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Ankit Jain <radical@gmail.com>
…t on objects and valuetypes.
src/mono/wasm/debugger/BrowserDebugProxy/JObjectValueCreator.cs
Outdated
Show resolved
Hide resolved
| @@ -1839,6 +1839,43 @@ public Task<JObject> InvokeMethod(DotnetObjectId dotnetObjectId, CancellationTok | |||
| : throw new ArgumentException($"Cannot invoke method with id {methodId} on {dotnetObjectId}", nameof(dotnetObjectId)); | |||
| } | |||
|
|
|||
| public async Task<string> InvokeToStringAsync(List<int> typeIds, bool isValueType, bool isEnum, int objectId, BindingFlags extraFlags, CancellationToken token) | |||
There was a problem hiding this comment.
nit: extraFlags could have BindingFlags.Default as the default value.
Co-authored-by: Ankit Jain <radical@gmail.com>
| foreach (var methodId in methodIds) | ||
| { | ||
| var methodInfoFromRuntime = await GetMethodInfo(methodId, token); | ||
| if (methodInfoFromRuntime.Info.GetParametersInfo().Length > 0) |
There was a problem hiding this comment.
MethodInfoWithDebugInformation implements GetParametersInfo(), so we could shorten:
| if (methodInfoFromRuntime.Info.GetParametersInfo().Length > 0) | |
| if (methodInfoFromRuntime.GetParametersInfo().Length > 0) |
ghost
locked as resolved and limited conversation to collaborators
The description of an object that has ToString implemented should bethe result of the ToString and not the type name.
Fixes #76748
Using this PR:
