-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Fix Local-Symbol Containing-Symbol in New-Extension-Method-Rewriter for Capture Analysis #78160
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
Fix Local-Symbol Containing-Symbol in New-Extension-Method-Rewriter for Capture Analysis #78160
Conversation
@bernd5 Please provide a more descriptive title for the PR, the title that reflects the nature of the changes #Closed |
src/Compilers/CSharp/Portable/Lowering/BoundTreeToDifferentEnclosingContextRewriter.cs
Outdated
Show resolved
Hide resolved
"Containing-Type" or "Containing-Symbol"? #Closed |
src/Compilers/CSharp/Portable/Lowering/BoundTreeToDifferentEnclosingContextRewriter.cs
Outdated
Show resolved
Hide resolved
Done with review pass (commit 5) #Closed |
I assume the faild tests have nothing todo with my change - some helix stuff 🤷 |
src/Compilers/CSharp/Portable/Lowering/BoundTreeToDifferentEnclosingContextRewriter.cs
Outdated
Show resolved
Hide resolved
Done with review pass (commit 11) |
@jcouv For the second review |
src/Compilers/CSharp/Portable/Lowering/BoundTreeToDifferentEnclosingContextRewriter.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Portable/Lowering/BoundTreeToDifferentEnclosingContextRewriter.cs
Outdated
Show resolved
Hide resolved
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.
LGTM Thanks (iteration 14)
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.
LGTM (commit 15)
@bernd5 Thanks for the contribution! |
@AlekseyTs @jcouv |
Main branch is currently locked due to some infra issues. |
fixes #78135
fixes #78042
Relates to test plan #76130