[wasm][debugger] Don't fail in presence of multicast delegates#41685
Closed
radical wants to merge 12 commits intodotnet:masterfrom
Closed
[wasm][debugger] Don't fail in presence of multicast delegates#41685radical wants to merge 12 commits intodotnet:masterfrom
radical wants to merge 12 commits intodotnet:masterfrom
Conversation
- this ensures that we check the datetime, and some property getters on it, wherever we have a datetime.
- surface inherited fields, and properties
- we try to support `Runtime.getProperties`'s two arguments:
- `ownProperties`, and `accessorsOnly`
- `ownProperties`: for JS, this means return only the object's own
members (not inherited ones)
- `accessorsOnly`: for JS, this means return all the getters
Actual implementation:
- In practice, VSCode, and Chrome debugger seem to only send
`{ ownProperties: true, accessorsOnly: false }`,
and `{ ownProperties: false, accessorsOnly: true }`. The combination of
which means - that we don't return any inherited fields!
- But we want to show inherited fields too, so to get that behavior we
essentially *ignore* `ownProperties`. IOW,
- `ownProperties`: we return all fields, and properties
- `accessorsOnly`: we return only the getters, including the
inherited ones
- Another thing to note is the case for auto-properties
- these have a backing field
- and we usually return the backing field's value, instead of
returning a getter
- To continue with that, auto-properties are *not* returned for
`accessorsOnly`
- The code in `mini-wasm-debugger.c` does handle these two arguments,
but that is currently disabled by not passing the args to debugger.c at
all
- Instead, we get the *full* list of members, and try to filter it
in `library_mono.js`
- which includes handling property overrides, or shadowing by new
properties/fields in derived classes
- When we have a struct local in an async instance method, it doesn't
get expanded, since we have a containerId (the async object), and we can
expand/access it later.
- When the IDE asks us to expand it with `{accessorPropertiesOnly: true}`:
- we get the expanded json, but `_filter_automatic_properties` tries
to return just the accessors, but that doesn't handle the expanded
members of nested structs!
- That is done in `extract_and_cache_value_types`, which is run *after*
`_filter_automatic_properties`, but by that time we have already
lost the expanded members!
- So, `_get_vt_properties` fails with `Unknown valuetype id`,
because it doesn't have anything to return at that point.
- This is being solved by ignoring the getProperties args in case of
expanding valuetypes.
- that means that we can correctly extract, and cache the whole
object.
- And after that, we can return accessors/others, based on the args.
.. property. Some times we might not get a `value`, or `name`, or it might instead have a `get`. Handle those cases correctly when combining the name/value/get objects. This showed up in case of a `MulticastDelegate`, where we didn't have a `value`, and ended up incorrectly combining the name/value objects, thus returning incorrect locals.
- Essentially we just surface these as a symbol showing the type name
radical
requested review from
CoffeeFlux,
SamMonoRT,
lambdageek,
marek-safar and
vargaz
as code owners
|
Tagging subscribers to this area: @thaystg |
Member
Author
|
Based on #41480 , so it includes changes from that. I can rebase once that gets merged. |
radical
added
the
NO-MERGE
radical
removed
the
NO-MERGE
ghost
locked as resolved and limited conversation to collaborators
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In presence of a
MulticastDelegateobject as a field/local, we had a bug with handling that member but ended up not being able return any of the members at all. So, try to skip such cases, so we can return others at least.And then fix that multicast delegate case. Essentially, we just return a "symbol" with the typename.