Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Handle nullable annotations on user defined operators and conversions #59169

Merged
merged 7 commits into from
Feb 2, 2022
Merged

Handle nullable annotations on user defined operators and conversions #59169

merged 7 commits into from
Feb 2, 2022

Conversation

RikkiGibson
Copy link
Contributor

@RikkiGibson RikkiGibson commented Jan 30, 2022 •
edited
Loading

Closes #34556 (replacing an old PR)
Closes #32671
Closes #49575

chsienki and others added 4 commits March 28, 2019 11:46
@chsienki
Honor annotations for user defined operators and conversions:
2e80b20 
- Add a VisitOperandsHonoringAnnotations that will split the state if needed based on annotations
- Call it from user defined conversions and binary operators
- Add tests
@chsienki
Move VisitOperands call into ApplyUserDefinedConversion
badc5ba
@chsienki
Clarify tests
9b1e02c
@RikkiGibson
Merge remote-tracking branch 'chsienki/user_defined_operators_annotat…
0211097 
…ions' into nullable-conv-annotations
@RikkiGibson RikkiGibson marked this pull request as ready for review January 31, 2022 04:04
@RikkiGibson RikkiGibson requested a review from a team as a code owner January 31, 2022 04:04
@cston
Copy link
Member

cston commented Feb 2, 2022 

                rightState = VisitOptionalImplicitConversion(right, targetTypeOpt: discarded ? default : leftLValueType, UseLegacyWarnings(left, leftLValueType), trackMembers: true, AssignmentKind.Assignment);

Do we need Unsplit() after this call?

Refers to: src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs:8771 in ecfe673. [](commit_id = ecfe673, deletion_comment = False)

@cston
Copy link
Member

cston commented Feb 2, 2022 

                        valueType = VisitOptionalImplicitConversion(rightPart, lvalueType, useLegacyWarnings: true, trackMembers: true, AssignmentKind.Assignment);

Unsplit()?

Refers to: src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs:9092 in ecfe673. [](commit_id = ecfe673, deletion_comment = False)

@RikkiGibson
Copy link
Contributor Author

I couldn't think of ways that the conditional state from the VisitOptionalImplicitConversion would be meaningfully used at either of those locations, so I added the Unsplit() call to be "conservative" about it.

@runfoapp runfoapp bot mentioned this pull request Feb 2, 2022
Closed
@RikkiGibson RikkiGibson merged commit 29e1ec8 into dotnet:main Feb 2, 2022
@ghost ghost added this to the Next milestone Feb 2, 2022
@RikkiGibson RikkiGibson deleted the nullable-conv-annotations branch February 2, 2022 22:32
@RikkiGibson RikkiGibson mentioned this pull request Feb 4, 2022
Closed
@RikkiGibson RikkiGibson modified the milestones: Next, 17.2.P1 Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cston cston cston approved these changes

@chsienki chsienki chsienki approved these changes

Assignees
No one assigned
Labels
Area-Compilers
Projects
None yet
Milestone
17.2.P1
3 participants
@RikkiGibson @cston @chsienki