Skip to content

Initial support for non-boxing union access pattern HasValue property#82011

Merged
AlekseyTs merged 3 commits into
dotnet:features/Unionsfrom
AlekseyTs:Unions_11
Jan 27, 2026
Merged

Initial support for non-boxing union access pattern HasValue property#82011
AlekseyTs merged 3 commits into
dotnet:features/Unionsfrom
AlekseyTs:Unions_11

Conversation

@AlekseyTs

Copy link
Copy Markdown
Contributor

No description provided.

@AlekseyTs AlekseyTs requested review from a team as code owners January 14, 2026 17:59
@dotnet-policy-service dotnet-policy-service Bot added the Needs API Review Needs to be reviewed by the API review council label Jan 14, 2026
@AlekseyTs

AlekseyTs commented Jan 14, 2026

Copy link
Copy Markdown
Contributor Author

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

2 similar comments
@AlekseyTs

AlekseyTs commented Jan 15, 2026

Copy link
Copy Markdown
Contributor Author

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

@AlekseyTs

AlekseyTs commented Jan 16, 2026

Copy link
Copy Markdown
Contributor Author

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

IsStatic: false,
DeclaredAccessibility: Accessibility.Public,
GetMethod: not null,
SetMethod: null,

@333fred 333fred Jan 16, 2026

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.

This seems unusual. Why would we checking this? For example, the foreach pattern doesn't ensure that Enumerator.Value has a get but no set, it only checks for the get being present. #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.

This seems unusual. Why would we checking this?

This is how the spec defines the API. I can see that there could be other interpretations. I will capture a clarification question in the spec.

input.Type is NamedTypeSymbol { IsUnionTypeNoUseSiteDiagnostics: true } unionType &&
Binder.IsUnionTypeValueProperty(unionType, property))
{
// This sub-puttern is a union matching

@333fred 333fred Jan 16, 2026

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.

Suggested change
// This sub-puttern is a union matching
// This sub-pattern is a union matching

}
";
var comp = CreateCompilation([src, IUnionSource], options: TestOptions.ReleaseExe);
var verifier = CompileAndVerify(comp, expectedOutput: "FalseTrueFalse").VerifyDiagnostics();

@333fred 333fred Jan 16, 2026

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: verifier is unused. And a few others.
#Resolved

// s.Value.ToString();
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s.Value").WithLocation(400, 13)
);
}

@333fred 333fred Jan 16, 2026

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.

Consider a generic constraint case, something like:

interface IA
{
    bool HasValue { get; }
}

static void Test1<T>(T t) where T : IA, IUnion
{
    _ = s is null;
}
``` #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.

Consider a generic constraint case, something like ...

Perhaps I am missing something. Type parameters aren't union types regardless of constraints, and aren't subject for union matching.

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.

Perhaps I am missing something. Type parameters aren't union types regardless of constraints, and aren't subject for union matching.

Interesting. The wording in the spec is "Any non-abstract class or struct type that implements the IUnion interface is considered a union type.". I think that means that a type parameter constrained to IUnion, struct should be considered a union type, no?

@AlekseyTs AlekseyTs Jan 16, 2026

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.

In general, type parameters are not classes or structures. They can be reference types or value types. Also, some union behaviors require constructors, type parameters do not have those. Since the spec doesn't explicitly say what scenarios are supposed to work with type parameters, I interpret that as: "Union behaviors aren't applicable to type parameters." However, I requested a clarification - https://github.com/dotnet/csharplang/blob/main/proposals/unions.md#confirm-that-a-type-parameter-is-never-a-union-type-even-when-constrained-to-one

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.

There is also a test reflecting the fact - UnionMatching_35_TypeParameter

@AlekseyTs AlekseyTs requested review from a team and 333fred January 16, 2026 22:13
@AlekseyTs AlekseyTs requested a review from a team January 16, 2026 23:33
@AlekseyTs

AlekseyTs commented Jan 16, 2026

Copy link
Copy Markdown
Contributor Author

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

3 similar comments
@AlekseyTs

AlekseyTs commented Jan 20, 2026

Copy link
Copy Markdown
Contributor Author

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

@AlekseyTs

AlekseyTs commented Jan 21, 2026

Copy link
Copy Markdown
Contributor Author

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

@AlekseyTs

AlekseyTs commented Jan 22, 2026

Copy link
Copy Markdown
Contributor Author

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

Comment thread src/Compilers/CSharp/Test/CSharp15/UnionsTests.cs Outdated
Comment thread src/Compilers/CSharp/Test/CSharp15/UnionsTests.cs Outdated
@AlekseyTs AlekseyTs enabled auto-merge (squash) January 25, 2026 05:58
@AlekseyTs AlekseyTs disabled auto-merge January 25, 2026 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Feature - Unions Needs API Review Needs to be reviewed by the API review council

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants