NotificationsYou must be signed in to change notification settings
Fork4.2k
Star20k

Support NotNullIfNotNull in user-defined binary operators#48490

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

jcouv merged 2 commits intodotnet:masterfromduracellko:notnullifnotnull-binaryoperator

Oct 19, 2020

Merged

Support NotNullIfNotNull in user-defined binary operators#48490

jcouv merged 2 commits intodotnet:masterfromduracellko:notnullifnotnull-binaryoperator

Oct 19, 2020

Conversation

Copy link

Contributor

duracellko commentedOct 10, 2020

Closes#48489

This implementsNotNullIfNotNull attribute on return type of user defined binary operators.

Support NotNullIfNotNull in binary operators

142062f

duracellko requested a review froma team as acode owner

October 10, 2020 13:54

RikkiGibson self-assigned this

Oct 10, 2020

RikkiGibson added Area-Compilers Community

The pull request was submitted by a contributor who is not a Microsoft employee.

Feature - Nullable Reference TypesNullable Reference Types labels

Oct 10, 2020

RikkiGibson reviewed

Oct 12, 2020

View reviewed changes

src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs

		(c1 / c2).ToString(); // no warn
		(c1 / cn2).ToString(); // warn
		(cn1 / c2).ToString(); // no warn
		(cn1 / cn2).ToString(); // warn

Copy link

Member

RikkiGibsonOct 12, 2020•
edited by jcouv
Loading

Choose a reason for hiding this comment

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

Could you please add a test that verifies expected behavior around+=/-=/*=//= with these operators? Both the nullable state of the expression and the nullable state of the LHS of the assignment.
#Closed

Copy link

ContributorAuthor

duracellkoOct 13, 2020•
edited by jcouv
Loading

Choose a reason for hiding this comment

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

Test added. Let me know, if you still find some missing coverage. #Closed

RikkiGibson approved these changes

Oct 12, 2020

View reviewed changes

Copy link

Member

RikkiGibson left a comment•
edited
Loading

Choose a reason for hiding this comment

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

PR looks good to me, thanks, would just like to have a small bit of additional testing. We should think about what other nullability attributes could be applicable to user-defined binary operators to handle in a follow-up.