-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add lambda/method group defaults proposal #6274
Conversation
Let's add some pointers to relevant background:
|
This is a breaking change, right? Currently, the following code compiles: void M(int i = 1) {}
var m = M;
Action<int> a = m; But with this proposal, this will be an error, since |
@svick yes this proposal would amount to a level of breaking change around lambdas and |
I have now revised the proposal based on LDM, PR, and other feedback so I think it is ready for a second look |
Excellent write up @adamperlin! |
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.
@cston, please take a look as well.
|
||
Del del1 = (int x = 1) => x; // Allowed, because default parameter value in lambda matches default parameter value in delegate | ||
|
||
Del del2 = D; // This behavior does not change and compiles as before as per the method group conversion rules |
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.
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.
It looks like it's covered below:
d = M; // Allowed with warning.
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.
Yep the case is covered there
delegate void M2(long i = 2); | ||
M2 m = (x = 3.0) => ...; //Error: cannot convert implicitly from long to double | ||
``` | ||
This inference leads to some tricky conversion issues which would require more discussion. |
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.
It's not just inference that is a problem, it would increase the look ahead for our parser. Consider the state of the parser when it's looking at the expression that starts with a (
. It has to decide if that is a lambda expression or not. Today the parser can bail out the moment it gets to (x =
as that disqualifies it from being a lambda. If we allowed defaults on implicit then it has to parse all the way through the =>
.
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.
Ah thanks for pointing this out. I will add a note about that as well
Championed Issue: #6051. This is a draft for a proposal to add default parameters to lambda functions and method groups, based heavily on the description in the championed issue. This topic has been discussed several times, but the proposal has yet to be merged.