Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Compare Nullable modifiers while comparing type symbols by default. #30770

Merged
merged 1 commit into from
Oct 29, 2018
Merged

Compare Nullable modifiers while comparing type symbols by default. #30770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Semantics/BestTypeInferrer.cs
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs
View file
Open in desktop
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
22 changes: 11 additions & 11 deletions src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs
View file
Open in desktop
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))
Copy link
Member

@333fred 333fred Oct 25, 2018 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be used relatively often. Might be worth adding an explicit option in the enum. #Pending

{
// 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
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Portable/BoundTree/DecisionTreeBuilder.cs
View file
Open in desktop
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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs
View file
Open in desktop
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
View file
Open in desktop
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