Skip to content

Fix a crash when a nullable reference type is used in a pattern.#45206

Merged
gafter merged 2 commits intodotnet:masterfrom
gafter:master-45103
Jun 17, 2020
Merged

Fix a crash when a nullable reference type is used in a pattern.#45206
gafter merged 2 commits intodotnet:masterfrom
gafter:master-45103

Conversation

@gafter
Copy link
Member

@gafter gafter commented Jun 15, 2020

Fixes #45103

@gafter gafter requested a review from a team as a code owner June 15, 2020 21:52
@gafter gafter self-assigned this Jun 15, 2020
@gafter gafter added this to the 16.7 milestone Jun 15, 2020
}
else if (typeSyntax is NullableTypeSyntax)
{
Error(diagnostics, ErrorCode.ERR_PatternNullableType, typeSyntax, patternType.ToDisplayString(CodeAnalysis.NullableFlowState.MaybeNull), patternType);
Copy link
Contributor

@cston cston Jun 15, 2020

Choose a reason for hiding this comment

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

Ideally we should delay converting the patternType argument to a string until the diagnostic message is needed. Could we use typeSyntax as the diagnostic argument instead of patternType.ToDisplayString(...)? #Resolved

Copy link
Member Author

@gafter gafter Jun 15, 2020

Choose a reason for hiding this comment

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

No, because we are using ToDisplayString to tack on a ?. I suppose I could put the ? into the diagnostic template? The type syntax might contain multiple lines including comments.


In reply to: 440484936 [](ancestors = 440484936)

Copy link
Contributor

@cston cston Jun 16, 2020

Choose a reason for hiding this comment

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

typeSyntax is a NullableTypeSyntax. Doesn't it include ?? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

typeSyntax is for the squiggles.


In reply to: 440552553 [](ancestors = 440552553)

@gafter
Copy link
Member Author

gafter commented Jun 16, 2020

@cston I've responded to your comments. I don't know if you still think I should do something about realizing the type as a string in the diagnostic early. I suppose I could put the ? into the diagnostic template and make sure to use the nullable underlying type the other place that error code is used a few lines earlier. Please advise what you would suggest. #Resolved

@gafter
Copy link
Member Author

gafter commented Jun 16, 2020

@cston I added a commit that keeps the diagnostic arguments lazy. Would you please have a look? #Resolved

@gafter gafter merged commit 2ad1435 into dotnet:master Jun 17, 2020
@ghost ghost modified the milestones: 16.7, Next Jun 17, 2020
@dibarbet dibarbet modified the milestones: Next, 16.7.P4 Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Binder crash with nullable reference types

4 participants