Skip to content

Support NotNullIfNotNull in user-defined binary operators - #48490

Merged
jcouv merged 2 commits into
dotnet:masterfrom
duracellko:notnullifnotnull-binaryoperator
Oct 19, 2020
Merged

jcouv merged 2 commits into
dotnet:masterfrom
duracellko:notnullifnotnull-binaryoperator

Conversation

@duracellko

Copy link
Copy Markdown
Contributor

Closes #48489

This implements NotNullIfNotNull attribute on return type of user defined binary operators.

@duracellko
duracellko requested a review from a team as a code owner October 10, 2020 13:54
@RikkiGibson RikkiGibson self-assigned this Oct 10, 2020
@RikkiGibson RikkiGibson added Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee. Feature - Nullable Reference Types Nullable Reference Types labels Oct 10, 2020
(c1 / c2).ToString(); // no warn
(c1 / cn2).ToString(); // warn
(cn1 / c2).ToString(); // no warn
(cn1 / cn2).ToString(); // warn

@RikkiGibson RikkiGibson Oct 12, 2020 •

Copy link
Copy Markdown
Member

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

@duracellko duracellko Oct 13, 2020 •

Copy link
Copy Markdown
Contributor Author

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 RikkiGibson left a comment •

Copy link
Copy Markdown
Member

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.

@RikkiGibson

Copy link
Copy Markdown
Member

@dotnet/roslyn-compiler for senior review

@RikkiGibson

Copy link
Copy Markdown
Member

@dotnet/roslyn-compiler Please review

@jcouv jcouv self-assigned this Oct 19, 2020

@jcouv jcouv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 2)

@jcouv
jcouv merged commit 1a258ac into dotnet:master Oct 19, 2020
@ghost ghost added this to the Next milestone Oct 19, 2020
@jcouv

jcouv commented Oct 19, 2020

Copy link
Copy Markdown
Contributor

Thanks @duracellko!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee. Feature - Nullable Reference Types Nullable Reference Types

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NotNullIfNotNull attribute does not work with binary operators

4 participants