Implement lambda params with modifiers without type name #69273
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
ghost
added
the
Community
|
@dotnet/roslyn-compiler @jaredpar i approved making this PR (as draft) as the corresponding proposal champion. The intent is not to take this in (at least until LDM approval), but instead to just see the scope of the change needed to support this lang change. Thanks! |
Rekkonnect
changed the title
| int firstDefault = -1; | ||
| for (int i = 0; i < lambda.ParameterCount; i++) | ||
| var paramSyntax = lambda.ParameterSyntax(i); | ||
| if (paramSyntax is not null) |
There was a problem hiding this comment.
Why can paramSyntax be null? If the lambda has non-zero ParameterCount I would expect it to have corrsponding parameter syntaxes.
There was a problem hiding this comment.
It can be null in some tests for parameters with modifiers without parameter parentheses (e.g. ref x => x), which is illegal per the spec. Without the underlying changes, now permitting the null parameter syntax, the assertion would fail.
Since the modifiers are not allowed in single-parameter unparenthesized lambda expressions, we need to at least expect the case to arise.
There was a problem hiding this comment.
i don't really get it either. i would not expect any sort of lambda to have a nulll parameter syntax.
There was a problem hiding this comment.
Since the modifiers are not allowed in single-parameter unparenthesized lambda expressions, we need to at least expect the case to arise.
this sentence doesn't make sense. Just because the modifiers are not legal doesn't mean you get a null parameter syntax.
There was a problem hiding this comment.
Made changes to preserve this, but kept the removal of the check about having an explicitly typed parameter list. To circumvent breaks, lambda binding data for simple lambda expressions (parameters without parentheses) now contain a singleton separated syntax list of parameter syntaxes.
src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs
Outdated
Show resolved
Hide resolved
| @@ -779,7 +779,8 @@ public static void M(ref readonly int x) { } | |||
| Diagnostic(ErrorCode.ERR_CantConvAnonMethParams, "=>").WithArguments("lambda expression", "System.Linq.Expressions.Expression<D>").WithLocation(7, 28), | |||
| // (8,49): error CS1615: Argument 1 may not be passed with the 'out' keyword | |||
| // Expression<Action<int>> e5 = (int p) => C.M(out p); | |||
| Diagnostic(ErrorCode.ERR_BadArgExtraRef, "p").WithArguments("1", "out").WithLocation(8, 49)); | |||
| Diagnostic(ErrorCode.ERR_BadArgExtraRef, "p").WithArguments("1", "out").WithLocation(8, 49) | |||
|
Yours seems cleaner and refactors some code that I don't remember touching in my PR, so I think it's better to roll with yours. Perhaps bring some of the tests from this one into the other PR if they prove useful and cover a wider range. I don't really have a strong opinion with this implementation as it wasn't as structured, but rather patchy. Feel free to use whatever parts are necessary, including the tests. |
|
Closing in favor of #75400 |
Based on the spec for dotnet/csharplang#338.