Skip to content

Commit

Permalink
Help compiler enforce nullability annotations (#32090)
Browse files Browse the repository at this point in the history
* Help compiler enforce nullability annotations

* Revert repro changes

* Simplifications

* Address feedback

* Tweak

* Another cleanup

* Fixup on ImportTypes

* Revert from T? to [AllowNull]T

* Enable nullability on one file

* Addressing PR feedback from Stephen

* Use Debug.Assert instead of pragma

* Revert "Use Debug.Assert instead of pragma"

This reverts commit f7175ba.
  • Loading branch information
jcouv committed Feb 14, 2020
1 parent eedeeb6 commit 0f834db
Show file tree
Hide file tree
Showing 36 changed files with 93 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public static partial class Environment

// Terminates this process with the given exit code.
[DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)]
[DoesNotReturn]
private static extern void _Exit(int exitCode);

[DoesNotReturn]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable enable
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

namespace System.Collections.Generic
Expand All @@ -15,7 +17,7 @@ private ReferenceEqualityComparer()
{
}

public bool Equals(T x, T y)
public bool Equals(T? x, T? y)
{
return ReferenceEquals(x, y);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -918,11 +918,13 @@ public TValue this[TKey key]
// as these are uncommonly needed and when inlined are observed to prevent the inlining
// of important methods like TryGetValue and ContainsKey.

[DoesNotReturn]
private static void ThrowKeyNotFoundException(object key)
{
throw new KeyNotFoundException(SR.Format(SR.Arg_KeyNotFoundWithKey, key.ToString()));
}

[DoesNotReturn]
private static void ThrowKeyNullException()
{
throw new ArgumentNullException("key");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace System.Collections.Immutable
{
Expand Down Expand Up @@ -120,6 +121,7 @@ internal interface IImmutableListQueries<T> : IReadOnlyList<T>
/// The first element that matches the conditions defined by the specified predicate,
/// if found; otherwise, the default value for type <typeparamref name="T"/>.
/// </returns>
[return: MaybeNull]
T Find(Predicate<T> match);

/// <summary>
Expand Down Expand Up @@ -193,6 +195,7 @@ internal interface IImmutableListQueries<T> : IReadOnlyList<T>
/// The last element that matches the conditions defined by the specified predicate,
/// if found; otherwise, the default value for type <typeparamref name="T"/>.
/// </returns>
[return: MaybeNull]
T FindLast(Predicate<T> match);

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public ImmutableHashSet<T> Remove(T item)
[Pure]
public bool TryGetValue(T equalValue, out T actualValue)
{
int hashCode = _equalityComparer.GetHashCode(equalValue);
int hashCode = equalValue != null ? _equalityComparer.GetHashCode(equalValue) : 0;
HashBucket bucket;
if (_root.TryGetValue(hashCode, out bucket))
{
Expand Down Expand Up @@ -638,7 +638,7 @@ private static bool IsSupersetOf(IEnumerable<T> other, MutationInput origin)
private static MutationResult Add(T item, MutationInput origin)
{
OperationResult result;
int hashCode = origin.EqualityComparer.GetHashCode(item);
int hashCode = item != null ? origin.EqualityComparer.GetHashCode(item) : 0;
HashBucket bucket = origin.Root.GetValueOrDefault(hashCode);
var newBucket = bucket.Add(item, origin.EqualityComparer, out result);
if (result == OperationResult.NoChangeRequired)
Expand All @@ -657,7 +657,7 @@ private static MutationResult Add(T item, MutationInput origin)
private static MutationResult Remove(T item, MutationInput origin)
{
var result = OperationResult.NoChangeRequired;
int hashCode = origin.EqualityComparer.GetHashCode(item);
int hashCode = item != null ? origin.EqualityComparer.GetHashCode(item) : 0;
HashBucket bucket;
var newRoot = origin.Root;
if (origin.Root.TryGetValue(hashCode, out bucket))
Expand All @@ -679,7 +679,7 @@ private static MutationResult Remove(T item, MutationInput origin)
/// </summary>
private static bool Contains(T item, MutationInput origin)
{
int hashCode = origin.EqualityComparer.GetHashCode(item);
int hashCode = item != null ? origin.EqualityComparer.GetHashCode(item) : 0;
HashBucket bucket;
if (origin.Root.TryGetValue(hashCode, out bucket))
{
Expand All @@ -700,7 +700,7 @@ private static MutationResult Union(IEnumerable<T> other, MutationInput origin)
var newRoot = origin.Root;
foreach (var item in other.GetEnumerableDisposable<T, Enumerator>())
{
int hashCode = origin.EqualityComparer.GetHashCode(item);
int hashCode = item != null ? origin.EqualityComparer.GetHashCode(item) : 0;
HashBucket bucket = newRoot.GetValueOrDefault(hashCode);
OperationResult result;
var newBucket = bucket.Add(item, origin.EqualityComparer, out result);
Expand Down Expand Up @@ -811,7 +811,7 @@ private static MutationResult Except(IEnumerable<T> other, IEqualityComparer<T>
var newRoot = root;
foreach (var item in other.GetEnumerableDisposable<T, Enumerator>())
{
int hashCode = equalityComparer.GetHashCode(item);
int hashCode = item != null ? equalityComparer.GetHashCode(item) : 0;
HashBucket bucket;
if (newRoot.TryGetValue(hashCode, out bucket))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ public int GetHashCode(HashSet<T>? obj)
{
foreach (T t in obj)
{
hashCode = hashCode ^ (_comparer.GetHashCode(t) & 0x7FFFFFFF);
if (t != null)
{
hashCode ^= (_comparer.GetHashCode(t) & 0x7FFFFFFF);
}
}
} // else returns hashcode of 0 for null hashsets
return hashCode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ internal override int TotalCount()
// This passes functionality down to the underlying tree, clipping edges if necessary
// There's nothing gained by having a nested subset. May as well draw it from the base
// Cannot increase the bounds of the subset, can only decrease it
public override SortedSet<T> GetViewBetween(T lowerValue, T upperValue)
public override SortedSet<T> GetViewBetween([AllowNull] T lowerValue, [AllowNull] T upperValue)
{
if (_lBoundActive && Comparer.Compare(_min, lowerValue) > 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ public static IEqualityComparer<SortedSet<T>> CreateSetComparer(IEqualityCompare
/// <param name="set2">The second set.</param>
/// <param name="comparer">The fallback comparer to use if the sets do not have equal comparers.</param>
/// <returns><c>true</c> if the sets have equal contents; otherwise, <c>false</c>.</returns>
internal static bool SortedSetEquals(SortedSet<T> set1, SortedSet<T> set2, IComparer<T> comparer)
internal static bool SortedSetEquals(SortedSet<T>? set1, SortedSet<T>? set2, IComparer<T> comparer)
{
if (set1 == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ private SortedSetEqualityComparer(IComparer<T>? comparer, IEqualityComparer<T>?
}

// Use _comparer to keep equals properties intact; don't want to choose one of the comparers.
public bool Equals(SortedSet<T> x, SortedSet<T> y) => SortedSet<T>.SortedSetEquals(x, y, _comparer);
public bool Equals(SortedSet<T>? x, SortedSet<T>? y) => SortedSet<T>.SortedSetEquals(x, y, _comparer);

// IMPORTANT: this part uses the fact that GetHashCode() is consistent with the notion of equality in the set.
public int GetHashCode(SortedSet<T> obj)
Expand All @@ -37,7 +37,10 @@ public int GetHashCode(SortedSet<T> obj)
{
foreach (T t in obj)
{
hashCode = hashCode ^ (_memberEqualityComparer.GetHashCode(t) & 0x7FFFFFFF);
if (t != null)
{
hashCode ^= (_memberEqualityComparer.GetHashCode(t) & 0x7FFFFFFF);
}
}
}
// Returns 0 for null sets.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics;
using System.Linq;

namespace Microsoft.Internal.Collections
Expand Down Expand Up @@ -144,11 +145,13 @@ private static List<T> FastAppendToListAllowNulls<T>(this List<T>? source, T val
}

public static List<T>? FastAppendToListAllowNulls<T>(
this List<T>? source, T value,
this List<T>? source, T? value,
IEnumerable<T>? second)
where T : class
{
if (second == null)
{
Debug.Assert(value != null);
source = source.FastAppendToListAllowNulls(value);
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ protected ComposablePartDefinition()
/// </remarks>
public abstract ComposablePart CreatePart();

internal virtual bool TryGetExports(ImportDefinition definition, [NotNullWhen(true)] out Tuple<ComposablePartDefinition, ExportDefinition>? singleMatch, out IEnumerable<Tuple<ComposablePartDefinition, ExportDefinition>>? multipleMatches)
internal virtual bool TryGetExports(ImportDefinition definition, out Tuple<ComposablePartDefinition, ExportDefinition>? singleMatch, out IEnumerable<Tuple<ComposablePartDefinition, ExportDefinition>>? multipleMatches)
{
singleMatch = null;
multipleMatches = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ internal class ImportType
private readonly bool _isOpenGeneric = false;

[ThreadStatic]
internal static Dictionary<Type, Func<Export, object>>? _castSingleValueCache;
internal static Dictionary<Type, Func<Export, object>?>? _castSingleValueCache;

private static Dictionary<Type, Func<Export, object>> CastSingleValueCache
private static Dictionary<Type, Func<Export, object>?> CastSingleValueCache
{
get
{
return _castSingleValueCache = _castSingleValueCache ?? new Dictionary<Type, Func<Export, object>>();
return _castSingleValueCache = _castSingleValueCache ?? new Dictionary<Type, Func<Export, object>?>();
}
}

Expand Down Expand Up @@ -170,7 +170,7 @@ private static bool IsLazyGenericType(Type genericType)
return (genericType == LazyOfTType) || (genericType == LazyOfTMType);
}

private static bool TryGetCastFunction(Type genericType, bool isOpenGeneric, Type[] arguments, [NotNullWhen(true)] out Func<Export, object>? castFunction)
private static bool TryGetCastFunction(Type genericType, bool isOpenGeneric, Type[] arguments, out Func<Export, object>? castFunction)
{
castFunction = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ public static int ListHashCode<T>(this ReadOnlyCollection<T> list)
int h = 6551;
foreach (T t in list)
{
h ^= (h << 5) ^ cmp.GetHashCode(t);
if (t != null)
{
h ^= (h << 5) ^ cmp.GetHashCode(t);
}
}
return h;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ internal bool HasHandler(InterpretedFrame frame, Exception exception, [NotNullWh
// Unreachable.
// Want to assert that this case isn't hit, but an assertion failure here will be eaten because
// we are in an exception filter. Therefore return true here and assert in the catch block.
handler = null;
handler = null!;
unwrappedException = exception;
return true;
}
Expand Down Expand Up @@ -215,8 +215,9 @@ internal sealed class DebugInfo
private class DebugInfoComparer : IComparer<DebugInfo>
{
//We allow comparison between int and DebugInfo here
int IComparer<DebugInfo>.Compare(DebugInfo d1, DebugInfo d2)
int IComparer<DebugInfo>.Compare(DebugInfo? d1, DebugInfo? d2)
{
Debug.Assert(d1 != null && d2 != null);
if (d1.Index > d2.Index) return 1;
else if (d1.Index == d2.Index) return 0;
else return -1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,19 +91,16 @@ internal int GetHashCode(TInputOutput element)
{
return
(HashCodeMask &
(_elementComparer == null ?
(element == null ? NULL_ELEMENT_HASH_CODE : element.GetHashCode()) :
_elementComparer.GetHashCode(element)))
% _distributionMod;
(element == null ? NULL_ELEMENT_HASH_CODE : (_elementComparer?.GetHashCode(element) ?? element.GetHashCode())))
% _distributionMod;
}

internal int GetHashCode(THashKey key)
{
return
(HashCodeMask &
(_keyComparer == null ?
(key == null ? NULL_ELEMENT_HASH_CODE : key.GetHashCode()) :
_keyComparer.GetHashCode(key))) % _distributionMod;
(key == null ? NULL_ELEMENT_HASH_CODE : (_keyComparer?.GetHashCode(key) ?? key.GetHashCode())))
% _distributionMod;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ internal ForAllEnumerator(QueryOperatorEnumerator<TInput, TKey> source, Action<T
// element action for each element.
//

internal override bool MoveNext([MaybeNull, AllowNull] ref TInput currentElement, ref int currentKey)
internal override bool MoveNext([MaybeNullWhen(false), AllowNull] ref TInput currentElement, ref int currentKey)
{
Debug.Assert(_elementAction != null, "expected a compiled operator");

Expand All @@ -159,14 +159,13 @@ internal override bool MoveNext([MaybeNull, AllowNull] ref TInput currentElement
TInput element = default(TInput)!;
TKey keyUnused = default(TKey)!;
int i = 0;
while (_source.MoveNext(ref element!, ref keyUnused))
while (_source.MoveNext(ref element, ref keyUnused))
{
if ((i++ & CancellationState.POLL_INTERVAL) == 0)
CancellationState.ThrowIfCanceled(_cancellationToken);
_elementAction(element);
}


return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,7 @@ internal TValue this[TKey key]
private int GetKeyHashCode(TKey key)
{
return HashCodeMask &
(comparer == null ?
(key == null ? 0 : key.GetHashCode()) :
comparer.GetHashCode(key));
(key == null ? 0 : (comparer?.GetHashCode(key) ?? key.GetHashCode()));
}

private bool AreKeysEqual(TKey key1, TKey key2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace System.Linq.Parallel
{
Expand All @@ -26,7 +27,7 @@ internal ReverseComparer(IComparer<T> comparer)
_comparer = comparer;
}

public int Compare(T x, T y)
public int Compare([AllowNull] T x, [AllowNull] T y)
{
return _comparer.Compare(y, x);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ public bool Equals(Wrapper<T> x, Wrapper<T> y)
public int GetHashCode(Wrapper<T> x)
{
Debug.Assert(_comparer != null);
return _comparer.GetHashCode(x.Value);
T value = x.Value;
return value == null ? 0 : _comparer.GetHashCode(value);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ private EmptyPartition()
public bool MoveNext() => false;

[ExcludeFromCodeCoverage] // Shouldn't be called, and as undefined can return or throw anything anyway.
[MaybeNull]
public TElement Current => default!;

[ExcludeFromCodeCoverage] // Shouldn't be called, and as undefined can return or throw anything anyway.
Expand Down
4 changes: 3 additions & 1 deletion src/libraries/System.Private.CoreLib/src/System/Action.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics.CodeAnalysis;

namespace System
{
public delegate void Action();
Expand All @@ -24,7 +26,7 @@ namespace System
public delegate TResult Func<in T1, in T2, in T3, in T4, in T5, in T6, in T7, out TResult>(T1 arg1, T2 arg2, T3 arg3, T4 arg4, T5 arg5, T6 arg6, T7 arg7);
public delegate TResult Func<in T1, in T2, in T3, in T4, in T5, in T6, in T7, in T8, out TResult>(T1 arg1, T2 arg2, T3 arg3, T4 arg4, T5 arg5, T6 arg6, T7 arg7, T8 arg8);

public delegate int Comparison<in T>(T x, T y);
public delegate int Comparison<in T>([AllowNull] T x, [AllowNull] T y);

public delegate TOutput Converter<in TInput, out TOutput>(TInput input);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public ComparisonComparer(Comparison<T> comparison)
_comparison = comparison;
}

public override int Compare(T x, T y)
public override int Compare([AllowNull] T x, [AllowNull] T y)
{
return _comparison(x, y);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace System.Collections.Generic
{
// The generic IEqualityComparer interface implements methods to if check two objects are equal
// and generate Hashcode for an object.
// It is use in Dictionary class.
// It is used in Dictionary class.
public interface IEqualityComparer<in T>
{
bool Equals([AllowNull] T x, [AllowNull] T y);
Expand Down
Loading

0 comments on commit 0f834db

Please sign in to comment.