Skip to content

Commit b450b26

Browse files
AlekseyTsAlekseyTs
authored
Unions: Address/remove some PROTOTYPE comments (#82381)
1 parent d6040ad commit b450b26

File tree

5 files changed

+161
-32
lines changed

5 files changed

+161
-32
lines changed

‎src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -471,8 +471,6 @@ private Conversion ClassifyConversionFromTypeForCast(TypeSymbol source, TypeSymb
471471
//
472472
// fail.
473473

474-
// PROTOTYPE: Note, an explicit user-defined conversion may come before a union conversion in this case.
475-
// Confirm that this is acceptable.
476474
var conversion = GetExplicitUserDefinedConversion(source, destination, isChecked: isChecked, ref useSiteInfo);
477475
if (conversion.Exists)
478476
{
@@ -791,7 +789,6 @@ private Conversion GetImplicitUserDefinedOrUnionConversion(BoundExpression sourc
791789
return result;
792790
}
793791

794-
// PROTOTYPE: Confirm that union conversions are considered after user-defined conversions.
795792
Conversion unionConversion = AnalyzeImplicitUnionConversions(sourceExpression, source, destination, ref useSiteInfo);
796793

797794
if (unionConversion.Exists)

‎src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/UserDefinedImplicitConversions.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -991,8 +991,6 @@ private Conversion AnalyzeImplicitUnionConversions(
991991
Debug.Assert(sourceExpression != null || (object)source != null);
992992
Debug.Assert((object)target != null);
993993

994-
// PROTOTYPE: It might make sense to block conversions from a base class or any interface. See comments in tests.
995-
996994
if (target.StrippedType() is not NamedTypeSymbol namedTarget || !namedTarget.IsUnionTypeWithUseSiteDiagnostics(ref useSiteInfo))
997995
{
998996
return Conversion.NoConversion;

‎src/Compilers/CSharp/Portable/Binder/SwitchBinder.cs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,10 +268,12 @@ protected BoundExpression ConvertCaseExpression(CSharpSyntaxNode node, BoundExpr
268268
hasErrors = true;
269269
}
270270

271-
caseExpression = CreateConversion(caseExpression, conversion, SwitchGoverningType, diagnostics);
271+
if (!conversion.IsUnion)
272+
{
273+
caseExpression = CreateConversion(caseExpression, conversion, SwitchGoverningType, diagnostics);
274+
}
272275
}
273276

274-
// PROTOTYPE: Test that consumer actually does the right thing when a union matching is involved. Cover error scenarios as well.
275277
var inputType = SwitchGoverningType;
276278
NamedTypeSymbol unionType = PrepareForUnionMatchingIfAppropriateAndReturnUnionType(node, ref inputType, diagnostics);
277279
return ConvertPatternExpression(unionType, inputType, node, caseExpression, out constantValueOpt, hasErrors, diagnostics, out _);
@@ -433,7 +435,7 @@ private BoundExpression BindSwitchGoverningExpression(BindingDiagnosticBag diagn
433435
if (conversion.IsValid)
434436
{
435437
// Condition (2) satisfied
436-
Debug.Assert(conversion.Kind == ConversionKind.ImplicitUserDefined); // PROTOTYPE: Follow up
438+
Debug.Assert(conversion.Kind == ConversionKind.ImplicitUserDefined);
437439
Debug.Assert(conversion.Method.IsUserDefinedConversion());
438440
Debug.Assert(conversion.UserDefinedToConversion.IsIdentity);
439441
Debug.Assert(resultantGoverningType.IsValidV6SwitchGoverningType(isTargetTypeOfUserDefinedOp: true));

‎src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1843,8 +1843,6 @@ internal ImmutableArray<TypeSymbol> UnionCaseTypes
18431843
internal static bool IsSuitableUnionConstructor(MethodSymbol ctor)
18441844
{
18451845
Debug.Assert(ctor.MethodKind is MethodKind.Constructor);
1846-
// PROTOTYPE: Confirm RefKind restriction. Conversion operators allow only RefKind.None or RefKind.In.
1847-
// It feels like it makes sense to use the same restriction here.
18481846
return ctor is { DeclaredAccessibility: Accessibility.Public, ParameterCount: 1, Parameters: [{ RefKind: RefKind.In or RefKind.None }] };
18491847
}
18501848

‎src/Compilers/CSharp/Test/CSharp15/UnionsTests.cs‎

Lines changed: 156 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2360,6 +2360,160 @@ .maxstack 2
23602360
");
23612361
}
23622362

2363+
[Fact]
2364+
public void UnionMatching_36_SwitchStatement()
2365+
{
2366+
var src = @"
2367+
struct S1 : System.Runtime.CompilerServices.IUnion
2368+
{
2369+
private readonly object _value;
2370+
public S1(int x) { _value = x; }
2371+
public S1(string x) { _value = x; }
2372+
object System.Runtime.CompilerServices.IUnion.Value => _value;
2373+
}
2374+
2375+
class Program
2376+
{
2377+
static void Main()
2378+
{
2379+
System.Console.Write(Test1(new S1(10)));
2380+
System.Console.Write(Test1(new S1(""10"")));
2381+
System.Console.Write(Test1(default));
2382+
System.Console.Write(Test1(new S1(11)));
2383+
System.Console.Write(Test1(new S1(""11"")));
2384+
System.Console.Write(Test1(new S1(0)));
2385+
}
2386+
2387+
static int Test1(S1 u)
2388+
{
2389+
switch (u)
2390+
{
2391+
case 10: return 1;
2392+
case ""11"": return 2;
2393+
}
2394+
2395+
return -1;
2396+
}
2397+
}
2398+
";
2399+
var comp = CreateCompilation([src, IUnionSource], options: TestOptions.ReleaseExe);
2400+
CompileAndVerify(comp, expectedOutput: "1-1-1-12-1").VerifyDiagnostics();
2401+
}
2402+
2403+
[Fact]
2404+
public void UnionMatching_37_SwitchStatement()
2405+
{
2406+
var src = @"
2407+
struct S1 : System.Runtime.CompilerServices.IUnion
2408+
{
2409+
private readonly object _value;
2410+
public S1(int x) { _value = x; }
2411+
public S1(string x) { _value = x; }
2412+
object System.Runtime.CompilerServices.IUnion.Value => _value;
2413+
}
2414+
2415+
class Program
2416+
{
2417+
static void Main()
2418+
{
2419+
System.Console.Write(Test1(new S1(10)));
2420+
System.Console.Write(Test1(new S1(""10"")));
2421+
System.Console.Write(Test1(default));
2422+
System.Console.Write(Test1(new S1(11)));
2423+
System.Console.Write(Test1(new S1(""11"")));
2424+
System.Console.Write(Test1(new S1(0)));
2425+
}
2426+
2427+
static int Test1(S1 u)
2428+
{
2429+
switch (u)
2430+
{
2431+
case 10: goto case 44;
2432+
case ""11"": goto case ""55"";
2433+
case 44: return 44;
2434+
case ""55"": return 55;
2435+
}
2436+
2437+
return -1;
2438+
}
2439+
}
2440+
";
2441+
var comp = CreateCompilation([src, IUnionSource], options: TestOptions.ReleaseExe);
2442+
CompileAndVerify(comp, expectedOutput: "44-1-1-155-1").VerifyDiagnostics();
2443+
}
2444+
2445+
[Fact]
2446+
public void UnionMatching_38_SwitchStatement()
2447+
{
2448+
var src = @"
2449+
struct S1 : System.Runtime.CompilerServices.IUnion
2450+
{
2451+
private readonly object _value;
2452+
public S1(int x) { _value = x; }
2453+
public S1(string x) { _value = x; }
2454+
object System.Runtime.CompilerServices.IUnion.Value => _value;
2455+
}
2456+
2457+
class Program
2458+
{
2459+
static int Test1(S1 u)
2460+
{
2461+
switch (u)
2462+
{
2463+
case 10: return 1;
2464+
case ""11"": return 2;
2465+
case true: return 3;
2466+
}
2467+
2468+
return -1;
2469+
}
2470+
}
2471+
";
2472+
var comp = CreateCompilation([src, IUnionSource]);
2473+
comp.VerifyDiagnostics(
2474+
// (18,18): error CS8121: An expression of type 'S1' cannot be handled by a pattern of type 'bool'.
2475+
// case true: return 3;
2476+
Diagnostic(ErrorCode.ERR_PatternWrongType, "true").WithArguments("S1", "bool").WithLocation(18, 18)
2477+
);
2478+
}
2479+
2480+
[Fact]
2481+
public void UnionMatching_39_SwitchStatement()
2482+
{
2483+
var src = @"
2484+
struct S1 : System.Runtime.CompilerServices.IUnion
2485+
{
2486+
private readonly object _value;
2487+
public S1(int x) { _value = x; }
2488+
public S1(string x) { _value = x; }
2489+
object System.Runtime.CompilerServices.IUnion.Value => _value;
2490+
}
2491+
2492+
class Program
2493+
{
2494+
static int Test1(S1 u)
2495+
{
2496+
switch (u)
2497+
{
2498+
case 10: goto case true;
2499+
case ""11"": return 2;
2500+
}
2501+
2502+
return -1;
2503+
}
2504+
}
2505+
";
2506+
var comp = CreateCompilation([src, IUnionSource]);
2507+
comp.VerifyDiagnostics(
2508+
// (16,13): error CS0163: Control cannot fall through from one case label ('case 10:') to another
2509+
// case 10: goto case true;
2510+
Diagnostic(ErrorCode.ERR_SwitchFallThrough, "case 10:").WithArguments("case 10:").WithLocation(16, 13),
2511+
// (16,22): error CS0029: Cannot implicitly convert type 'bool' to 'S1'
2512+
// case 10: goto case true;
2513+
Diagnostic(ErrorCode.ERR_NoImplicitConv, "goto case true;").WithArguments("bool", "S1").WithLocation(16, 22)
2514+
);
2515+
}
2516+
23632517
[Fact]
23642518
public void PatternWrongType_TypePattern_01_BindConstantPatternWithFallbackToTypePattern_UnionType_Out_UnionType_In()
23652519
{
@@ -3148,7 +3302,8 @@ static void Test1(S1 u)
31483302
// case 1:
31493303
Diagnostic(ErrorCode.ERR_SwitchFallOut, "case 1:").WithArguments("case 1:").WithLocation(102, 13),
31503304

3151-
// PROTOTYPE: This doesn't look like a union matching error. Something is likely missing in implementation.
3305+
// The following error is expected per language specification (https://github.com/dotnet/csharpstandard/blob/draft-v8/standard/statements.md#13104-the-goto-statement):
3306+
// "if the constant_expression is not implicitly convertible (§10.2) to the governing type of the nearest enclosing switch statement, a compile-time error occurs."
31523307

31533308
// (103,17): error CS0029: Cannot implicitly convert type 'string' to 'S1'
31543309
// goto case empty;
@@ -5222,8 +5377,6 @@ static S1 Test1(int? x)
52225377
";
52235378
var comp = CreateCompilation([src, IUnionSource]);
52245379

5225-
// PROTOTYPE: Confirm that there are no lifted forms.
5226-
52275380
comp.VerifyDiagnostics(
52285381
// (20,16): error CS0029: Cannot implicitly convert type 'int?' to 'S1'
52295382
// return x;
@@ -5781,11 +5934,6 @@ static S1 Test2(System.ValueType x)
57815934
var comp = CreateCompilation([src, IUnionSource], options: TestOptions.ReleaseExe);
57825935
var verifier = CompileAndVerify(comp, expectedOutput: "System.ValueType S1").VerifyDiagnostics();
57835936

5784-
// PROTOTYPE: Confirm that we are fine with this conversion behavior. See previous test as well.
5785-
// Cast performs unboxing conversion, but implicit conversion performs union conversion.
5786-
// Might be too confusing.
5787-
// Note, language disallows user-defined conversions like that, see errors below.
5788-
57895937
verifier.VerifyIL("Program.Test2", @"
57905938
{
57915939
// Code size 7 (0x7)
@@ -5889,11 +6037,6 @@ static S1 Test2(I1 x)
58896037
var comp = CreateCompilation([src, IUnionSource], options: TestOptions.ReleaseExe);
58906038
var verifier = CompileAndVerify(comp, expectedOutput: "I1 S1").VerifyDiagnostics();
58916039

5892-
// PROTOTYPE: Confirm that we are fine with this conversion behavior. See previous test as well.
5893-
// Cast performs unboxing conversion, but implicit conversion performs union conversion.
5894-
// Might be too confusing.
5895-
// Note, language disallows user-defined conversions like that, see errors below.
5896-
58976040
verifier.VerifyIL("Program.Test2", @"
58986041
{
58996042
// Code size 7 (0x7)
@@ -5968,10 +6111,6 @@ static S1 Test2(I1 x)
59686111
var comp = CreateCompilation([src, IUnionSource], options: TestOptions.ReleaseExe);
59696112
var verifier = CompileAndVerify(comp, expectedOutput: "I1 S1 I1 S1").VerifyDiagnostics();
59706113

5971-
// PROTOTYPE: Confirm that we are fine with this conversion behavior.
5972-
// Might be too confusing.
5973-
// Note, language disallows user-defined conversions like that, see errors below.
5974-
59756114
verifier.VerifyIL("Program.Test1", @"
59766115
{
59776116
// Code size 7 (0x7)
@@ -6054,11 +6193,6 @@ static S1 Test2(I1 x)
60546193
var comp = CreateCompilation([src, IUnionSource], options: TestOptions.ReleaseExe);
60556194
var verifier = CompileAndVerify(comp, expectedOutput: "I1 S1").VerifyDiagnostics();
60566195

6057-
// PROTOTYPE: Confirm that we are fine with this conversion behavior.
6058-
// Cast performs castclass conversion, but implicit conversion performs union conversion.
6059-
// Might be too confusing.
6060-
// Note, language disallows user-defined conversions like that, see errors below.
6061-
60626196
verifier.VerifyIL("Program.Test1", @"
60636197
{
60646198
// Code size 7 (0x7)

0 commit comments

Comments
 (0)