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

Allow by-ref Extension methods on value types #15535

Closed
wants to merge 15 commits into from

Conversation

pakrym
Copy link
Contributor

@pakrym pakrym commented Nov 25, 2016

Fixes: #165

Allows adding ref modifier to this parameter in extension methods for value types.

Done:

  1. Value type only compilation error.
  2. Overload resolution and ambiguity checks.
  3. Taking receiver address availability check.

Not done:

  1. ?

Do you think this is valid approach to implementing this feature? What's missing from this implementation?

@pakrym
Copy link
Contributor Author

pakrym commented Nov 28, 2016

@dotnet/roslyn-compiler

@benaadams
Copy link
Member

benaadams commented Nov 28, 2016

What about the combination case? (in tests)

public static ref S Get(this ref S s)
{
    return ref s;
}

@gafter gafter added this to the 2.1 milestone Nov 28, 2016
@jaredpar
Copy link
Member

Thanks for this contribution! Looks like a good start to the feature.

Have you taken a look at the language feature contribution guide?

https:/dotnet/roslyn/blob/master/docs/contributing/Developing%20a%20Language%20Feature.md

Language features are a pretty involved feature as we have to consider not just the compiler but all of the other pieces of the language experience: IDE, debugging, scripting, etc ... Even for simple features this other work is often significant. This guide outlines how we approach accepting features to ensure the feature is high quality and the other work is accounted for (both for our features and those driven at least in part by the community).

For this feature I don't think we need to have a formal feature specification and move onto prototyping the work. This is mostly a known feature with only a couple of undecided issues. For example:

  • Extension method calls on r-values: are they allowed and done silently on a temporary?
  • Extension method definitions on generic type parameters: do they need to be known to be a value type or can they work on an unconstrained T?
  • Integration with proposed "readonly ref" feature.

Hence I think the next step is moving this work into a feature branch and assigning someone from the compiler team to help with the feature. I'm traveling today but should be able to get that all setup early tomorrow morning.

@pakrym
Copy link
Contributor Author

pakrym commented Nov 28, 2016

Extension method calls on r-values: are they allowed and done silently on a temporary?

This should definitely be allowed on ref returns. For non-ref r-values I think this if fine to alow because even if by-ref method mutates the structure usually it won't return it so user would be forced to introduce temporary variable anyway.

Extension method definitions on generic type parameters: do they need to be known to be a value type or can they work on an unconstrained T?

I'm not sure about usefulness of unconstrained generic case. Do you have anything on your mind?

Integration with proposed "readonly ref" feature.

If readonly would be allowed on any argument this should "just work"™

@jaredpar
Copy link
Member

This should definitely be allowed on ref returns

Ref returns are l-values 😄

For non-ref r-values I think this if fine to alow because even if by-ref method mutates the structure usually it won't return it so user would be forced to introduce temporary variable anyway.

There is some debate to this. The main issue is that by-ref extension methods allows for observable side effects in the struct. Using a temporary means we're silently discarding the side effect. Ultimately I think that's fine though because it already happens silently today when you invoke a method on a struct in a readonly location.

If readonly would be allowed on any argument this should "just work"™

Sure but it has some downstream implications that we've been discussing. In particular about the above r-value / l-value issue. If both features are present it's a reasonable stance to say:

  • ref Point this extension methods can only be invoked on l-values.
  • readonly ref Point this extension methods can be invoked on any value of the appropriate type.

The latter is safe because the readonly eliminates the potential for a side effect on the struct instance itself.

@benaadams
Copy link
Member

benaadams commented Nov 29, 2016

The main issue is that by-ref extension methods allows for observable side effects in the struct.

Yes please 👍 Much like extensions allow observable side effects on a class; else you need to modify the struct to add the methods directly to it.

An optional readonly modifier would be a good enhancement.

Question: Does this change allow C# to see VB.NET's ByRef extension methods?

@pakrym
Copy link
Contributor Author

pakrym commented Nov 29, 2016

@benaadams just pushed a commit allowing discovery of VB byref methods. Did not test if it works in Intellisense yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants