-
Notifications
You must be signed in to change notification settings - Fork 4.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
Update BoundCall method based on receiver nullability #31000
Conversation
|
|
||
[WorkItem(29605, "https://github.com/dotnet/roslyn/issues/29605")] | ||
[Fact] | ||
public void InferMethodReceiverType_01() |
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.
Is this issue specific to methods? Would it make sense to also test properties or fields?
Oh, I see that you updated VisitMemberAccess
too, which covers fields, properties and events. It may still be good to add a test for those (if not covered already). #Closed
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 are a number of existing tests that rely on the update in VisitMemberAccess
, but I've added explicit tests here. #Closed
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 1)
Should this PR target Update: it looks like the snap may not have happened yet. We'll have to check with Andy. |
See #30946. |
@dotnet/roslyn-compiler for a second review. |
b3.E += (A<object> a) => { }; // 5 | ||
b3.E += (A<object>? a) => { }; // 6 | ||
b3.E += (A<object?> a) => { }; | ||
b3.E += (A<object?>? a) => { }; // 7 |
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.
This seems unfortunate, but given that this is already how variance works for lambdas/method groups, I don't see a way to change this without making overload resolution rules extremely convoluted and inconsistent.
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 3)
@jaredpar for approval |
approved |
…out-if-error-message * origin/master: (174 commits) Add Compilers filter for Roslyn (dotnet#30880) Remove NonNullTypes context and other unnecessary information stored in TypeSymbolWithAnnotations. (dotnet#30913) Update BoundCall method based on receiver nullability (dotnet#31000) Make nullabiulity inference associative and commutative (dotnet#30990) Async-streams: minimal test for IOperation and CFG. Improve diagnostics (dotnet#30363) Add src. Add test. Remove dead code Update the build status table Script for generating our build status tables Add parsing tests to compiler benchmarks (dotnet#31033) Fix Edit and Continue in CPS projects Add comment Sorting. Save work Address PR feedback only produce optprof data on an official build Add bunch of exclusions to dead code analysis for special methods Disable WinRT tests on Linux (dotnet#31026) Change prerelease version to beta2 (dotnet#31017) ... # Conflicts: # src/Compilers/CSharp/Portable/CSharpResources.resx # src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
No description provided.