Address nullable feedback for S.T.Overlapped, S.Memory, S.R.InteropServices (#25214)
authorSantiago Fernandez Madero <safern@microsoft.com>
Wed, 19 Jun 2019 21:33:29 +0000 (14:33 -0700)
committerGitHub <noreply@github.com>
Wed, 19 Jun 2019 21:33:29 +0000 (14:33 -0700)
* Address nullable feedback for System.Threading.Overlapped

* Address nullable feedback for System.Memory

* Address nullable for System.Runtime.InteropServices

* Update xml comments and make CultureInfo nullable to match String.ToLower/Upper feedback

* PR feedback: revert accepting null comparer in BinarySearch

* Disable test that needs to be updated in corefx

src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs
src/System.Private.CoreLib/shared/System/Runtime/InteropServices/BStrWrapper.cs
src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs
src/System.Private.CoreLib/src/System/Threading/Overlapped.cs
src/System.Private.CoreLib/src/System/Variant.cs
tests/CoreFX/CoreFX.issues.rsp

index 6de285d..33617b4 100644 (file)
@@ -220,15 +220,12 @@ namespace System
         /// <param name="destination">The destination span which contains the transformed characters.</param>
         /// <param name="culture">An object that supplies culture-specific casing rules.</param>
         /// <remarks>If the source and destinations overlap, this method behaves as if the original values are in
-        /// a temporary location before the destination is overwritten.</remarks>
+        /// a temporary location before the destination is overwritten. If <paramref name="culture"/> is null, <see cref="System.Globalization.CultureInfo.CurrentCulture"/> will be used.</remarks>
         /// <returns>The number of characters written into the destination span. If the destination is too small, returns -1.</returns>
-        /// <exception cref="System.ArgumentNullException">
-        /// Thrown when <paramref name="culture"/> is null.
-        /// </exception>
-        public static int ToLower(this ReadOnlySpan<char> source, Span<char> destination, CultureInfo culture)
+        public static int ToLower(this ReadOnlySpan<char> source, Span<char> destination, CultureInfo? culture)
         {
             if (culture == null)
-                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.culture);
+                culture = CultureInfo.CurrentCulture;
 
             // Assuming that changing case does not affect length
             if (destination.Length < source.Length)
@@ -237,7 +234,7 @@ namespace System
             if (GlobalizationMode.Invariant)
                 TextInfo.ToLowerAsciiInvariant(source, destination);
             else
-                culture!.TextInfo.ChangeCaseToLower(source, destination); // TODO-NULLABLE: Remove ! when [DoesNotReturn] respected
+                culture.TextInfo.ChangeCaseToLower(source, destination);
             return source.Length;
         }
 
@@ -271,15 +268,12 @@ namespace System
         /// <param name="destination">The destination span which contains the transformed characters.</param>
         /// <param name="culture">An object that supplies culture-specific casing rules.</param>
         /// <remarks>If the source and destinations overlap, this method behaves as if the original values are in
-        /// a temporary location before the destination is overwritten.</remarks>
+        /// a temporary location before the destination is overwritten. If <paramref name="culture"/> is null, <see cref="System.Globalization.CultureInfo.CurrentCulture"/> will be used.</remarks>
         /// <returns>The number of characters written into the destination span. If the destination is too small, returns -1.</returns>
-        /// <exception cref="System.ArgumentNullException">
-        /// Thrown when <paramref name="culture"/> is null.
-        /// </exception>
-        public static int ToUpper(this ReadOnlySpan<char> source, Span<char> destination, CultureInfo culture)
+        public static int ToUpper(this ReadOnlySpan<char> source, Span<char> destination, CultureInfo? culture)
         {
             if (culture == null)
-                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.culture);
+                culture = CultureInfo.CurrentCulture;
 
             // Assuming that changing case does not affect length
             if (destination.Length < source.Length)
@@ -288,7 +282,7 @@ namespace System
             if (GlobalizationMode.Invariant)
                 TextInfo.ToUpperAsciiInvariant(source, destination);
             else
-                culture!.TextInfo.ChangeCaseToUpper(source, destination); // TODO-NULLABLE: Remove ! when [DoesNotReturn] respected
+                culture.TextInfo.ChangeCaseToUpper(source, destination);
             return source.Length;
         }
 
index f6eee34..9329b05 100644 (file)
@@ -7,16 +7,16 @@ namespace System.Runtime.InteropServices
     // Wrapper that is converted to a variant with VT_BSTR.
     public sealed class BStrWrapper
     {
-        public BStrWrapper(string value)
+        public BStrWrapper(string? value)
         {
             WrappedObject = value;
         }
 
-        public BStrWrapper(object value)
+        public BStrWrapper(object? value)
         {
-            WrappedObject = (string)value;
+            WrappedObject = (string?)value;
         }
 
-        public string WrappedObject { get; }
+        public string? WrappedObject { get; }
     }
 }
index f92000f..edaa9d7 100644 (file)
@@ -687,7 +687,7 @@ namespace System.Runtime.InteropServices
         {
             object?[] objects = GetObjectsForNativeVariants(aSrcNativeVariant, cVars);
 
-            T[]? result = new T[objects.Length];
+            T[] result = new T[objects.Length];
             Array.Copy(objects, 0, result, 0, objects.Length);
 
             return result;
index e05611e..1f49852 100644 (file)
@@ -164,7 +164,7 @@ namespace System.Threading
             _overlappedData = new OverlappedData(this);
         }
 
-        public Overlapped(int offsetLo, int offsetHi, IntPtr hEvent, IAsyncResult ar) : this()
+        public Overlapped(int offsetLo, int offsetHi, IntPtr hEvent, IAsyncResult? ar) : this()
         {
             Debug.Assert(_overlappedData != null, "Initialized in delegated ctor");
             _overlappedData.OffsetLow = offsetLo;
@@ -174,7 +174,7 @@ namespace System.Threading
         }
 
         [Obsolete("This constructor is not 64-bit compatible.  Use the constructor that takes an IntPtr for the event handle.  http://go.microsoft.com/fwlink/?linkid=14202")]
-        public Overlapped(int offsetLo, int offsetHi, int hEvent, IAsyncResult ar) : this(offsetLo, offsetHi, new IntPtr(hEvent), ar)
+        public Overlapped(int offsetLo, int offsetHi, int hEvent, IAsyncResult? ar) : this(offsetLo, offsetHi, new IntPtr(hEvent), ar)
         {
         }
 
index 0e8cc7d..334bdc8 100644 (file)
@@ -303,7 +303,7 @@ namespace System
             else if (obj is BStrWrapper)
             {
                 vt = VarEnum.VT_BSTR;
-                obj = (object)(((BStrWrapper)obj).WrappedObject);
+                obj = (object?)(((BStrWrapper)obj).WrappedObject);
             }
 
             if (obj != null)
index 40c0fd4..98af810 100644 (file)
 -nomethod System.SpanTests.SpanTests.ZeroLengthLastIndexOfAny_ManyByte
 -nomethod System.SpanTests.SpanTests.ZeroLengthLastIndexOfAny_String_ManyByte
 
+# requires corefx test updates: https://github.com/dotnet/corefx/pull/38692
+-nomethod System.Tests.StringTests.CasingAsSpan_NullCulture_ThrowsArgumentNullException