Skip to content

React to latest Roslyn nullability changes #36104

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.

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
<UsingToolMicrosoftNetCompilers>true</UsingToolMicrosoftNetCompilers>
<UsingToolIbcOptimization>true</UsingToolIbcOptimization>
<UsingToolXliff>false</UsingToolXliff>
<!-- Downgrade compiler version to fix nullability issues: https://github.com/dotnet/runtime/issues/34417. -->
<MicrosoftNetCompilersToolsetVersion>3.6.0-2.20166.2</MicrosoftNetCompilersToolsetVersion>
<!-- Upgrade compiler version to work around build failures: https://github.com/dotnet/runtime/issues/34417. -->
<MicrosoftNetCompilersToolsetVersion>3.7.0-2.20258.1</MicrosoftNetCompilersToolsetVersion>
<!-- Blob storage container that has the "Latest" channel to publish to. -->
<ContainerName>dotnet</ContainerName>
<ChecksumContainerName>$(ContainerName)</ChecksumContainerName>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

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

namespace System.Collections.Generic
{
Expand Down Expand Up @@ -36,12 +37,12 @@ public void Add(T1 item1, T2 item2)
_backward.Add(item2, item1);
}

public bool TryGetForward(T1 item1, out T2 item2)
public bool TryGetForward(T1 item1, [MaybeNullWhen(false)] out T2 item2)
{
return _forward.TryGetValue(item1, out item2);
}

public bool TryGetBackward(T2 item2, out T1 item1)
public bool TryGetBackward(T2 item2, [MaybeNullWhen(false)] out T1 item1)
{
return _backward.TryGetValue(item2, out item1);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public void Clear() { }
void System.Collections.Generic.IDictionary<TKey, TValue>.Add(TKey key, TValue value) { }
bool System.Collections.Generic.IDictionary<TKey, TValue>.Remove(TKey key) { throw null; }
void System.Collections.ICollection.CopyTo(System.Array array, int index) { }
void System.Collections.IDictionary.Add(object key, object value) { }
void System.Collections.IDictionary.Add(object key, object? value) { }
bool System.Collections.IDictionary.Contains(object key) { throw null; }
System.Collections.IDictionaryEnumerator System.Collections.IDictionary.GetEnumerator() { throw null; }
void System.Collections.IDictionary.Remove(object key) { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,13 @@ private bool TryTakeWithNoTimeValidation([MaybeNullWhen(false)] out T item, int
}
}
}

#pragma warning disable CS8762
// https://github.com/dotnet/runtime/issues/36132
// Compiler can't automatically deduce that nullability constraints
// for 'item' are satisfied at this exit point.
return waitForSemaphoreWasSuccessful;
#pragma warning restore CS8762
Copy link
Member

Choose a reason for hiding this comment

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

There's no way to suppress this warning with a well-placed ! somewhere? The clutter caused by ! is bad enough, but adding a bunch of pragmas to suppress false positive nullable warnings just makes me sad.
cc: @jcouv

Copy link
Member

Choose a reason for hiding this comment

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

It's still not great, but possibly a bit better: add a return true; below the Debug.Assert(item != null); at line 761.

From the language/compiler perspective, it may be useful to allow return waitForSemaphoreWasSuccessful!; to solve this.


In reply to: 422141700 [](ancestors = 422141700)

Copy link
Member

Choose a reason for hiding this comment

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

From the language/compiler perspective, it may be useful to allow return waitForSemaphoreWasSuccessful!; to solve this.

@jcouv, yeah, let's do that 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

I did consider using the if (condition) { assert(); return true; } else { return false; } pattern, but it produces different IL and codegen than a simple return condition;. I tried to avoid making changes that affected the codegen in any way.

Copy link
Member

Choose a reason for hiding this comment

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

Argh, I missed that the Debug.Assert(item != null) doesn't work here. An alternative would be _ = item!; to pretend that it's not null, but that smells in this case, and also that's not yet implemented.
I'll push on allowing suppression on return someBoolValue!;.

Copy link
Member

Choose a reason for hiding this comment

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

Another option (also smells) is to suppress on places that assign to item (pretend like we're never assigning maybe-null values)


In reply to: 422242826 [](ancestors = 422242826)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, using the ! operator inappropriately early in the method strikes me as undesirable. I'll leave the pragma for now and add a code comment directing to #36132. Once the language team settles on guidance for this we can react to it here.

Much appreciate your responsiveness on this! :)

}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ private WorkStealingQueue CreateWorkStealingQueueForCurrentThread()
/// <param name="result">To receive the item retrieved from the bag</param>
/// <param name="take">Whether to remove or peek.</param>
/// <returns>True if succeeded, false otherwise.</returns>
private bool TrySteal(out T result, bool take)
private bool TrySteal([MaybeNullWhen(false)] out T result, bool take)
{
if (CDSCollectionETWBCLProvider.Log.IsEnabled())
{
Expand Down Expand Up @@ -219,7 +219,12 @@ private bool TrySteal(out T result, bool take)
(TryStealFromTo(localQueue._nextQueue, null, out result, take) || TryStealFromTo(_workStealingQueues, localQueue, out result, take));
if (gotItem)
{
#pragma warning disable CS8762
// https://github.com/dotnet/runtime/issues/36132
// Compiler can't automatically deduce that nullability constraints
// for 'result' are satisfied at this exit point.
return true;
#pragma warning restore CS8762
}

if (Interlocked.Read(ref _emptyToNonEmptyListTransitionCount) == initialEmptyToNonEmptyCounts)
Expand Down Expand Up @@ -248,7 +253,7 @@ private bool TryStealFromTo(WorkStealingQueue? startInclusive, WorkStealingQueue
}
}

result = default(T)!;
result = default(T);
return false;
}

Expand Down Expand Up @@ -870,7 +875,7 @@ internal bool TryLocalPop([MaybeNullWhen(false)] out T result)
int tail = _tailIndex;
if (_headIndex - tail >= 0)
{
result = default(T)!;
result = default(T);
return false;
}

Expand Down Expand Up @@ -914,7 +919,7 @@ internal bool TryLocalPop([MaybeNullWhen(false)] out T result)
{
// We encountered a race condition and the element was stolen, restore the tail.
_tailIndex = tail + 1;
result = default(T)!;
result = default(T);
return false;
}
}
Expand Down Expand Up @@ -958,7 +963,7 @@ internal bool TryLocalPeek([MaybeNullWhen(false)] out T result)
}
}

result = default(T)!;
result = default(T);
return false;
}

Expand Down Expand Up @@ -1015,7 +1020,7 @@ internal bool TrySteal([MaybeNullWhen(false)] out T result, bool take)
}

// The queue was empty.
result = default(T)!;
result = default(T);
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ private static void CopyRemovedItems(Node head, T[] collection, int startIndex,
/// <remarks>For <see cref="ConcurrentStack{T}"/>, this operation will attempt to pope the object at
/// the top of the <see cref="ConcurrentStack{T}"/>.
/// </remarks>
bool IProducerConsumerCollection<T>.TryTake(out T item)
bool IProducerConsumerCollection<T>.TryTake([MaybeNullWhen(false)] out T item)
{
return TryPop(out item);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,22 @@ public static void Test21_CopyToExceptions()
});
}

[Fact]
public static void Test_WithNullEntries()
{
BlockingCollection<string> collection = new BlockingCollection<string>()
{
"hello",
null,
"goodbye"
};

Assert.Equal("hello", collection.Take());
Assert.Null(collection.Take());
Assert.Equal("goodbye", collection.Take());
Assert.False(collection.TryTake(out _));
}

[Fact]
public static void Test_LargeSize()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ public void Sort(int index, int count, System.Collections.Generic.IComparer<T>?
System.Collections.Generic.IEnumerator<T> System.Collections.Generic.IEnumerable<T>.GetEnumerator() { throw null; }
void System.Collections.ICollection.CopyTo(System.Array array, int arrayIndex) { }
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; }
int System.Collections.IList.Add(object value) { throw null; }
int System.Collections.IList.Add(object? value) { throw null; }
void System.Collections.IList.Clear() { }
bool System.Collections.IList.Contains(object? value) { throw null; }
int System.Collections.IList.IndexOf(object? value) { throw null; }
Expand Down Expand Up @@ -819,7 +819,7 @@ void System.Collections.Generic.IDictionary<TKey,TValue>.Add(TKey key, TValue va
bool System.Collections.Generic.IDictionary<TKey,TValue>.Remove(TKey key) { throw null; }
System.Collections.Generic.IEnumerator<System.Collections.Generic.KeyValuePair<TKey, TValue>> System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<TKey,TValue>>.GetEnumerator() { throw null; }
void System.Collections.ICollection.CopyTo(System.Array array, int index) { }
void System.Collections.IDictionary.Add(object key, object value) { }
void System.Collections.IDictionary.Add(object key, object? value) { }
void System.Collections.IDictionary.Clear() { }
bool System.Collections.IDictionary.Contains(object key) { throw null; }
System.Collections.IDictionaryEnumerator System.Collections.IDictionary.GetEnumerator() { throw null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ protected virtual void OnDeserialization(object? sender) { }
public void Remove(object key) { }
public void RemoveAt(int index) { }
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; }
void System.Runtime.Serialization.IDeserializationCallback.OnDeserialization(object sender) { }
void System.Runtime.Serialization.IDeserializationCallback.OnDeserialization(object? sender) { }
}
public partial class StringCollection : System.Collections.ICollection, System.Collections.IEnumerable, System.Collections.IList
{
Expand Down
26 changes: 13 additions & 13 deletions src/libraries/System.Collections/ref/System.Collections.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ protected Comparer() { }
public static System.Collections.Generic.Comparer<T> Default { get { throw null; } }
public abstract int Compare([System.Diagnostics.CodeAnalysis.AllowNullAttribute] T x, [System.Diagnostics.CodeAnalysis.AllowNullAttribute] T y);
public static System.Collections.Generic.Comparer<T> Create(System.Comparison<T> comparison) { throw null; }
int System.Collections.IComparer.Compare(object x, object y) { throw null; }
int System.Collections.IComparer.Compare(object? x, object? y) { throw null; }
}
public partial class Dictionary<TKey, TValue> : System.Collections.Generic.ICollection<System.Collections.Generic.KeyValuePair<TKey, TValue>>, System.Collections.Generic.IDictionary<TKey, TValue>, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<TKey, TValue>>, System.Collections.Generic.IReadOnlyCollection<System.Collections.Generic.KeyValuePair<TKey, TValue>>, System.Collections.Generic.IReadOnlyDictionary<TKey, TValue>, System.Collections.ICollection, System.Collections.IDictionary, System.Collections.IEnumerable, System.Runtime.Serialization.IDeserializationCallback, System.Runtime.Serialization.ISerializable where TKey : notnull
{
Expand Down Expand Up @@ -102,7 +102,7 @@ public virtual void OnDeserialization(object? sender) { }
bool System.Collections.Generic.ICollection<System.Collections.Generic.KeyValuePair<TKey, TValue>>.Remove(System.Collections.Generic.KeyValuePair<TKey, TValue> keyValuePair) { throw null; }
System.Collections.Generic.IEnumerator<System.Collections.Generic.KeyValuePair<TKey, TValue>> System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<TKey, TValue>>.GetEnumerator() { throw null; }
void System.Collections.ICollection.CopyTo(System.Array array, int index) { }
void System.Collections.IDictionary.Add(object key, object value) { }
void System.Collections.IDictionary.Add(object key, object? value) { }
bool System.Collections.IDictionary.Contains(object key) { throw null; }
System.Collections.IDictionaryEnumerator System.Collections.IDictionary.GetEnumerator() { throw null; }
void System.Collections.IDictionary.Remove(object key) { }
Expand Down Expand Up @@ -187,7 +187,7 @@ protected EqualityComparer() { }
public static System.Collections.Generic.EqualityComparer<T> Default { get { throw null; } }
public abstract bool Equals([System.Diagnostics.CodeAnalysis.AllowNullAttribute] T x, [System.Diagnostics.CodeAnalysis.AllowNullAttribute] T y);
public abstract int GetHashCode([System.Diagnostics.CodeAnalysis.DisallowNullAttribute] T obj);
bool System.Collections.IEqualityComparer.Equals(object x, object y) { throw null; }
bool System.Collections.IEqualityComparer.Equals(object? x, object? y) { throw null; }
int System.Collections.IEqualityComparer.GetHashCode(object obj) { throw null; }
}
public partial class HashSet<T> : System.Collections.Generic.ICollection<T>, System.Collections.Generic.IEnumerable<T>, System.Collections.Generic.IReadOnlyCollection<T>, System.Collections.Generic.ISet<T>, System.Collections.Generic.IReadOnlySet<T>, System.Collections.IEnumerable, System.Runtime.Serialization.IDeserializationCallback, System.Runtime.Serialization.ISerializable
Expand Down Expand Up @@ -295,7 +295,7 @@ public partial struct Enumerator : System.Collections.Generic.IEnumerator<T>, Sy
public void Dispose() { }
public bool MoveNext() { throw null; }
void System.Collections.IEnumerator.Reset() { }
void System.Runtime.Serialization.IDeserializationCallback.OnDeserialization(object sender) { }
void System.Runtime.Serialization.IDeserializationCallback.OnDeserialization(object? sender) { }
void System.Runtime.Serialization.ISerializable.GetObjectData(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { }
}
}
Expand Down Expand Up @@ -361,11 +361,11 @@ public void Sort(int index, int count, System.Collections.Generic.IComparer<T>?
System.Collections.Generic.IEnumerator<T> System.Collections.Generic.IEnumerable<T>.GetEnumerator() { throw null; }
void System.Collections.ICollection.CopyTo(System.Array array, int arrayIndex) { }
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; }
int System.Collections.IList.Add(object item) { throw null; }
bool System.Collections.IList.Contains(object item) { throw null; }
int System.Collections.IList.IndexOf(object item) { throw null; }
void System.Collections.IList.Insert(int index, object item) { }
void System.Collections.IList.Remove(object item) { }
int System.Collections.IList.Add(object? item) { throw null; }
bool System.Collections.IList.Contains(object? item) { throw null; }
int System.Collections.IList.IndexOf(object? item) { throw null; }
void System.Collections.IList.Insert(int index, object? item) { }
void System.Collections.IList.Remove(object? item) { }
public T[] ToArray() { throw null; }
public void TrimExcess() { }
public bool TrueForAll(System.Predicate<T> match) { throw null; }
Expand Down Expand Up @@ -457,7 +457,7 @@ public void CopyTo(System.Collections.Generic.KeyValuePair<TKey, TValue>[] array
bool System.Collections.Generic.ICollection<System.Collections.Generic.KeyValuePair<TKey, TValue>>.Remove(System.Collections.Generic.KeyValuePair<TKey, TValue> keyValuePair) { throw null; }
System.Collections.Generic.IEnumerator<System.Collections.Generic.KeyValuePair<TKey, TValue>> System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<TKey, TValue>>.GetEnumerator() { throw null; }
void System.Collections.ICollection.CopyTo(System.Array array, int index) { }
void System.Collections.IDictionary.Add(object key, object value) { }
void System.Collections.IDictionary.Add(object key, object? value) { }
bool System.Collections.IDictionary.Contains(object key) { throw null; }
System.Collections.IDictionaryEnumerator System.Collections.IDictionary.GetEnumerator() { throw null; }
void System.Collections.IDictionary.Remove(object key) { }
Expand Down Expand Up @@ -572,7 +572,7 @@ public void RemoveAt(int index) { }
bool System.Collections.Generic.ICollection<System.Collections.Generic.KeyValuePair<TKey, TValue>>.Remove(System.Collections.Generic.KeyValuePair<TKey, TValue> keyValuePair) { throw null; }
System.Collections.Generic.IEnumerator<System.Collections.Generic.KeyValuePair<TKey, TValue>> System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<TKey, TValue>>.GetEnumerator() { throw null; }
void System.Collections.ICollection.CopyTo(System.Array array, int arrayIndex) { }
void System.Collections.IDictionary.Add(object key, object value) { }
void System.Collections.IDictionary.Add(object key, object? value) { }
bool System.Collections.IDictionary.Contains(object key) { throw null; }
System.Collections.IDictionaryEnumerator System.Collections.IDictionary.GetEnumerator() { throw null; }
void System.Collections.IDictionary.Remove(object key) { }
Expand Down Expand Up @@ -624,7 +624,7 @@ void System.Collections.Generic.ICollection<T>.Add(T item) { }
System.Collections.Generic.IEnumerator<T> System.Collections.Generic.IEnumerable<T>.GetEnumerator() { throw null; }
void System.Collections.ICollection.CopyTo(System.Array array, int index) { }
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; }
void System.Runtime.Serialization.IDeserializationCallback.OnDeserialization(object sender) { }
void System.Runtime.Serialization.IDeserializationCallback.OnDeserialization(object? sender) { }
void System.Runtime.Serialization.ISerializable.GetObjectData(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { }
public bool TryGetValue(T equalValue, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out T actualValue) { throw null; }
public void UnionWith(System.Collections.Generic.IEnumerable<T> other) { }
Expand All @@ -637,7 +637,7 @@ public partial struct Enumerator : System.Collections.Generic.IEnumerator<T>, Sy
public void Dispose() { }
public bool MoveNext() { throw null; }
void System.Collections.IEnumerator.Reset() { }
void System.Runtime.Serialization.IDeserializationCallback.OnDeserialization(object sender) { }
void System.Runtime.Serialization.IDeserializationCallback.OnDeserialization(object? sender) { }
void System.Runtime.Serialization.ISerializable.GetObjectData(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { }
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,10 +263,10 @@ public void Remove(System.Diagnostics.TraceListener? listener) { }
public void Remove(string name) { }
public void RemoveAt(int index) { }
void System.Collections.ICollection.CopyTo(System.Array array, int index) { }
int System.Collections.IList.Add(object value) { throw null; }
int System.Collections.IList.Add(object? value) { throw null; }
bool System.Collections.IList.Contains(object? value) { throw null; }
int System.Collections.IList.IndexOf(object? value) { throw null; }
void System.Collections.IList.Insert(int index, object value) { }
void System.Collections.IList.Insert(int index, object? value) { }
void System.Collections.IList.Remove(object? value) { }
}
[System.FlagsAttribute]
Expand Down
Loading