Skip to content

Commit

Permalink
Revert workarounds for IEquatable and nullable (dotnet#795)
Browse files Browse the repository at this point in the history
Roslyn now special-cases IEquatable to be contravariant for nullable, so we can remove our workarounds.
  • Loading branch information
stephentoub authored and safern committed Dec 13, 2019
1 parent 3782737 commit d605591
Show file tree
Hide file tree
Showing 17 changed files with 143 additions and 587 deletions.
27 changes: 4 additions & 23 deletions docs/coding-guidelines/api-guidelines/nullability.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,30 +77,11 @@ In contrast, we've annotated `Exception.Message` (which is also virtual) as bein

### Interface implementations

- **DO** implement `IComparable<T?>` instead of `IComparable<T>` on a type `T` that is a reference type.
- **DO** make an `IEquatable<T>` implementation oblivious when implemented on a type `T` that is a reference type. `IEquatable<T>` is invariant by design. However, the lack of covariance means that `T?` couldn't be used in places constrained to `T : IEquatable<T>`. Until a better solution is available, we make such implementations oblivious so they can be used in all places, e.g.
- **DO** implement `IComparable<T?>` instead of `IComparable<T>` on a reference type `T`.
- **DO** implement `IEquatable<T?>` instead of `IEquatable<T>` on a reference type `T`.
e.g.
```C#
public sealed class Version : ICloneable, IComparable, IComparable<Version?>,
#nullable disable // see comment on String
IEquatable<Version>
#nullable restore
```

### Generic constraints

Some special rules need to apply to generic constraints:

- **DO** make a generic constraint oblivious when constraining `T : IEquatable<T>`. As with the above interfaces discussion, this is to work around a limitation, where we want the `T` to be usable with both `T` and `T?` (but `IEquatable<T>` is invariant). For example, instead of defining a method like:
```C#
public static bool Contains<T>(this System.Span<T> span, T value)
where T : IEquatable<T>
```
it should be defined as:
```C#
public static bool Contains<T>(this System.Span<T> span, T value)
#nullable disable
where T : IEquatable<T>
#nullable restore
public sealed class Version : IComparable<Version?>, IEquatable<Version?>, ...
```

### Nullable attributes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ internal virtual int LastIndexOf(T[] array, T value, int startIndex, int count)
}
}

public sealed partial class GenericEqualityComparer<T> : EqualityComparer<T>
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
where T : IEquatable<T>
#nullable restore
public sealed partial class GenericEqualityComparer<T> : EqualityComparer<T> where T : IEquatable<T>
{
internal override int IndexOf(T[] array, T value, int startIndex, int count)
{
Expand Down Expand Up @@ -81,10 +78,7 @@ internal override int LastIndexOf(T[] array, T value, int startIndex, int count)
}
}

public sealed partial class NullableEqualityComparer<T> : EqualityComparer<T?> where T : struct,
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
IEquatable<T>
#nullable restore
public sealed partial class NullableEqualityComparer<T> : EqualityComparer<T?> where T : struct, IEquatable<T>
{
internal override int IndexOf(T?[] array, T? value, int startIndex, int count)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@ namespace System
/// <summary>
/// Represents an immutable string of UTF-8 code units.
/// </summary>
public sealed partial class Utf8String : IComparable<Utf8String?>,
#nullable disable // see comment on String
IEquatable<Utf8String>
#nullable restore
public sealed partial class Utf8String : IComparable<Utf8String?>, IEquatable<Utf8String?>
{
/*
* STATIC FIELDS
Expand Down
265 changes: 53 additions & 212 deletions src/libraries/System.Memory/ref/System.Memory.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ bool IEqualityComparer.Equals(object? x, object? y)
[Serializable]
[TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")]
// Needs to be public to support binary serialization compatibility
public sealed partial class GenericEqualityComparer<T> : EqualityComparer<T>
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
where T : IEquatable<T>
#nullable restore
public sealed partial class GenericEqualityComparer<T> : EqualityComparer<T> where T : IEquatable<T>
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public override bool Equals([AllowNull] T x, [AllowNull] T y)
Expand Down Expand Up @@ -73,10 +70,7 @@ public override int GetHashCode() =>
[Serializable]
[TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")]
// Needs to be public to support binary serialization compatibility
public sealed partial class NullableEqualityComparer<T> : EqualityComparer<T?> where T : struct,
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
IEquatable<T>
#nullable restore
public sealed partial class NullableEqualityComparer<T> : EqualityComparer<T?> where T : struct, IEquatable<T>
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public override bool Equals(T? x, T? y)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ namespace System.Globalization
{
[Serializable]
[System.Runtime.CompilerServices.TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")]
public sealed class SortVersion :
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
IEquatable<SortVersion>
#nullable restore
public sealed class SortVersion : IEquatable<SortVersion?>
{
private readonly int m_NlsVersion; // Do not rename (binary serialization)
private Guid m_SortId; // Do not rename (binary serialization)
Expand Down
Loading

0 comments on commit d605591

Please sign in to comment.