Skip to content

Report ERR_PatternWrongType for Union matching scenarios#81352

Merged
AlekseyTs merged 2 commits into
dotnet:features/Unionsfrom
AlekseyTs:Unions_02
Nov 25, 2025
Merged

Report ERR_PatternWrongType for Union matching scenarios#81352
AlekseyTs merged 2 commits into
dotnet:features/Unionsfrom
AlekseyTs:Unions_02

Conversation

@AlekseyTs

@AlekseyTs AlekseyTs commented Nov 19, 2025

Copy link
Copy Markdown
Contributor

Relates to test plan: #81074

Comment thread src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs Outdated
// (102,25): error CS8121: An expression of type 'S1' cannot be handled by a pattern of type 'C4'.
// _ = u is C1 and C4;
Diagnostic(ErrorCode.ERR_PatternWrongType, "C4").WithArguments("S1", "C4").WithLocation(102, 25),
// (103,24): error CS8121: An expression of type 'S1' cannot be handled by a pattern of type 'C4'.

@333fred 333fred Nov 20, 2025

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.

Think we should also take a look at improving the diagnostics for these cases too. It's not obvious to me from reading the message that the problem is "'C4' is not part of the union 'S1'". #Resolved

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.

Think we shoudl also take a look at improving the diagnostics for these cases too. It's not obvious to me from reading the message that the problem is "'C4' is not part of the union 'S1'".

I think already added PROTOTYPE comment will cover this as well.

Comment thread src/Compilers/CSharp/Test/Emit3/UnionsTests.cs
@AlekseyTs

Copy link
Copy Markdown
Contributor Author

@RikkiGibson, @333fred, @dotnet/roslyn-compiler Please review

@333fred 333fred 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.

One small test suggestion, otherwise LGTM

Comment thread src/Compilers/CSharp/Test/Emit3/UnionsTests.cs
@AlekseyTs

Copy link
Copy Markdown
Contributor Author

@RikkiGibson, @dotnet/roslyn-compiler For a second review

@AlekseyTs AlekseyTs requested a review from a team November 24, 2025 17:37
@AlekseyTs

Copy link
Copy Markdown
Contributor Author

@RikkiGibson, @dotnet/roslyn-compiler For a second review

if (match == ConstantValue.False || match == ConstantValue.Bad)
{
diagnostics.Add(ErrorCode.ERR_PatternWrongType, expression.Syntax.Location, unionType, expression.Display);
hasErrors = true;

@RikkiGibson RikkiGibson Nov 24, 2025

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.

It doesn't look like hasErrors is used after being assigned here, was that intentional? #Resolved

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.

It doesn't look like hasErrors is used after being assigned here, was that intentional?

Yes, a copy/paste artifact. I'll remove this assignment (probably in the next PR)

@AlekseyTs AlekseyTs requested review from a team and RikkiGibson November 24, 2025 23:24
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.

3 participants