Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetTypeFields should return ELEMENT_TYPE_BYREF. #65880

Merged
merged 3 commits into from
Feb 26, 2022

Conversation

AaronRobinsonMSFT
Copy link
Member

Did not handle the ByReference<T> special case.

There is some white space changes as well and spacing that was very confusing. That is modified in this PR.

/cc @noahfalk @hoyosjs

Did not handle the ByReference<T> special case.
@ghost
Copy link

ghost commented Feb 25, 2022

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

Issue Details

Did not handle the ByReference<T> special case.

There is some white space changes as well and spacing that was very confusing. That is modified in this PR.

/cc @noahfalk @hoyosjs

Author: AaronRobinsonMSFT
Assignees: -
Labels:

area-Diagnostics-coreclr

Milestone: 7.0.0

@AaronRobinsonMSFT AaronRobinsonMSFT changed the title GetTypeField should return ELEMENT_TYPE_BYREF. GetTypeFields should return ELEMENT_TYPE_BYREF. Feb 25, 2022
@tommcdon tommcdon self-requested a review February 25, 2022 15:44
Copy link
Member

@tommcdon tommcdon left a comment

Choose a reason for hiding this comment

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

LGTM, and thanks for adding some code cleanup to the change as well Aaron!

scenario and therefore is more confusing than helpful. Add a commment about
the expectations of the returned values.
@AaronRobinsonMSFT AaronRobinsonMSFT merged commit a3f7d29 into dotnet:main Feb 26, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the dbg_ref_fields branch February 26, 2022 02:38
@ghost ghost locked as resolved and limited conversation to collaborators Mar 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants