Implement exhaustiveness check for Union types #81442

AlekseyTs · 2025-11-25T16:08:17Z

First commit is a change that hopefully could be shared with "Closed Hierarchies" feature.

AlekseyTs · 2025-11-26T16:04:28Z

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

AlekseyTs · 2025-12-01T18:08:57Z

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

AlekseyTs · 2025-12-03T17:43:35Z

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

RikkiGibson · 2025-12-04T05:05:58Z

I have begun reviewing this, will need till tomorrow to finish. Thanks #Resolved

AlekseyTs · 2025-12-04T16:13:25Z

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

RikkiGibson

LGTM, only had some nits and a question.

When I proceed with closed class exhaustiveness checking I may try cherry picking the first commit as a starting point, I think it should be possible to add another implementation of ITypeUnionValueSetFactory for closed classes.

RikkiGibson · 2025-12-04T23:38:25Z

src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs

+                                {
+                                    // We don't know for sure that test for T2 is going to match, but if it fails,
+                                    // that means that some previous test must match and we simply cannot get to this test.
+                                    // In other words, this test is either unreachanble or it will succeed.


Suggested change

// In other words, this test is either unreachanble or it will succeed.

// In other words, this test is either unreachable or it will succeed.

``` #Resolved

RikkiGibson · 2025-12-04T23:38:44Z

src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs

                            falseTestImpliesTrueOther = true;
                            break;
-                        case BoundDagTypeTest _:
+                        case BoundDagTypeTest t:


nit: this t looks unused #Resolved

RikkiGibson · 2025-12-04T23:39:00Z

src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs

+                        {
+                            if (easyOutForLowering != (object)planA && !forLowering && ValueSetFactory.TypeUnionValueSetFactoryForInput(planA.Input) is not null)
+                            {
+                                // We need a test about `null` peresent in the Dag to properly handle exhaustiveness


Suggested change

// We need a test about `null` peresent in the Dag to properly handle exhaustiveness

// We need a test about `null` present in the Dag to properly handle exhaustiveness

``` #Resolved

RikkiGibson · 2025-12-05T00:04:28Z

src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs


+            string tryHandleTypeUnionLimits()
+            {
+                if (evaluations.IsEmpty && ValueSetFactory.TypeUnionValueSetFactoryForInput(input) is { } factory &&


To check my understanding, and, in case it helps the second reviewer:

It looks like evaluations.IsEmpty ensures that we don't propose one of our unmatched types as an example pattern, in cases where we would move on to find a more suitable example further along in the process, For example, in test Exhaustiveness_04, this allows us to propose pattern false as an example rather than pattern bool. #Resolved

RikkiGibson · 2025-12-05T00:39:33Z

src/Compilers/CSharp/Portable/Utilities/TypeUnionValueSet.cs

+
+        private static bool AnyTypeFromUnionMightMatch(ImmutableArray<TypeSymbol> typesInUnion, TypeSymbol type, ConversionsBase conversions, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
+        {
+            Debug.Assert(!typesInUnion.IsEmpty);


I didn't follow how it is that we are certain of this. The other two assertions I understand, but, I didn't follow why it is not possible for a pattern matching against an empty union to make it here. #Resolved

I didn't follow how it is that we are certain of this. The other two assertions I understand, but, I didn't follow why it is not possible for a pattern matching against an empty union to make it here.

It is not possible because any type test for an empty Union will result in an ERR_PatternWrongType and, since we are forcing a null check first, we end up with an empty value set after it. However, it is better to not create a value set factory for an empty Union type. I am going to make this change.

src/Compilers/CSharp/Test/Emit3/UnionsTests.cs

AlekseyTs · 2025-12-05T01:42:30Z

@333fred, @dotnet/roslyn-compiler For a second review

AlekseyTs · 2025-12-05T16:23:44Z

@333fred, @dotnet/roslyn-compiler For a second review

333fred

Mostly LGTM. Couple of test suggestions and one small question.

src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs

333fred · 2025-12-05T19:25:34Z

src/Compilers/CSharp/Test/Emit3/UnionsTests.cs

+                Diagnostic(ErrorCode.ERR_PatternWrongType, "C3").WithArguments("S1", "C3").WithLocation(28, 46),
+                // (33,57): hidden CS9335: The pattern is redundant.
+                //         return u switch { int => 1, I1 => 2, null => 3, C2 => 4 };
+                Diagnostic(ErrorCode.HDN_RedundantPattern, "C2").WithLocation(33, 57)


Does the prototype comment in _05 cover this diagnostic as well? #Resolved

Does the prototype comment in _05 cover this diagnostic as well?

No. This case is a known behavior of the redundancy checker, that is why it is a hidden diagnostic.
It can be observed in the scenario below, for example. Basically the checker often "complains" if a pattern
always matches, suggesting to use _ instead. The case in _05 is different, replacing the reported
pattern with _ wouldn't be semantically equivalent, hence the comment there.

[Fact] public void Test() { var src = @" struct S1 { public bool Value => false; } class Program { int M(S1 x) { return x switch {{Value: true} => 1, {Value: false} => 2}; } } "; var comp = CreateCompilation(src); CompileAndVerify(comp).VerifyDiagnostics( // (11,54): hidden CS9335: The pattern is redundant. // return x switch {{Value: true} => 1, {Value: false} => 2}; Diagnostic(ErrorCode.HDN_RedundantPattern, "false").WithLocation(11, 54) ); }

Interesting. Thanks for the explanation, I would not have assumed this was by-design.

Makes sense, I forgot about this behavior.

src/Compilers/CSharp/Test/Emit3/UnionsTests.cs

AlekseyTs · 2025-12-06T16:32:33Z

@RikkiGibson I think I responded to your feedback/questions. Please let me know if you have additional feedback. Otherwise, I will rebase the branch to prepare it for merging.

AlekseyTs force-pushed the Unions_03 branch from be1d0f2 to 0f18d52 Compare November 25, 2025 18:18

AlekseyTs added Area-Compilers Feature - Closed Class Hierarchies Feature - Unions labels Nov 25, 2025

AlekseyTs requested review from 333fred and RikkiGibson November 25, 2025 18:20

AlekseyTs marked this pull request as ready for review November 25, 2025 18:20

AlekseyTs requested a review from a team as a code owner November 25, 2025 18:20

RikkiGibson approved these changes Dec 5, 2025

View reviewed changes

AlekseyTs requested a review from a team December 5, 2025 16:22

333fred reviewed Dec 5, 2025

View reviewed changes

src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs Outdated Show resolved Hide resolved

333fred reviewed Dec 5, 2025

View reviewed changes

src/Compilers/CSharp/Test/Emit3/UnionsTests.cs Show resolved Hide resolved

333fred approved these changes Dec 6, 2025

View reviewed changes

AlekseyTs requested a review from RikkiGibson December 6, 2025 16:30

RikkiGibson approved these changes Dec 6, 2025

View reviewed changes

AlekseyTs added 2 commits December 6, 2025 13:51

Pattern matching: Implement value set for a union of types

3b33619

Unions: Implement exhaustiveness check for Union types

dddbef9

AlekseyTs force-pushed the Unions_03 branch from cf35d26 to dddbef9 Compare December 6, 2025 21:52

AlekseyTs enabled auto-merge December 6, 2025 22:15

AlekseyTs merged commit e9489a5 into dotnet:features/Unions Dec 6, 2025
24 of 25 checks passed

	// In other words, this test is either unreachanble or it will succeed.
	// In other words, this test is either unreachable or it will succeed.
	``` #Resolved

	// We need a test about `null` peresent in the Dag to properly handle exhaustiveness
	// We need a test about `null` present in the Dag to properly handle exhaustiveness
	``` #Resolved

Implement exhaustiveness check for Union types #81442

Implement exhaustiveness check for Union types #81442

Conversation

AlekseyTs commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RikkiGibson commented Dec 4, 2025 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RikkiGibson left a comment

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Dec 4, 2025 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Dec 4, 2025 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Dec 4, 2025 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Dec 5, 2025 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Dec 5, 2025 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AlekseyTs commented Dec 5, 2025

Uh oh!

AlekseyTs commented Dec 5, 2025

Uh oh!

333fred left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

333fred Dec 5, 2025 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

333fred Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AlekseyTs commented Dec 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

AlekseyTs commented Nov 25, 2025 •

edited

Loading

AlekseyTs commented Nov 26, 2025 •

edited

Loading

AlekseyTs commented Dec 1, 2025 •

edited

Loading

AlekseyTs commented Dec 3, 2025 •

edited

Loading

RikkiGibson commented Dec 4, 2025 •

edited by AlekseyTs

Loading

AlekseyTs commented Dec 4, 2025 •

edited

Loading

RikkiGibson Dec 4, 2025 •

edited by AlekseyTs

Loading

RikkiGibson Dec 4, 2025 •

edited by AlekseyTs

Loading

RikkiGibson Dec 4, 2025 •

edited by AlekseyTs

Loading

RikkiGibson Dec 5, 2025 •

edited by AlekseyTs

Loading

RikkiGibson Dec 5, 2025 •

edited by AlekseyTs

Loading

333fred Dec 5, 2025 •

edited by AlekseyTs

Loading

AlekseyTs Dec 6, 2025 •

edited

Loading