Skip to content

Propagate params to lambdas and local functions #79880

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Propagate params to lambdas and local functions #79880

wants to merge 14 commits into from
+741 −53

Conversation

jjonescz
Copy link
Member

Fixes #79752.

@jjonescz jjonescz marked this pull request as ready for review August 13, 2025 12:47
@jjonescz jjonescz requested a review from a team as a code owner August 13, 2025 12:47
@AlekseyTs
Copy link
Contributor

AlekseyTs commented Aug 14, 2025 •
edited
Loading

Does VB have the same issue with lambdas? At the very least we should add tests if the scenario is relevant for VB, I think. #Closed

@jjonescz
Copy link
Member Author

jjonescz commented Aug 14, 2025 •
edited
Loading

VB doesn't support params in lambdas. And VB doesn't have local functions. So I don't think there are any relevant tests I can add.

@333fred
Copy link
Member

333fred commented Aug 14, 2025

I'd like more understanding of the scenario before proceeding with implementation changes.

@KubaZ2 KubaZ2 mentioned this pull request Aug 15, 2025
Open
@AlekseyTs
Copy link
Contributor

AlekseyTs commented Aug 21, 2025 •
edited
Loading

Are we testing lambdas and local functions inside new extension methods? They undergo a special rewrite. #Closed

jcouv
jcouv reviewed Aug 28, 2025
View reviewed changes
@jcouv jcouv mentioned this pull request Aug 28, 2025
Merged
jcouv
jcouv reviewed Aug 28, 2025
View reviewed changes
@jcouv jcouv self-assigned this Aug 28, 2025
@jjonescz jjonescz requested a review from a team September 3, 2025 08:47
jcouv
jcouv approved these changes Sep 8, 2025
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thanks (commit 8)

@jjonescz jjonescz requested review from AlekseyTs and a team September 8, 2025 09:18
AlekseyTs
AlekseyTs reviewed Sep 10, 2025
View reviewed changes
AlekseyTs
AlekseyTs reviewed Sep 10, 2025
View reviewed changes
@AlekseyTs
Copy link
Contributor

AlekseyTs commented Sep 10, 2025 •
edited
Loading

Done with review pass (commit 9) #Closed

@jjonescz jjonescz added the breaking-change https://github.com/dotnet/sdk/blob/main/documentation/project-docs/breaking-change-guidelines.md label Sep 11, 2025
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET SDK Breaking Change Notification email list.

You can refer to the .NET SDK breaking change guidelines

@jjonescz
Copy link
Member Author

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET SDK Breaking Change Notification email list.

You can refer to the .NET SDK breaking change guidelines

I think these steps are not necessary - when I tried them previously, I was told the documentation of the breaking change here in the roslyn repo is enough.

AlekseyTs
AlekseyTs reviewed Sep 11, 2025
View reviewed changes
AlekseyTs
AlekseyTs reviewed Sep 11, 2025
View reviewed changes
@AlekseyTs
Copy link
Contributor

AlekseyTs commented Sep 11, 2025 •
edited
Loading

Done with review pass (commit 12) #Closed

AlekseyTs
AlekseyTs reviewed Sep 11, 2025
View reviewed changes
AlekseyTs
AlekseyTs reviewed Sep 11, 2025
View reviewed changes
@AlekseyTs
Copy link
Contributor

AlekseyTs commented Sep 11, 2025 •
edited
Loading

Done with review pass (commit 13) #Closed

@AlekseyTs
Copy link
Contributor

Done with review pass (commit 14)

@AlekseyTs
Copy link
Contributor

Overall LGTM, but I'll wait for the breaking change approval before signing off.

@jjonescz jjonescz requested a review from AlekseyTs September 17, 2025 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcouv jcouv jcouv approved these changes

@AlekseyTs AlekseyTs Awaiting requested review from AlekseyTs

Assignees

@jcouv jcouv

Labels
Area-Compilers breaking-change https://github.com/dotnet/sdk/blob/main/documentation/project-docs/breaking-change-guidelines.md
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ParamArrayAttribute and ParamCollectionAttribute missing for compiler generated symbols
4 participants
@jjonescz @AlekseyTs @333fred @jcouv