-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
API Change Request: Expression support for ref and readonly ref types #24884
Comments
I think we should change it to |
Good idea @OmarTawfik and I can't say I was overly happy with |
Can you please update the rest of the proposal if you agree with my previous comment? for example: |
Oops. I think that's the only one I missed (fixed now) unless there's "Paris in the the Spring" mistakes left. |
cc @VSadov in case he had comments on the API. |
@jaredpar, based on our conversation it seems the language team + consumers (e.g. EF) should drive the design. Is that you or someone in particular we should tag? |
Attempts to support All that reflection has in this area is copyback via an object array for Are we starting on the |
Ignoring the reflection issues, the proposal here makes sense. It may be worth some prototyping to see if there is something missing. I think there will be a need for an assignment expression to be able to tell whether it is a |
There is a general item on the compiler team to figure out how expression trees are going to be evolved (and if). |
@terrajobst you are scaring me and my customers. I built huge pieces of infrastructure for many customers that heavily depends on the lightweight generation and compiling offered by the expression trees. |
The reason we haven't touched expression trees in a while is precisely because out of fear of breaking consumers. We have seen cases where Roslyn's code gen for the trees was slightly different from the original C# compiler that was implemented in C++ and that broke consumers because they made assumptions around the shape of those trees. It seems to me that updating this feature makes sense, but it's a work item that is larger than us just changing the APIs. We have to coordinate that change with the compiler teams and with popular linq providers -- otherwise it's not helping anyone. |
I totally understand your points. While retro-compatibility on the current implementation is very important, I would welcome any of the two possible solutions coming in my mind:
Probably choosing to create a new API would be not welcome from many because it would require migrating (very complex) code. But I would personally be happy to migrate in change of powerful features like:
Thanks |
Linking dotnet/csharplang#2545 which covers this from the language side and has further links to prototypes and proposals, including:
|
Currently expressions represents
ref
types only in one place, viz.ParameterExpression
objects for whichIsByRef
is true.With a greater use of
ref
types due to greater language support, it seems fruitful to increase the availability ofref
types in other expressions. While allowing such types directly on expressions'Type
would be one reasonable approach (so that e.g.exp.Type == typeof(int).MakeByRefType()
could be true) this has two problems:ParameterExpression
.There is also metadata support for
ref
types for which the address should not be written through (in
parameters andref readonly
variables and returns in C#). It would be useful to be able to express this in expressions.Note that #24621 would require such
ref
support as a prerequisite to making it available to Microsoft.CSharp.This proposal suggests adding a
IsByRef
property toExpression
similar to that ofParameterExpression
(which would hide it in that case, but should return the save value) to representref
types and anIsByRefReadOnly
property to representin
/ref readonly
types.The default implementation is given as part of the proposal as that would affect custom classes derived from
Expression
.IsByRefReadOnly
only varies in the case whereIsByRef
is true. IfIsByRef
is false thenIsByRefReadOnly
should always be false.GetDelegateType
would need at least an internal overload that indicated the readonly quality ofin
parameters andref readonly
returns, so it should probably be made public.The
Parameter
factory would similarly need to be able to indicatein
parameters.Other factories will depend on inferring such types, unless experience shows that explicit factories are beneficial.
The text was updated successfully, but these errors were encountered: