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
Allow the PSGetMemberBinder
to handle ByRef property
#16956
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this LGTM, I think it's the right solution 🎉
I do wish we could hook this up to the set binder. But unless PropertyInfo.SetValue
supports it for ref values now (last time I looked it didn't Edit: and still doesn't) that probably isn't possible atm.
Very happy we can at least get the value though, well done!
Side note, I did also briefly try a intermediary static T Deref<T>(ref T value) => value
method but that's a bust unfortunately
I can see there is a possibility that we run into a similar issue with a method that has a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
🎉 Handy links: |
PR Summary
Fix #14632
Allow the
PSGetMemberBinder
to handleByRef
property.There is no way to work with ByRef type directly in expression tree, so we turn to reflection in this case.
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.