Skip to content

Cache more information for Closed Classes and Unions on NamedTypeSymbol - #84660

Merged
AlekseyTs merged 3 commits into
dotnet:mainfrom
AlekseyTs:CacheTryGetClosedSubtypes
Jul 29, 2026
Merged

Cache more information for Closed Classes and Unions on NamedTypeSymbol#84660
AlekseyTs merged 3 commits into
dotnet:mainfrom
AlekseyTs:CacheTryGetClosedSubtypes

Conversation

@AlekseyTs

@AlekseyTs AlekseyTs commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
Microsoft Reviewers: Open in CodeFlow

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 moves computation of type-union “case” expansions (including closed-class subtype expansion) into cached NamedTypeSymbol state, and updates the existing value-set factories/consumers to use those cached results. This should reduce repeated allocations/work when unions/closed classes participate in pattern/exhaustiveness analysis.

Changes:

  • Cache computed TypeUnionValueSet.CaseInfo expansions for (1) closed classes and (2) union types on NamedTypeSymbol.
  • Update ValueSetFactory union/closed-class factories and TypeUnionValueSet evaluation to consume the cached case lists.
  • Add a compiler test covering a closed union scenario and switch-exhaustiveness warnings.

Reviewed changes

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

Show a summary per file
File Description
src/Compilers/CSharp/Test/CSharp15/ClosedClassesTests.cs Adds a new closed-union switch exhaustiveness test.
src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.UnionTypeTypeUnionValueSetFactory.cs Switches union case expansion to use cached NamedTypeSymbol results.
src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.ClosedClassTypeUnionValueSetFactory.cs Uses cached closed-class case expansion; widens factory visibility to support reuse.
src/Compilers/CSharp/Portable/Utilities/TypeUnionValueSet.cs Uses cached closed-class expansion cases when reasoning about type-parameter substitutions.
src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs Introduces caching structures and APIs for closed-class subtypes and union value-set cases.
Comments suppressed due to low confidence (4)

src/Compilers/CSharp/Test/CSharp15/ClosedClassesTests.cs:7775

  • Trailing whitespace was added in the source string for Test2 (after switch, after string => 3,, and on the closing brace line). Please trim it to avoid whitespace-lint failures.
        return u switch 
        {
            int => 2,
            string => 3, 
        };

src/Compilers/CSharp/Test/CSharp15/ClosedClassesTests.cs:7785

  • Trailing whitespace was added in the source string for Test3 (after switch and on the closing brace line). Please trim it to avoid whitespace-lint failures.
        return u switch 
        {
            C2 => 1,
        };
    }   

src/Compilers/CSharp/Test/CSharp15/ClosedClassesTests.cs:7794

  • Trailing whitespace was added in the source string for Test4 (after switch and on the closing brace line). Please trim it to avoid whitespace-lint failures.
        return u switch 
        {
            C2 => 1,
            C3 => 2,
        };

src/Compilers/CSharp/Test/CSharp15/ClosedClassesTests.cs:7804

  • These diagnostic comment lines end with trailing whitespace (after switch). Please trim the trailing space to avoid whitespace-lint failures in the test file.
            //         return u switch 
            Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithArguments("string").WithLocation(100, 18),
            // (300,18): warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern 'C3' is not covered.
            //         return u switch 

Comment thread src/Compilers/CSharp/Test/CSharp15/ClosedClassesTests.cs
Comment thread src/Compilers/CSharp/Test/CSharp15/ClosedClassesTests.cs
Copilot AI review requested due to automatic review settings July 28, 2026 17:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs
Comment thread src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs
Copilot AI review requested due to automatic review settings July 29, 2026 01:47
@AlekseyTs
AlekseyTs requested a review from 333fred July 29, 2026 01:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs:775

  • _lazySubtypesIsComplete is written unconditionally before InterlockedInitialize of _lazySubtypes. If two threads race here, the losing thread can still overwrite _lazySubtypesIsComplete even though _lazySubtypes was initialized by the other thread, causing TryGetClosedSubtypes to return an incorrect completeness value for the cached subtypes.

Consider caching the subtypes and completeness together in a single atomically-initialized reference (e.g., a small sealed holder type/StrongBox), or use an interlocked state machine so only the winning initializer publishes both values (and readers never observe mismatched subtypes/completeness).

            (lazyClosedClassData._lazySubtypesIsComplete, lazySubtypes) = calculateClosedSubtypes(cancellationToken);
            ImmutableInterlocked.InterlockedInitialize(ref lazyClosedClassData._lazySubtypes, lazySubtypes);

            subtypes = lazyClosedClassData._lazySubtypes;
            return lazyClosedClassData._lazySubtypesIsComplete;

@AlekseyTs

Copy link
Copy Markdown
Contributor Author

@jjonescz, @333fred For a second review

@AlekseyTs
AlekseyTs merged commit f24acf1 into dotnet:main Jul 29, 2026
24 checks passed
@jjonescz jjonescz added this to the 18.11 milestone Aug 25, 2026
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