Skip to content

Do not expand closed subtypes in old language versions#83704

Merged
RikkiGibson merged 6 commits into
dotnet:features/closed-classfrom
RikkiGibson:cc-8
May 16, 2026
Merged

Do not expand closed subtypes in old language versions#83704
RikkiGibson merged 6 commits into
dotnet:features/closed-classfrom
RikkiGibson:cc-8

Conversation

@RikkiGibson
Copy link
Copy Markdown
Member

@RikkiGibson RikkiGibson commented May 14, 2026

Test plan: #81039

Microsoft Reviewers: Open in CodeFlow

Copilot AI review requested due to automatic review settings May 14, 2026 18:38
@RikkiGibson RikkiGibson requested a review from a team as a code owner May 14, 2026 18:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the C# compiler’s pattern-matching exhaustiveness/type-union machinery so that closed class subtype expansion is only performed when the closed classes feature is enabled for the compilation’s language version. This prevents older language versions from treating metadata “closed” hierarchies as exhaustive sets of subtypes.

Changes:

  • Add a regression test verifying that C# 14 does not treat closed classes as special for exhaustiveness, including metadata reference scenarios.
  • Thread CSharpCompilation through ValueSetFactory type-union factories and call sites, and gate ExpandClosedSubtypes on compilation.IsFeatureEnabled(MessageID.IDS_FeatureClosedClasses).
  • Update pattern-matching DAG construction and counterexample generation to use the new TypeUnionValueSetFactoryForInput(CSharpCompilation, ...) signature.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Compilers/CSharp/Test/CSharp15/ClosedClassesTests.cs Adds C# 14 exhaustiveness coverage to ensure closed subtype expansion does not happen in older language versions.
src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.cs Updates the type-union factory entry point to accept a CSharpCompilation for feature gating.
src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.ClosedClassTypeUnionValueSetFactory.cs Gates closed subtype expansion on feature availability and carries CSharpCompilation through the factory.
src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.UnionTypeTypeUnionValueSetFactory.cs Threads CSharpCompilation so union case handling can reuse the gated closed-subtype expansion logic.
src/Compilers/CSharp/Portable/Binder/PatternExplainer.cs / src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs Updates call sites to pass compilation into type-union value-set queries and adjusts selected-test plumbing accordingly.

Comment thread src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.cs Outdated
Copilot AI review requested due to automatic review settings May 14, 2026 19:59
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

""";

var comp = CreateCompilation([source1, source2, ClosedAttributeDefinition], parseOptions: TestOptions.Regular14, targetFramework: TargetFramework.Net100);
comp.VerifyDiagnostics(
comp0.VerifyDiagnostics();

var comp1 = CreateCompilation([source2, ClosedAttributeDefinition], references: [comp0.ToMetadataReference()], parseOptions: TestOptions.Regular14, targetFramework: TargetFramework.Net100);
comp1.VerifyDiagnostics(
Comment thread src/Compilers/CSharp/Portable/Binder/DecisionDagBuilder.cs Outdated
@AlekseyTs
Copy link
Copy Markdown
Contributor

AlekseyTs commented May 14, 2026

Done with review pass (commit 3) #Closed

Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 4)

// (100,18): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern '_' is not covered.
// return c switch
Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("_").WithLocation(100, 18));
}
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.

nit: I think it would be better to test langversion next and preview too, for completness

Copilot AI review requested due to automatic review settings May 15, 2026 16:52
@RikkiGibson RikkiGibson review requested due to automatic review settings May 15, 2026 16:56
@RikkiGibson RikkiGibson merged commit 935c676 into dotnet:features/closed-class May 16, 2026
24 checks passed
@RikkiGibson RikkiGibson deleted the cc-8 branch May 16, 2026 01:42
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.

5 participants