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.

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

React to latest Roslyn nullability changes #36104

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 2 additions & 1 deletion eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
<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>
<!-- <MicrosoftNetCompilersToolsetVersion>3.6.0-2.20166.2</MicrosoftNetCompilersToolsetVersion> -->
GrabYourPitchforks marked this conversation as resolved.
Show resolved Hide resolved
<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,16 @@ private bool TryTakeWithNoTimeValidation([MaybeNullWhen(false)] out T item, int
}
}
}

if (waitForSemaphoreWasSuccessful)
{
Debug.Assert(item != null);
Copy link
Member Author

@GrabYourPitchforks GrabYourPitchforks May 8, 2020

Choose a reason for hiding this comment

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

Open question: I'm not sure if this assertion is actually correct. Is it possible to have - say - BlockingCollection<string?>? If that's legal, we should remove this assert.

Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to have - say - BlockingCollection<string?>?

Yes, that's valid. Please revert the added code. It's a little disheartening this didn't cause any existing tests to fail. Sigh.

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'll remove the assertion and add an explicit unit test.

}

#pragma warning disable CS8762
// Compiler can't automatically deduce 'item' has a non-null value when returning false.
GrabYourPitchforks marked this conversation as resolved.
Show resolved Hide resolved
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,11 @@ 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
// Compiler can't automatically deduce that 'result' is set to a valid value
// (which may be null if T allows it) at this exit point.
return true;
#pragma warning restore CS8762
GrabYourPitchforks marked this conversation as resolved.
Show resolved Hide resolved
}

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

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

Expand Down Expand Up @@ -870,7 +874,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 +918,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 +962,7 @@ internal bool TryLocalPeek([MaybeNullWhen(false)] out T result)
}
}

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

Expand Down Expand Up @@ -1015,7 +1019,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 @@ -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