Skip to content

Commit

Permalink
Make sure nullability mismatch in constraints specified in different …
Browse files Browse the repository at this point in the history
…partial declarations (types/methods) are properly detected and reported. (#35272)

 Make sure nullability mismatch in constraints specified in different partial declarations (types/methods) are properly detected and reported.

Fixes #30229.
Fixes #35179.

Implements the following LDM decision:

For partial types, the invariant matching from type inference and merging. A mismatch
between two non-oblivious candidates produces an error. No warnings are produced.

For partial methods, nullability has to match with exception for oblivious and we produce warnings.
For the result, we use the implementation signature inside the implementation, and the
declaration signature for the callers.
  • Loading branch information
AlekseyTs committed Apr 29, 2019
1 parent 96cec73 commit b252adb
Show file tree
Hide file tree
Showing 29 changed files with 1,605 additions and 421 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ private void AddBound(TypeWithAnnotations addedBound, HashSet<TypeWithAnnotation

if (collectedBounds[methodTypeParameterIndex] == null)
{
collectedBounds[methodTypeParameterIndex] = new HashSet<TypeWithAnnotations>(TypeWithAnnotations.EqualsComparer.Instance);
collectedBounds[methodTypeParameterIndex] = new HashSet<TypeWithAnnotations>(TypeWithAnnotations.EqualsComparer.ConsiderEverythingComparer);
}

collectedBounds[methodTypeParameterIndex].Add(addedBound);
Expand Down
20 changes: 19 additions & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -2124,7 +2124,7 @@ If such a class is used as a base class and if the deriving class defines a dest
<value>Both partial method declarations, '{0}' and '{1}', must use the same tuple element names.</value>
</data>
<data name="ERR_PartialMethodInconsistentConstraints" xml:space="preserve">
<value>Partial method declarations of '{0}' have inconsistent type parameter constraints</value>
<value>Partial method declarations of '{0}' have inconsistent constraints for type parameter '{1}'</value>
</data>
<data name="ERR_PartialMethodToDelegate" xml:space="preserve">
<value>Cannot create delegate from method '{0}' because it is a partial method without an implementing declaration</value>
Expand Down Expand Up @@ -5910,4 +5910,10 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="IDS_OverrideWithConstraints" xml:space="preserve">
<value>constraints for override and explicit interface implementation methods</value>
</data>
<data name="WRN_NullabilityMismatchInConstraintsOnPartialImplementation" xml:space="preserve">
<value>Partial method declarations of '{0}' have inconsistent nullability in constraints for type parameter '{1}'</value>
</data>
<data name="WRN_NullabilityMismatchInConstraintsOnPartialImplementation_Title" xml:space="preserve">
<value>Partial method declarations have inconsistent nullability in constraints for type parameter</value>
</data>
</root>
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,8 @@ internal enum ErrorCode
ERR_OverrideRefConstraintNotSatisfied = 8665,
ERR_OverrideValConstraintNotSatisfied = 8666,

WRN_NullabilityMismatchInConstraintsOnPartialImplementation = 8667,

ERR_MultipleAnalyzerConfigsInSameDir = 8700,

ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation = 8701,
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ internal static int GetWarningLevel(ErrorCode code)
case ErrorCode.WRN_NullabilityMismatchInReturnTypeOnOverride:
case ErrorCode.WRN_NullabilityMismatchInParameterTypeOnOverride:
case ErrorCode.WRN_NullabilityMismatchInParameterTypeOnPartial:
case ErrorCode.WRN_NullabilityMismatchInConstraintsOnPartialImplementation:
case ErrorCode.WRN_NullabilityMismatchInTypeOnImplicitImplementation:
case ErrorCode.WRN_NullabilityMismatchInReturnTypeOnImplicitImplementation:
case ErrorCode.WRN_NullabilityMismatchInParameterTypeOnImplicitImplementation:
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 6 additions & 10 deletions src/Compilers/CSharp/Portable/Symbols/MemberSignatureComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -536,15 +536,14 @@ private static bool HaveSameConstraints(Symbol member1, TypeMap typeMap1, Symbol
return HaveSameConstraints(typeParameters1, typeMap1, typeParameters2, typeMap2);
}

public static bool HaveSameConstraints(ImmutableArray<TypeParameterSymbol> typeParameters1, TypeMap typeMap1, ImmutableArray<TypeParameterSymbol> typeParameters2, TypeMap typeMap2, bool includingNullability = false)
public static bool HaveSameConstraints(ImmutableArray<TypeParameterSymbol> typeParameters1, TypeMap typeMap1, ImmutableArray<TypeParameterSymbol> typeParameters2, TypeMap typeMap2)
{
Debug.Assert(typeParameters1.Length == typeParameters2.Length);

int arity = typeParameters1.Length;
for (int i = 0; i < arity; i++)
{
if (!HaveSameConstraints(typeParameters1[i], typeMap1, typeParameters2[i], typeMap2) ||
(includingNullability && !HaveSameNullabilityInConstraints(typeParameters1[i], typeMap1, typeParameters2[i], typeMap2, unknownNullabilityMatchesAny: false)))
if (!HaveSameConstraints(typeParameters1[i], typeMap1, typeParameters2[i], typeMap2))
{
return false;
}
Expand Down Expand Up @@ -596,23 +595,20 @@ private static bool HaveSameTypeConstraints(TypeParameterSymbol typeParameter1,
AreConstraintTypesSubset(substitutedTypes2, substitutedTypes1, typeParameter1);
}

public static bool HaveSameNullabilityInConstraints(TypeParameterSymbol typeParameter1, TypeMap typeMap1, TypeParameterSymbol typeParameter2, TypeMap typeMap2, bool unknownNullabilityMatchesAny = true)
public static bool HaveSameNullabilityInConstraints(TypeParameterSymbol typeParameter1, TypeMap typeMap1, TypeParameterSymbol typeParameter2, TypeMap typeMap2)
{
if (!typeParameter1.IsValueType)
{
bool? isNotNullableIfReferenceType1 = typeParameter1.IsNotNullableIfReferenceType;
bool? isNotNullableIfReferenceType2 = typeParameter2.IsNotNullableIfReferenceType;
if (isNotNullableIfReferenceType1 != isNotNullableIfReferenceType2 &&
!(unknownNullabilityMatchesAny && (!isNotNullableIfReferenceType1.HasValue || !isNotNullableIfReferenceType2.HasValue)))
if (isNotNullableIfReferenceType1.HasValue && isNotNullableIfReferenceType2.HasValue &&
isNotNullableIfReferenceType1.GetValueOrDefault() != isNotNullableIfReferenceType2.GetValueOrDefault())
{
return false;
}
}

return HaveSameTypeConstraints(typeParameter1, typeMap1, typeParameter2, typeMap2,
unknownNullabilityMatchesAny ?
TypeSymbol.EqualsAllIgnoreOptionsPlusNullableWithUnknownMatchesAnyComparer :
TypeSymbol.EqualsAllIgnoreOptionsPlusNullableComparer);
return HaveSameTypeConstraints(typeParameter1, typeMap1, typeParameter2, typeMap2, TypeSymbol.EqualsAllIgnoreOptionsPlusNullableWithUnknownMatchesAnyComparer);
}

/// <summary>
Expand Down

0 comments on commit b252adb

Please sign in to comment.