Unions: enforce the same order of processing for base interfaces of a member provider interface source vs. metadata.

[AlekseyTs]

Microsoft Reviewers: Open in CodeFlow

[azure-pipelines]

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]

Pull request overview

This PR updates the union-member-provider interface processing so that base interfaces are traversed in a consistent, deterministic order across source vs. metadata, and it re-enables a previously skipped regression test covering the scenario.

Changes:

• Re-enabled determinism regression coverage by un-skipping NonBoxingUnionMatching_MemberProvider_TryGetValue_Inheritance_37_ImplicitReferenceConversion_Determinism.
• Added caching + a deterministic “all interfaces” computation for the union member-provider interface, and switched union-member discovery/use-site accumulation to use that cached ordering.
• Made TypeSymbol.MakeAllInterfaces() non-virtual (still protected) as part of standardizing interface processing behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/Compilers/CSharp/Test/CSharp15/UnionsTests.cs	Un-skips a theory to validate source/metadata determinism for union matching.
src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs	Adjusts MakeAllInterfaces to be non-virtual.
src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs	Introduces cached base-interface ordering for union member-provider interfaces and uses it for union member/method discovery and use-site info.

[AlekseyTs]

@RikkiGibson, @jjonescz Please review

[jjonescz]

Since we should keep TypeSymbol.cs and TypeSymbol.vb in sync per a comment near the top of the file, should we make a similar change in VB?

[AlekseyTs]

I assume you are referring to a comment about public API surface. First, this isn't a public method. Second, C# and VB symbols are no longer public.

[jjonescz]

Nevertheless, it feels like if we can make the same change on the VB side, perhaps we should?

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

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

• GetMemberProviderInterfaceAllInterfacesForDefinition returns a default ImmutableArray when the member-provider interface is missing. Default ImmutableArray values are easy to misuse (e.g., foreach/Length can throw), and this method is internal so future callers might not consistently guard against IsDefault. Consider caching/returning ImmutableArray.Empty instead.

            NamedTypeSymbol? memberProviderInterface = GetMemberProviderInterfaceForDefinition();
            if (memberProviderInterface is null)
            {
                return default;
            }

src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs:339

• TypeSymbol contains a repo convention comment that its public interface should stay synchronized with the VB TypeSymbol. The VB implementation still has an overridable MakeAllInterfaces, but this change removes virtual from the C# version, which risks diverging the two implementations over time.

        protected ImmutableArray<NamedTypeSymbol> MakeAllInterfaces()

[jjonescz]

Nevertheless, it feels like if we can make the same change on the VB side, perhaps we should?

[AlekseyTs]

Nevertheless, it feels like if we can make the same change on the VB side, perhaps we should?

There is no behavior change here, therefore, I don't think any synchronization is necessary. I removed the modifier to confirm that there is no alternative implementation.

[AlekseyTs]

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

[AlekseyTs]

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

[AlekseyTs]

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