-
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
Honor abstract required during state initialization in nullable #76363
Conversation
@RikkiGibson @dotnet/roslyn-compiler for reviews. |
It looks like some diagnostics on non-required override properties and events are now missing. |
Fixes dotnet#74423. When we're getting the list of members to default initialize, we look for properties that override `abstract` properties and have nullable annotations that line up with the base ones. For these properties, we can assume that if we chain to a `SetsRequiredMembers` constructor, it took care of setting a non-null state.
7b24880
to
848ec10
Compare
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.
Done review pass, I will need a little more time to soak it all in and to see a few more tests.
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.
I meant to hit approve before, must have misclicked.
@dotnet/roslyn-compiler for a second review |
@333fred Can you think of any VB scenario affected as well? |
VB cannot subclass types with required members, and also does not have NRT, so I do not think it can encounter this issue. |
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 4)
Fixes #74423. When we're enforcing that all members of a type must have non-null values for
SetsRequiredMembers
, we don't want to include any members of the current type that are overrides of a base type. Instead, those members should be enforced when we check the chained constructor call, since the chained constructor may itself haveSetsRequiredMembers
, and therefore already enforced that overridden properties have a valid state.