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

Discussion: ref this vs this ref in ref extension methods (15.6, as a 7.2 bug fix) #1022

Open
alrz opened this issue Oct 21, 2017 · 10 comments · Fixed by dotnet/roslyn#23643
Assignees
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion
Milestone

Comments

@alrz
Copy link
Member

alrz commented Oct 21, 2017

Currently this ref int is not permitted and you need to use ref this int while throughout the language ref T denotes a "ref type" and you may add this modifier on a parameter to define an extension method, so it would be sensible to use this ref int to define a ref extensions method. That being said, the restriction seems arbitrary, both could be allowed but IMO this ref int looks more natural based on current rules.

As a minor point, the fact that all extension method's parameter lists start with this makes it easier to scan for extension methods in a static class.

LDM:

@jcouv
Copy link
Member

jcouv commented Oct 22, 2017

Tagging @VSadov in case something to add.

@alrz
Copy link
Member Author

alrz commented Oct 26, 2017

@jcouv @VSadov Any news here? My impression is that this can't be done within 15.5 timeframe but maybe as part of #946? And tbh I can't see the rationale behind the current restriction. partial has had this restriction for a long time and it's ok to make it a "feature" but this is new, maybe it would be sensible to not restrict it in the first place..

@benaadams
Copy link
Member

You can change the this pointer? So it is kinda ref this int?

@alrz
Copy link
Member Author

alrz commented Oct 27, 2017

@benaadams this is just a modifier, the actual "pointer" is the parameter itself, and that's of type ref T, making ref this int nonsensical. I'd go so far to say that we should only allow this ref T.

@benaadams
Copy link
Member

benaadams commented Oct 27, 2017

I'd go so far to say that we should only allow this ref T.

Makes what would define an extension very explicit e.g. (this which is probably a good thing.

What would be readonly ref extension? e.g.

(this in T or (this readonly ref T

@ufcpp
Copy link

ufcpp commented Oct 27, 2017

I also prefer (this ref T and (this in T. I'd rather like to know why the compiler team chose (ref this T from which I can't find any rationality.

@VSadov
Copy link
Member

VSadov commented Nov 3, 2017

Both ref and this are modifiers to the parameter. The formal type of the parameter is T, regardless of the kind of argument passing.

We could allow parameter modifiers in any order and just picked one order to not allow more than necessary.

I.E. it can be relaxed to make combinations like (this in T self, ...) valid

@alrz
Copy link
Member Author

alrz commented Nov 4, 2017

I supposed ref and in are modifiers on the "type" because they change the type of the parameter to a T&. Anyways, relaxing the ordering is probably the best option here. Thank you.

PS: for comparison, consider ref int x = e; vs. const int x = e; the former denotes a "ref type" i,e. ref is part of the type, but the latter has const as a modifier on the whole declaration statement.

@jcouv
Copy link
Member

jcouv commented Dec 13, 2017

@jcouv jcouv changed the title Discussion: ref this vs this ref in ref extension methods Discussion: ref this vs this ref in ref extension methods (15.6, as a 7.2 bug fix) Jan 4, 2018
@jcouv
Copy link
Member

jcouv commented Jan 9, 2018

Keeping the issue open so that it appears in the milestone as expected.
The fix was merged to 15.6

@jcouv jcouv reopened this Jan 9, 2018
@gafter gafter assigned jcouv and unassigned VSadov Jan 9, 2018
@333fred 333fred added the Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification label Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants