Use T? for unconstrained nullable types (#40197)

* Temporarily upgrade compiler version for T? support

* Use T? for unconstrained nullable types

docs/coding-guidelines/api-guidelines/nullability.md

@@ -99,6 +99,7 @@ The C# compiler respects a set of attributes that impact its flow analysis.  We
 - **DO** add `[NotNullIfNotNull(string)]` if nullable ref argument will be non-`null` upon exit, when an other argument passed evaluated to non-`null`, pass that argument name as string. Example: `public void Exchange([NotNullIfNotNull("value")] ref object? location, object? value);`.
 - **DO** add `[return: NotNullIfNotNull(string)]` if a method would not return `null` in case an argument passed evaluated to non-`null`, pass that argument name as string. Example: `[return: NotNullIfNotNull("name")] public string? FormatName(string? name);`
 - **DO** add `[MemberNotNull(string fieldName)]` to a helper method which initializes member field(s), passing in the field name. Example: `[MemberNotNull("_buffer")] private void InitializeBuffer()`.  This will help to avoid spurious warnings at call sites that call the initialization method and then proceed to use the specified field.  Note that there are two constructors to `MemberNotNull`; one that takes a single `string`, and one that takes a `params string[]`.  When the number of fields initialized is small (e.g. <= 3), it's preferable to use multiple `[MemberNotNull(string)]` attributes on the method rather than one `[MemberNotNull(string, string, string, ...)]` attribute, as the latter is not CLS compliant and will likely require `#pragma warning disable` and `#pragma warning restore` around the line to suppress warnings.
+- **AVOID** using `[MaybeNull]`, not because it's problematic, but because there's almost always a better option, such as `T?` (as of this writing, in all of the dotnet/runtime there are only 7 occurrences of `[MaybeNull]`).  One example of where it's applicable is `AsyncLocal<T>.Value`; `[DisallowNull]` can't be used here, because `null` is valid if `T` is nullable, and `T?` shouldn't be used because `Value` shouldn't be set to `null` if `T` isn't nullable.  Another is in the relatively rare case where a public or protected field is exposed, may begin life as null, but shouldn't be explicitly set to null.
 
 ## Code Review Guidance
 

eng/Versions.props

@@ -16,6 +16,8 @@
     <DotNetFinalVersionKind Condition="'$(StabilizePackageVersion)' == 'true'">release</DotNetFinalVersionKind>
     <!-- Opt-in/out repo features -->
     <UsingToolMicrosoftNetCompilers>true</UsingToolMicrosoftNetCompilers>
+    <!-- TODO: Upgrade compiler version to enable T?; remove this once the employed SDK uses a new enough version. -->
+    <MicrosoftNetCompilersToolsetVersion>3.8.0-2.20379.3</MicrosoftNetCompilersToolsetVersion>
     <UsingToolIbcOptimization>true</UsingToolIbcOptimization>
     <UsingToolXliff>false</UsingToolXliff>
     <UsingToolNetFrameworkReferenceAssemblies>true</UsingToolNetFrameworkReferenceAssemblies>

src/coreclr/src/System.Private.CoreLib/src/System/GC.cs

@@ -659,7 +659,7 @@ internal static void UnregisterMemoryLoadChangeNotification(Action notification)
         /// If pinned is set to true, <typeparamref name="T"/> must not be a reference type or a type that contains object references.
         /// </remarks>
         [MethodImpl(MethodImplOptions.AggressiveInlining)] // forced to ensure no perf drop for small memory buffers (hot path)
-        public static T[] AllocateUninitializedArray<T>(int length, bool pinned = false)
+        public static T[] AllocateUninitializedArray<T>(int length, bool pinned = false) // T[] rather than T?[] to match `new T[length]` behavior
         {
             if (!pinned)
             {
@@ -705,7 +705,7 @@ static T[] AllocateNewUninitializedArray(int length, bool pinned)
         /// <remarks>
         /// If pinned is set to true, <typeparamref name="T"/> must not be a reference type or a type that contains object references.
         /// </remarks>
-        public static T[] AllocateArray<T>(int length, bool pinned = false)
+        public static T[] AllocateArray<T>(int length, bool pinned = false) // T[] rather than T?[] to match `new T[length]` behavior
         {
             GC_ALLOC_FLAGS flags = GC_ALLOC_FLAGS.GC_ALLOC_NO_FLAGS;
 

src/coreclr/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs

@@ -706,7 +706,7 @@ public static bool SetComObjectData(object obj, object key, object? data)
         }
 
         [SupportedOSPlatform("windows")]
-        public static TWrapper CreateWrapperOfType<T, TWrapper>([AllowNull] T o)
+        public static TWrapper CreateWrapperOfType<T, TWrapper>(T? o)
         {
             return (TWrapper)CreateWrapperOfType(o, typeof(TWrapper))!;
         }
@@ -756,7 +756,7 @@ public static unsafe int Release(IntPtr pUnk)
         public static extern void GetNativeVariantForObject(object? obj, /* VARIANT * */ IntPtr pDstNativeVariant);
 
         [SupportedOSPlatform("windows")]
-        public static void GetNativeVariantForObject<T>([AllowNull] T obj, IntPtr pDstNativeVariant)
+        public static void GetNativeVariantForObject<T>(T? obj, IntPtr pDstNativeVariant)
         {
             GetNativeVariantForObject((object?)obj, pDstNativeVariant);
         }
@@ -766,10 +766,9 @@ public static void GetNativeVariantForObject<T>([AllowNull] T obj, IntPtr pDstNa
         public static extern object? GetObjectForNativeVariant(/* VARIANT * */ IntPtr pSrcNativeVariant);
 
         [SupportedOSPlatform("windows")]
-        [return: MaybeNull]
-        public static T GetObjectForNativeVariant<T>(IntPtr pSrcNativeVariant)
+        public static T? GetObjectForNativeVariant<T>(IntPtr pSrcNativeVariant)
         {
-            return (T)GetObjectForNativeVariant(pSrcNativeVariant)!;
+            return (T?)GetObjectForNativeVariant(pSrcNativeVariant);
         }
 
         [SupportedOSPlatform("windows")]

src/libraries/Common/src/System/Data/ProviderBase/DbReferenceCollection.cs

@@ -137,8 +137,7 @@ protected void AddItem(object value, int tag)
             }
         }
 
-        [return: MaybeNull]
-        internal T FindItem<T>(int tag, Func<T, bool> filterMethod) where T : class
+        internal T? FindItem<T>(int tag, Func<T, bool> filterMethod) where T : class
         {
             bool lockObtained = false;
             try

src/libraries/Common/src/System/Net/Http/aspnetcore/Quic/Implementations/MsQuic/MsQuicStream.cs

@@ -398,7 +398,7 @@ internal override Task FlushAsync(CancellationToken cancellationToken = default)
         {
             ThrowIfDisposed();
 
-            return default!;
+            return Task.CompletedTask;
         }
 
         public override ValueTask DisposeAsync()

src/libraries/Common/src/System/Threading/Tasks/RendezvousAwaitable.cs

@@ -24,7 +24,7 @@ internal class RendezvousAwaitable<TResult> : ICriticalNotifyCompletion
         /// <summary>The exception representing the failed async operation, if it failed.</summary>
         private ExceptionDispatchInfo? _error;
         /// <summary>The result of the async operation, if it succeeded.</summary>
-        [AllowNull] private TResult _result = default;
+        private TResult? _result;
 #if DEBUG
         private bool _resultSet;
 #endif
@@ -64,7 +64,7 @@ public TResult GetResult()
             }
 
             // The operation completed successfully.  Clear and return the result.
-            TResult result = _result;
+            TResult result = _result!;
             _result = default(TResult);
 #if DEBUG
             _resultSet = false;

src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/ref/Microsoft.Extensions.DependencyInjection.Abstractions.cs

@@ -111,8 +111,7 @@ public static partial class ServiceProviderServiceExtensions
         public static T GetRequiredService<T>(this System.IServiceProvider provider) where T : notnull { throw null; }
         public static System.Collections.Generic.IEnumerable<object?> GetServices(this System.IServiceProvider provider, System.Type serviceType) { throw null; }
         public static System.Collections.Generic.IEnumerable<T> GetServices<T>(this System.IServiceProvider provider) { throw null; }
-        [return: System.Diagnostics.CodeAnalysis.MaybeNullAttribute]
-        public static T GetService<T>(this System.IServiceProvider provider) { throw null; }
+        public static T? GetService<T>(this System.IServiceProvider provider) { throw null; }
     }
 }
 namespace Microsoft.Extensions.DependencyInjection.Extensions

src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ServiceProviderServiceExtensions.cs

@@ -18,15 +18,14 @@ public static class ServiceProviderServiceExtensions
         /// <typeparam name="T">The type of service object to get.</typeparam>
         /// <param name="provider">The <see cref="IServiceProvider"/> to retrieve the service object from.</param>
         /// <returns>A service object of type <typeparamref name="T"/> or null if there is no such service.</returns>
-        [return: MaybeNull]
-        public static T GetService<T>(this IServiceProvider provider)
+        public static T? GetService<T>(this IServiceProvider provider)
         {
             if (provider == null)
             {
                 throw new ArgumentNullException(nameof(provider));
             }
 
-            return (T)provider.GetService(typeof(T));
+            return (T?)provider.GetService(typeof(T));
         }
 
         /// <summary>

src/libraries/Microsoft.VisualBasic.Core/ref/Microsoft.VisualBasic.Core.cs

@@ -771,9 +771,8 @@ internal Conversions() { }
         public static decimal ToDecimal(string? Value) { throw null; }
         public static double ToDouble(object? Value) { throw null; }
         public static double ToDouble(string? Value) { throw null; }
-        [return: System.Diagnostics.CodeAnalysis.MaybeNullAttribute]
         [return: System.Diagnostics.CodeAnalysis.NotNullIfNotNull("Value")]
-        public static T ToGenericParameter<T>(object? Value) { throw null; }
+        public static T? ToGenericParameter<T>(object? Value) { throw null; }
         public static int ToInteger(object? Value) { throw null; }
         public static int ToInteger(string? Value) { throw null; }
         public static long ToLong(object? Value) { throw null; }

src/libraries/System.Collections.Concurrent/ref/System.Collections.Concurrent.cs

@@ -33,8 +33,8 @@ void System.Collections.ICollection.CopyTo(System.Array array, int index) { }
         System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; }
         public T Take() { throw null; }
         public T Take(System.Threading.CancellationToken cancellationToken) { throw null; }
-        public static int TakeFromAny(System.Collections.Concurrent.BlockingCollection<T>[] collections, [System.Diagnostics.CodeAnalysis.MaybeNullAttribute] out T item) { throw null; }
-        public static int TakeFromAny(System.Collections.Concurrent.BlockingCollection<T>[] collections, [System.Diagnostics.CodeAnalysis.MaybeNullAttribute] out T item, System.Threading.CancellationToken cancellationToken) { throw null; }
+        public static int TakeFromAny(System.Collections.Concurrent.BlockingCollection<T>[] collections, out T? item) { throw null; }
+        public static int TakeFromAny(System.Collections.Concurrent.BlockingCollection<T>[] collections, out T? item, System.Threading.CancellationToken cancellationToken) { throw null; }
         public T[] ToArray() { throw null; }
         public bool TryAdd(T item) { throw null; }
         public bool TryAdd(T item, int millisecondsTimeout) { throw null; }
@@ -48,10 +48,10 @@ void System.Collections.ICollection.CopyTo(System.Array array, int index) { }
         public bool TryTake([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out T item, int millisecondsTimeout) { throw null; }
         public bool TryTake([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out T item, int millisecondsTimeout, System.Threading.CancellationToken cancellationToken) { throw null; }
         public bool TryTake([System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out T item, System.TimeSpan timeout) { throw null; }
-        public static int TryTakeFromAny(System.Collections.Concurrent.BlockingCollection<T>[] collections, [System.Diagnostics.CodeAnalysis.MaybeNullAttribute] out T item) { throw null; }
-        public static int TryTakeFromAny(System.Collections.Concurrent.BlockingCollection<T>[] collections, [System.Diagnostics.CodeAnalysis.MaybeNullAttribute] out T item, int millisecondsTimeout) { throw null; }
-        public static int TryTakeFromAny(System.Collections.Concurrent.BlockingCollection<T>[] collections, [System.Diagnostics.CodeAnalysis.MaybeNullAttribute] out T item, int millisecondsTimeout, System.Threading.CancellationToken cancellationToken) { throw null; }
-        public static int TryTakeFromAny(System.Collections.Concurrent.BlockingCollection<T>[] collections, [System.Diagnostics.CodeAnalysis.MaybeNullAttribute] out T item, System.TimeSpan timeout) { throw null; }
+        public static int TryTakeFromAny(System.Collections.Concurrent.BlockingCollection<T>[] collections, out T? item) { throw null; }
+        public static int TryTakeFromAny(System.Collections.Concurrent.BlockingCollection<T>[] collections, out T? item, int millisecondsTimeout) { throw null; }
+        public static int TryTakeFromAny(System.Collections.Concurrent.BlockingCollection<T>[] collections, out T? item, int millisecondsTimeout, System.Threading.CancellationToken cancellationToken) { throw null; }
+        public static int TryTakeFromAny(System.Collections.Concurrent.BlockingCollection<T>[] collections, out T? item, System.TimeSpan timeout) { throw null; }
     }
     public partial class ConcurrentBag<T> : System.Collections.Concurrent.IProducerConsumerCollection<T>, System.Collections.Generic.IEnumerable<T>, System.Collections.Generic.IReadOnlyCollection<T>, System.Collections.ICollection, System.Collections.IEnumerable
     {

src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/BlockingCollection.cs

@@ -1169,7 +1169,7 @@ private static int UpdateTimeOut(uint startTime, int originalWaitMillisecondsTim
         /// <exception cref="System.ArgumentOutOfRangeException">The count of <paramref name="collections"/> is greater than the maximum size of
         /// 62 for STA and 63 for MTA.</exception>
         /// <remarks>A call to TakeFromAny may block until an item is available to be removed.</remarks>
-        public static int TakeFromAny(BlockingCollection<T>[] collections, [MaybeNull] out T item)
+        public static int TakeFromAny(BlockingCollection<T>[] collections, out T? item)
         {
             return TakeFromAny(collections, out item, CancellationToken.None);
         }
@@ -1198,7 +1198,7 @@ public static int TakeFromAny(BlockingCollection<T>[] collections, [MaybeNull] o
         /// <exception cref="System.ArgumentOutOfRangeException">The count of <paramref name="collections"/> is greater than the maximum size of
         /// 62 for STA and 63 for MTA.</exception>
         /// <remarks>A call to TakeFromAny may block until an item is available to be removed.</remarks>
-        public static int TakeFromAny(BlockingCollection<T>[] collections, [MaybeNull] out T item, CancellationToken cancellationToken)
+        public static int TakeFromAny(BlockingCollection<T>[] collections, out T? item, CancellationToken cancellationToken)
         {
             int returnValue = TryTakeFromAnyCore(collections, out item, Timeout.Infinite, true, cancellationToken);
             Debug.Assert(returnValue >= 0 && returnValue < collections.Length,
@@ -1226,7 +1226,7 @@ public static int TakeFromAny(BlockingCollection<T>[] collections, [MaybeNull] o
         /// <exception cref="System.ArgumentOutOfRangeException">The count of <paramref name="collections"/> is greater than the maximum size of
         /// 62 for STA and 63 for MTA.</exception>
         /// <remarks>A call to TryTakeFromAny may block until an item is available to be removed.</remarks>
-        public static int TryTakeFromAny(BlockingCollection<T>[] collections, [MaybeNull] out T item)
+        public static int TryTakeFromAny(BlockingCollection<T>[] collections, out T? item)
         {
             return TryTakeFromAny(collections, out item, 0);
         }
@@ -1257,7 +1257,7 @@ public static int TryTakeFromAny(BlockingCollection<T>[] collections, [MaybeNull
         /// <exception cref="System.ArgumentOutOfRangeException">The count of <paramref name="collections"/> is greater than the maximum size of
         /// 62 for STA and 63 for MTA.</exception>
         /// <remarks>A call to TryTakeFromAny may block until an item is available to be removed.</remarks>
-        public static int TryTakeFromAny(BlockingCollection<T>[] collections, [MaybeNull] out T item, TimeSpan timeout)
+        public static int TryTakeFromAny(BlockingCollection<T>[] collections, out T? item, TimeSpan timeout)
         {
             ValidateTimeout(timeout);
             return TryTakeFromAnyCore(collections, out item, (int)timeout.TotalMilliseconds, false, CancellationToken.None);
@@ -1287,7 +1287,7 @@ public static int TryTakeFromAny(BlockingCollection<T>[] collections, [MaybeNull
         /// <exception cref="System.ArgumentOutOfRangeException">The count of <paramref name="collections"/> is greater than the maximum size of
         /// 62 for STA and 63 for MTA.</exception>
         /// <remarks>A call to TryTakeFromAny may block until an item is available to be removed.</remarks>
-        public static int TryTakeFromAny(BlockingCollection<T>[] collections, [MaybeNull] out T item, int millisecondsTimeout)
+        public static int TryTakeFromAny(BlockingCollection<T>[] collections, out T? item, int millisecondsTimeout)
         {
             ValidateMillisecondsTimeout(millisecondsTimeout);
             return TryTakeFromAnyCore(collections, out item, millisecondsTimeout, false, CancellationToken.None);
@@ -1321,7 +1321,7 @@ public static int TryTakeFromAny(BlockingCollection<T>[] collections, [MaybeNull
         /// <exception cref="System.ArgumentOutOfRangeException">The count of <paramref name="collections"/> is greater than the maximum size of
         /// 62 for STA and 63 for MTA.</exception>
         /// <remarks>A call to TryTakeFromAny may block until an item is available to be removed.</remarks>
-        public static int TryTakeFromAny(BlockingCollection<T>[] collections, [MaybeNull] out T item, int millisecondsTimeout, CancellationToken cancellationToken)
+        public static int TryTakeFromAny(BlockingCollection<T>[] collections, out T? item, int millisecondsTimeout, CancellationToken cancellationToken)
         {
             ValidateMillisecondsTimeout(millisecondsTimeout);
             return TryTakeFromAnyCore(collections, out item, millisecondsTimeout, false, cancellationToken);
@@ -1344,7 +1344,7 @@ public static int TryTakeFromAny(BlockingCollection<T>[] collections, [MaybeNull
         /// <exception cref="System.ArgumentException">If the collections argument is a 0-length array or contains a
         /// null element. Also, if at least one of the collections has been marked complete for adds.</exception>
         /// <exception cref="System.ObjectDisposedException">If at least one of the collections has been disposed.</exception>
-        private static int TryTakeFromAnyCore(BlockingCollection<T>[] collections, [MaybeNull] out T item, int millisecondsTimeout, bool isTakeOperation, CancellationToken externalCancellationToken)
+        private static int TryTakeFromAnyCore(BlockingCollection<T>[] collections, out T? item, int millisecondsTimeout, bool isTakeOperation, CancellationToken externalCancellationToken)
         {
             ValidateCollectionsArray(collections, false);
 
@@ -1378,7 +1378,7 @@ private static int TryTakeFromAnyCore(BlockingCollection<T>[] collections, [Mayb
         /// <exception cref="System.ArgumentException">If the collections argument is a 0-length array or contains a
         /// null element. Also, if at least one of the collections has been marked complete for adds.</exception>
         /// <exception cref="System.ObjectDisposedException">If at least one of the collections has been disposed.</exception>
-        private static int TryTakeFromAnyCoreSlow(BlockingCollection<T>[] collections, [MaybeNull] out T item, int millisecondsTimeout, bool isTakeOperation, CancellationToken externalCancellationToken)
+        private static int TryTakeFromAnyCoreSlow(BlockingCollection<T>[] collections, out T? item, int millisecondsTimeout, bool isTakeOperation, CancellationToken externalCancellationToken)
         {
             const int OPERATION_FAILED = -1;
 

src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs

@@ -1089,7 +1089,7 @@ internal enum Operation
         private sealed class Enumerator : IEnumerator<T>
         {
             private readonly T[] _array;
-            [AllowNull] private T _current = default;
+            private T? _current;
             private int _index;
 
             public Enumerator(T[] array)
@@ -1110,7 +1110,7 @@ public bool MoveNext()
                 return false;
             }
 
-            public T Current => _current;
+            public T Current => _current!;
 
             object? IEnumerator.Current
             {