Skip to content

Commit 97447cb

Browse files
AlekseyTsAlekseyTs
authored
Unions: Align implementation with recent design changes around case types and default nullability (#83161)
See: - dotnet/csharplang#10092 - dotnet/csharplang#10098
1 parent 1e9493b commit 97447cb

7 files changed

Lines changed: 517 additions & 265 deletions

File tree

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

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,9 @@ static bool hasHasValueSignature(PropertySymbol property)
219219
{
220220
Conversion conversion = conversions.ClassifyBuiltInConversion(type, candidate.Parameters[0].Type, isChecked: false, ref discardedUseSiteInfo);
221221

222-
if (!conversion.Exists || !conversion.IsImplicit || !(conversion.IsIdentity || conversion.IsReference || conversion.IsBoxing))
222+
if (!conversion.Exists || !conversion.IsImplicit ||
223+
!(conversion.IsIdentity || conversion.IsReference || conversion.IsBoxing ||
224+
(conversion.IsNullable && conversion.UnderlyingConversions[0].IsIdentity)))
223225
{
224226
continue;
225227
}
@@ -239,7 +241,7 @@ static bool hasHasValueSignature(PropertySymbol property)
239241
bool isMatch =
240242
HasTryGetValueSignature(declaredMethod) &&
241243
declaredMethod.GetUseSiteInfo().DiagnosticInfo?.DefaultSeverity != DiagnosticSeverity.Error &&
242-
unionDefinitionCaseTypes.Contains(declaredMethod.Parameters[0].Type, Symbols.SymbolEqualityComparer.AllIgnoreOptions); // https://github.com/dotnet/roslyn/issues/82636: Optimize this check?
244+
HasUnionTypeTryGetValueParameterType(declaredMethod, unionDefinitionCaseTypes);
243245

244246
Debug.Assert(isMatch == IsUnionTypeTryGetValueMethod(inputUnionType, candidate));
245247

@@ -251,8 +253,15 @@ static bool hasHasValueSignature(PropertySymbol property)
251253
}
252254
else
253255
{
254-
Debug.Assert(bestMatch is null || bestMatchConversion.IsReference || bestMatchConversion.IsBoxing);
255-
if (bestMatch is null || (conversion.IsReference && bestMatchConversion.IsBoxing))
256+
Debug.Assert(bestMatch is null ||
257+
bestMatchConversion.IsReference || bestMatchConversion.IsBoxing ||
258+
(bestMatchConversion.IsNullable && bestMatchConversion.UnderlyingConversions[0].IsIdentity));
259+
Debug.Assert(!bestMatchConversion.IsReference || !conversion.IsNullable);
260+
Debug.Assert(!conversion.IsReference || !bestMatchConversion.IsNullable);
261+
Debug.Assert(!bestMatchConversion.IsReference || !conversion.IsBoxing);
262+
Debug.Assert(!conversion.IsReference || !bestMatchConversion.IsBoxing);
263+
264+
if (bestMatch is null || (!conversion.IsBoxing && bestMatchConversion.IsBoxing))
256265
{
257266
bestMatch = candidate;
258267
bestMatchConversion = conversion;
@@ -266,6 +275,27 @@ static bool hasHasValueSignature(PropertySymbol property)
266275
return bestMatch;
267276
}
268277

278+
private static bool HasUnionTypeTryGetValueParameterType(MethodSymbol declaredMethod, ImmutableArray<TypeSymbol> unionDefinitionCaseTypes)
279+
{
280+
return unionDefinitionCaseTypes.Any(
281+
static (caseType, parameterType) =>
282+
{
283+
if (caseType.Equals(parameterType, TypeCompareKind.AllIgnoreOptions))
284+
{
285+
return true;
286+
}
287+
288+
if (caseType.IsNullableType() &&
289+
caseType.GetNullableUnderlyingType().Equals(parameterType, TypeCompareKind.AllIgnoreOptions))
290+
{
291+
return true;
292+
}
293+
294+
return false;
295+
},
296+
declaredMethod.Parameters[0].Type); // https://github.com/dotnet/roslyn/issues/82636: Optimize this check?
297+
}
298+
269299
internal static bool HasTryGetValueSignature(MethodSymbol method)
270300
{
271301
// https://github.com/dotnet/roslyn/issues/82636: Cover individual conditions with tests
@@ -302,7 +332,7 @@ internal static bool IsUnionTypeTryGetValueMethod(NamedTypeSymbol unionType, Met
302332
}
303333

304334
return HasTryGetValueSignature(method) && method.GetUseSiteInfo().DiagnosticInfo?.DefaultSeverity != DiagnosticSeverity.Error &&
305-
unionDefinition.UnionCaseTypes.Contains(method.Parameters[0].Type, Symbols.SymbolEqualityComparer.AllIgnoreOptions); // https://github.com/dotnet/roslyn/issues/82636: Optimize this check?
335+
HasUnionTypeTryGetValueParameterType(method, unionDefinition.UnionCaseTypes);
306336
}
307337
}
308338

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -804,11 +804,11 @@ private TestInputOutputInfo MakeConvertToType(
804804
// Add type evaluation after return value test to separate result value from deconstruct evaluation
805805
// This helps us unify the same value accessed through different Union APIs.
806806
// See IsSameEntity/IsEqualEvaluation helpers.
807-
var typeEvaluation = new BoundDagTypeEvaluation(syntax, outParameterTemp.Type, outParameterTemp);
807+
var typeEvaluation = new BoundDagTypeEvaluation(syntax, outParameterTemp.Type.StrippedType(), outParameterTemp);
808808
tests.Add(new Tests.One(typeEvaluation));
809809
BoundDagTemp typeEvaluationTemp = typeEvaluation.MakeResultTemp();
810810

811-
if (!outParameterTemp.Type.Equals(type, TypeCompareKind.AllIgnoreOptions))
811+
if (!typeEvaluationTemp.Type.Equals(type, TypeCompareKind.AllIgnoreOptions))
812812
{
813813
tests.Add(new Tests.One(new BoundDagTypeTest(syntax, type, typeEvaluationTemp)));
814814
typeEvaluation = new BoundDagTypeEvaluation(syntax, type, typeEvaluationTemp);

‎src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2047,6 +2047,25 @@ private NullableFlowState GetDefaultState(ref LocalState state, int slot)
20472047
}
20482048
return GetParameterState(parameterType, parameter.FlowAnalysisAnnotations).State;
20492049
}
2050+
case PropertySymbol { Name: WellKnownMemberNames.ValuePropertyName } property when
2051+
variable.ContainingSlot is > 0 and var containingSlot &&
2052+
_variables[containingSlot].Symbol.GetTypeOrReturnType().Type is NamedTypeSymbol { IsUnionType: true, UnionCaseTypes: not [] } unionType &&
2053+
Binder.IsUnionTypeValueProperty(unionType, property):
2054+
{
2055+
// For union types where none of the case types are nullable, the default state for Value is "not null" rather than "maybe null".
2056+
var result = NullableFlowState.NotNull;
2057+
2058+
foreach (var ctor in unionType.InstanceConstructors)
2059+
{
2060+
if (NamedTypeSymbol.IsSuitableUnionConstructor(ctor))
2061+
{
2062+
var parameter = ctor.Parameters[0];
2063+
result = result.Join(GetParameterState(parameter.TypeWithAnnotations, parameter.FlowAnalysisAnnotations).State);
2064+
}
2065+
}
2066+
2067+
return result;
2068+
}
20502069

20512070
case FieldSymbol:
20522071
case PropertySymbol:

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1852,7 +1852,7 @@ internal bool IsUnionType
18521852

18531853
static void addCaseType(ArrayBuilder<TypeSymbol> builder, MethodSymbol factory)
18541854
{
1855-
var candidate = factory.Parameters[0].Type.StrippedType();
1855+
var candidate = factory.Parameters[0].Type;
18561856
if (!builder.Any(static (t1, t2) => t1.Equals(t2, TypeCompareKind.AllIgnoreOptions), candidate))
18571857
{
18581858
builder.Add(candidate);

‎src/Compilers/CSharp/Portable/Utilities/TypeUnionValueSet.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ private TypeUnionValueSet(
5151
{
5252
Debug.Assert(!typesInUnion.IsEmpty);
5353
Debug.Assert(!typesInUnion.Any(t => t.IsNullableType()));
54-
Debug.Assert(typesInUnion.Distinct().Length == typesInUnion.Length);
54+
5555
_typesInUnion = typesInUnion;
5656
_root = root;
5757
_conversions = conversions;
@@ -77,7 +77,6 @@ private static bool AnyTypeFromUnionMightMatch(ImmutableArray<TypeSymbol> typesI
7777
{
7878
Debug.Assert(!typesInUnion.IsEmpty);
7979
Debug.Assert(!typesInUnion.Any(TypeSymbolExtensions.IsNullableType));
80-
Debug.Assert(typesInUnion.Distinct().Length == typesInUnion.Length);
8180

8281
foreach (var t in typesInUnion)
8382
{

‎src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.UnionTypeTypeUnionValueSetFactory.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
using System.Collections.Immutable;
66
using System.Diagnostics;
7-
using System.Linq;
87
using Microsoft.CodeAnalysis.CSharp.Symbols;
98

109
namespace Microsoft.CodeAnalysis.CSharp
@@ -23,8 +22,7 @@ public UnionTypeTypeUnionValueSetFactory(NamedTypeSymbol unionType)
2322

2423
private ImmutableArray<TypeSymbol> AdjustedTypesInUnion()
2524
{
26-
Debug.Assert(!_unionType.UnionCaseTypes.Any(t => t.IsNullableType()));
27-
return _unionType.UnionCaseTypes;
25+
return _unionType.UnionCaseTypes.SelectAsArray(TypeSymbolExtensions.StrippedType);
2826
}
2927

3028
public TypeUnionValueSet AllValues(ConversionsBase conversions)

0 commit comments

Comments
 (0)