Skip to content

Commit

Permalink
Compare Nullable modifiers while comparing type symbols by default.
Browse files Browse the repository at this point in the history
  • Loading branch information
AlekseyTs committed Oct 25, 2018
1 parent ca387bb commit d74070e
Show file tree
Hide file tree
Showing 48 changed files with 600 additions and 226 deletions.
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ internal static bool IsValidConstraint(
}

// Ignore nullability when comparing constraints.
if (constraintTypes.Contains(c => type.Equals(c, TypeCompareKind.ConsiderEverything)))
if (constraintTypes.Contains(c => type.Equals(c, TypeCompareKind.IgnoreNullableModifiersForReferenceTypes)))
{
// "Duplicate constraint '{0}' for type parameter '{1}'"
Error(diagnostics, ErrorCode.ERR_DuplicateBound, syntax, type.TypeSymbol.SetUnknownNullabilityForReferenceTypes(), typeParameterName);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ protected BoundExpression CreateConversion(

// We need to preserve any conversion that changes the type (even identity conversions, like object->dynamic),
// or that was explicitly written in code (so that GetSemanticInfo can find the syntax in the bound tree).
if (!isCast && source.Type == destination)
if (!isCast && source.Type.Equals(destination, TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
return source;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1388,7 +1388,7 @@ private ImmutableArray<BoundExpression> BuildArgumentsForErrorRecovery(AnalyzedA
{
candidateType = parameterType;
}
else if (!candidateType.Equals(parameterType, TypeCompareKind.IgnoreCustomModifiersAndArraySizesAndLowerBounds))
else if (!candidateType.Equals(parameterType, TypeCompareKind.IgnoreCustomModifiersAndArraySizesAndLowerBounds | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
// type mismatch
candidateType = null;
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1517,6 +1517,11 @@ internal static bool AccessingAutoPropertyFromConstructor(BoundPropertyAccess pr

private static bool AccessingAutoPropertyFromConstructor(BoundExpression receiver, PropertySymbol propertySymbol, Symbol fromMember)
{
if (!propertySymbol.IsDefinition && propertySymbol.ContainingType.Equals(propertySymbol.ContainingType.OriginalDefinition, TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
propertySymbol = propertySymbol.OriginalDefinition;
}

var sourceProperty = propertySymbol as SourcePropertySymbol;
var propertyIsStatic = propertySymbol.IsStatic;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public static TypeSymbol InferBestType(
// SPEC: If no such S exists, the expressions have no best common type.

// All non-null types are candidates for best type inference.
IEqualityComparer<TypeSymbol> comparer = conversions.IncludeNullability ? TypeSymbol.EqualsIncludingNullableComparer : TypeSymbol.EqualsConsiderEverything;
IEqualityComparer<TypeSymbol> comparer = conversions.IncludeNullability ? TypeSymbol.EqualsConsiderEverything : TypeSymbol.EqualsIgnoringNullableComparer;
HashSet<TypeSymbol> candidateTypes = new HashSet<TypeSymbol>(comparer);
foreach (BoundExpression expr in exprs)
{
Expand Down Expand Up @@ -201,7 +201,7 @@ internal static TypeSymbol GetBestType(
}
else
{
if (!better.Equals(best, TypeCompareKind.IgnoreDynamicAndTupleNames))
if (!better.Equals(best, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
hadNullabilityMismatch = false;
}
Expand All @@ -224,7 +224,7 @@ internal static TypeSymbol GetBestType(
{
TypeSymbol type = types[i];
TypeSymbol better = Better(best, type, conversions, out bool hadMismatch, ref useSiteDiagnostics);
if (!best.Equals(better, TypeCompareKind.ConsiderEverything))
if (!best.Equals(better, TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
hadNullabilityMismatch = false;
return null;
Expand Down Expand Up @@ -275,7 +275,7 @@ private static TypeSymbol Better(
return type2;
}

if (type1.Equals(type2, TypeCompareKind.IgnoreDynamicAndTupleNames))
if (type1.Equals(type2, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
return MethodTypeInferrer.Merge(
TypeSymbolWithAnnotations.Create(type1),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1379,7 +1379,7 @@ private static bool HasIdentityConversionInternal(TypeSymbol type1, TypeSymbol t
Debug.Assert((object)type2 != null);

var compareKind = includeNullability ?
TypeCompareKind.AllIgnoreOptions | TypeCompareKind.CompareNullableModifiersForReferenceTypes | TypeCompareKind.UnknownNullableModifierMatchesAny :
TypeCompareKind.AllIgnoreOptions & ~TypeCompareKind.IgnoreNullableModifiersForReferenceTypes :
TypeCompareKind.AllIgnoreOptions;
return type1.Equals(type2, compareKind);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2365,7 +2365,7 @@ private bool Fix(int iParam, ref bool hadNullabilityMismatch, ref HashSet<Diagno
var withoutNullability = Fix(exact, lower, upper, ref ignoredDiagnostics, _conversions.WithNullability(false), ref ignoredHadMismatch);
// https://github.com/dotnet/roslyn/issues/27961 Results may differ by tuple names or dynamic.
// See NullableReferenceTypesTests.TypeInference_TupleNameDifferences_01 for example.
Debug.Assert(best.TypeSymbol.Equals(withoutNullability.TypeSymbol, TypeCompareKind.IgnoreDynamicAndTupleNames));
Debug.Assert(best.TypeSymbol.Equals(withoutNullability.TypeSymbol, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes));
}
#endif

Expand Down Expand Up @@ -2394,7 +2394,7 @@ private static TypeSymbolWithAnnotations Fix(

// Optimization: if we have two or more exact bounds, fixing is impossible.

var candidates = new Dictionary<TypeSymbolWithAnnotations, TypeSymbolWithAnnotations>(EqualsIgnoringDynamicAndTupleNamesComparer.Instance);
var candidates = new Dictionary<TypeSymbolWithAnnotations, TypeSymbolWithAnnotations>(EqualsIgnoringDynamicTupleNamesAndNullabilityComparer.Instance);

// Optimization: if we have one exact bound then we need not add any
// inexact bounds; we're just going to remove them anyway.
Expand Down Expand Up @@ -2458,7 +2458,7 @@ private static TypeSymbolWithAnnotations Fix(
{
foreach (var candidate2 in initialCandidates)
{
if (!candidate.Equals(candidate2, TypeCompareKind.CompareNullableModifiersForReferenceTypes) &&
if (!candidate.Equals(candidate2, TypeCompareKind.ConsiderEverything) &&
!ImplicitConversionExists(candidate2, candidate, ref useSiteDiagnostics, conversions.WithNullability(false)))
{
goto OuterBreak;
Expand All @@ -2471,7 +2471,7 @@ private static TypeSymbolWithAnnotations Fix(
}
else
{
Debug.Assert(!best.Equals(candidate, TypeCompareKind.IgnoreDynamicAndTupleNames));
Debug.Assert(!best.Equals(candidate, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes));
// best candidate is not unique
best = default;
break;
Expand Down Expand Up @@ -2658,7 +2658,7 @@ private TypeSymbolWithAnnotations InferReturnType(BoundExpression source, NamedT
{
for (int p = 0; p < anonymousFunction.ParameterCount; ++p)
{
if (!anonymousFunction.ParameterType(p).TypeSymbol.Equals(fixedDelegateParameters[p].Type.TypeSymbol, TypeCompareKind.IgnoreDynamicAndTupleNames))
if (!anonymousFunction.ParameterType(p).TypeSymbol.Equals(fixedDelegateParameters[p].Type.TypeSymbol, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
return default;
}
Expand Down Expand Up @@ -2851,7 +2851,7 @@ private static void AddOrMergeCandidate(
ref bool hadNullabilityMismatch)
{
Debug.Assert(conversions.IncludeNullability ||
newCandidate.SetUnknownNullabilityForReferenceTypes().Equals(newCandidate, TypeCompareKind.CompareNullableModifiersForReferenceTypes));
newCandidate.SetUnknownNullabilityForReferenceTypes().Equals(newCandidate, TypeCompareKind.ConsiderEverything));

if (candidates.TryGetValue(newCandidate, out TypeSymbolWithAnnotations oldCandidate))
{
Expand Down Expand Up @@ -2880,7 +2880,7 @@ private static void MergeOrRemoveCandidates(
Debug.Assert(variance == VarianceKind.In || variance == VarianceKind.Out);
// SPEC: For each lower (upper) bound U of Xi all types to which there is not an
// SPEC: implicit conversion from (to) U are removed from the candidate set.
var comparison = conversions.IncludeNullability ? TypeCompareKind.CompareNullableModifiersForReferenceTypes : TypeCompareKind.ConsiderEverything;
var comparison = conversions.IncludeNullability ? TypeCompareKind.ConsiderEverything : TypeCompareKind.IgnoreNullableModifiersForReferenceTypes;
foreach (var bound in bounds)
{
foreach (var candidate in initialCandidates)
Expand All @@ -2905,7 +2905,7 @@ private static void MergeOrRemoveCandidates(
{
candidates.Remove(candidate);
}
else if (bound.Equals(candidate, TypeCompareKind.IgnoreDynamicAndTupleNames))
else if (bound.Equals(candidate, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
// SPEC: 4.7 The Dynamic Type
// Type inference (7.5.2) will prefer dynamic over object if both are candidates.
Expand Down Expand Up @@ -2956,9 +2956,9 @@ private static void MergeAndReplaceIfStillCandidate(
/// This is a comparer that ignores differences in dynamic-ness and tuple names.
/// But it has a special case for top-level object vs. dynamic for purpose of method type inference.
/// </summary>
private sealed class EqualsIgnoringDynamicAndTupleNamesComparer : EqualityComparer<TypeSymbolWithAnnotations>
private sealed class EqualsIgnoringDynamicTupleNamesAndNullabilityComparer : EqualityComparer<TypeSymbolWithAnnotations>
{
internal static readonly EqualsIgnoringDynamicAndTupleNamesComparer Instance = new EqualsIgnoringDynamicAndTupleNamesComparer();
internal static readonly EqualsIgnoringDynamicTupleNamesAndNullabilityComparer Instance = new EqualsIgnoringDynamicTupleNamesAndNullabilityComparer();

public override int GetHashCode(TypeSymbolWithAnnotations obj)
{
Expand All @@ -2971,7 +2971,7 @@ public override bool Equals(TypeSymbolWithAnnotations x, TypeSymbolWithAnnotatio
// but dynamic and object are not considered equal for backwards compatibility.
if (x.IsDynamic() ^ y.IsDynamic()) { return false; }

return x.Equals(y, TypeCompareKind.IgnoreDynamicAndTupleNames);
return x.Equals(y, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ private DecisionTree AddByValue(DecisionTree.Guarded guarded, BoundConstantPatte

private DecisionTree AddByValue(DecisionTree.ByValue byValue, BoundConstantPattern value, DecisionMaker makeDecision)
{
Debug.Assert(value.Value.Type.Equals(byValue.Type, TypeCompareKind.IgnoreDynamicAndTupleNames));
Debug.Assert(value.Value.Type.Equals(byValue.Type, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes));
if (byValue.Default != null)
{
return AddByValue(byValue.Default, value, makeDecision);
Expand Down Expand Up @@ -277,7 +277,7 @@ private DecisionTree AddByValue(DecisionTree.ByType byType, BoundConstantPattern
var kvp = byType.TypeAndDecision[i];
var matchedType = kvp.Key;
var decision = kvp.Value;
if (matchedType.Equals(value.Value.Type, TypeCompareKind.IgnoreDynamicAndTupleNames))
if (matchedType.Equals(value.Value.Type, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
forType = decision;
break;
Expand Down Expand Up @@ -354,7 +354,7 @@ private DecisionTree AddByType(DecisionTree decision, TypeSymbol type, DecisionM
DecisionTree result;
if (byValue.Default == null)
{
if (byValue.Type.Equals(type, TypeCompareKind.IgnoreDynamicAndTupleNames))
if (byValue.Type.Equals(type, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
result = byValue.Default = makeDecision(byValue.Expression, byValue.Type);
}
Expand Down Expand Up @@ -421,7 +421,7 @@ private DecisionTree AddByType(DecisionTree.ByType byType, TypeSymbol type, Deci
if (byType.TypeAndDecision.Count != 0)
{
var lastTypeAndDecision = byType.TypeAndDecision.Last();
if (lastTypeAndDecision.Key.Equals(type, TypeCompareKind.IgnoreDynamicAndTupleNames))
if (lastTypeAndDecision.Key.Equals(type, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
result = Add(lastTypeAndDecision.Value, makeDecision);
}
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ internal abstract class UnboundLambdaState
[PerformanceSensitive(
"https://github.com/dotnet/roslyn/issues/23582",
Constraint = "Avoid " + nameof(ConcurrentDictionary<NamedTypeSymbol, BoundLambda>) + " which has a large default size, but this cache is normally small.")]
private ImmutableDictionary<NamedTypeSymbol, BoundLambda> _bindingCache = ImmutableDictionary<NamedTypeSymbol, BoundLambda>.Empty.WithComparers(TypeSymbol.EqualsIncludingNullableComparer);
private ImmutableDictionary<NamedTypeSymbol, BoundLambda> _bindingCache = ImmutableDictionary<NamedTypeSymbol, BoundLambda>.Empty.WithComparers(TypeSymbol.EqualsConsiderEverything);

[PerformanceSensitive(
"https://github.com/dotnet/roslyn/issues/23582",
Expand Down Expand Up @@ -468,7 +468,7 @@ private BoundLambda ReallyBind(NamedTypeSymbol delegateType)
lambdaSymbol = returnInferenceLambda.Symbol;
var lambdaReturnType = lambdaSymbol.ReturnType;
if ((object)LambdaSymbol.InferenceFailureReturnType != lambdaReturnType.TypeSymbol &&
lambdaReturnType.Equals(returnType, TypeCompareKind.CompareNullableModifiersForReferenceTypes) && lambdaSymbol.RefKind == refKind)
lambdaReturnType.Equals(returnType, TypeCompareKind.ConsiderEverything) && lambdaSymbol.RefKind == refKind)
{
lambdaBodyBinder = returnInferenceLambda.Binder;
block = returnInferenceLambda.Body;
Expand Down Expand Up @@ -663,7 +663,7 @@ public override bool Equals(object obj)

for (int i = 0; i < this.ParameterTypes.Length; i++)
{
if (!other.ParameterTypes[i].Equals(this.ParameterTypes[i], TypeCompareKind.CompareNullableModifiersForReferenceTypes) ||
if (!other.ParameterTypes[i].Equals(this.ParameterTypes[i], TypeCompareKind.ConsiderEverything) ||
other.ParameterRefKinds[i] != this.ParameterRefKinds[i])
{
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,7 @@ public bool Equals(TypeSymbol source, TypeSymbol other)
var visitedSource = (TypeSymbol)_matcher.Visit(source);
var visitedOther = (_deepTranslatorOpt != null) ? (TypeSymbol)_deepTranslatorOpt.Visit(other) : other;

return visitedSource?.Equals(visitedOther, TypeCompareKind.IgnoreDynamicAndTupleNames) == true;
return visitedSource?.Equals(visitedOther, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes) == true;
}
}
}
Expand Down
5 changes: 1 addition & 4 deletions src/Compilers/CSharp/Portable/Emitter/Model/SymbolAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,7 @@ void Cci.IReference.Dispatch(Cci.MetadataVisitor visitor)
/// </summary>
internal bool IsDefinitionOrDistinct()
{
return this.IsDefinition ||
!(this is NamedTypeSymbol namedTypeSymbol ?
namedTypeSymbol.Equals(namedTypeSymbol.OriginalDefinition, TypeCompareKind.CompareNullableModifiersForReferenceTypes) :
this.Equals(this.OriginalDefinition));
return this.IsDefinition || !this.Equals(this.OriginalDefinition);
}

IEnumerable<Cci.ICustomAttribute> Cci.IReference.GetAttributes(EmitContext context)
Expand Down
Loading

0 comments on commit d74070e

Please sign in to comment.