From 23918ce912fc051b4a2443977e6973e34988c4f4 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Wed, 19 Jun 2019 14:33:29 -0700 Subject: [PATCH] Address nullable feedback for S.T.Overlapped, S.Memory, S.R.InteropServices (#25214) * 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 --- .../shared/System/MemoryExtensions.Fast.cs | 22 ++++++++-------------- .../System/Runtime/InteropServices/BStrWrapper.cs | 8 ++++---- .../Runtime/InteropServices/Marshal.CoreCLR.cs | 2 +- .../src/System/Threading/Overlapped.cs | 4 ++-- src/System.Private.CoreLib/src/System/Variant.cs | 2 +- tests/CoreFX/CoreFX.issues.rsp | 2 ++ 6 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs b/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs index 6de285d..33617b4 100644 --- a/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs +++ b/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs @@ -220,15 +220,12 @@ namespace System /// The destination span which contains the transformed characters. /// An object that supplies culture-specific casing rules. /// If the source and destinations overlap, this method behaves as if the original values are in - /// a temporary location before the destination is overwritten. + /// a temporary location before the destination is overwritten. If is null, will be used. /// The number of characters written into the destination span. If the destination is too small, returns -1. - /// - /// Thrown when is null. - /// - public static int ToLower(this ReadOnlySpan source, Span destination, CultureInfo culture) + public static int ToLower(this ReadOnlySpan source, Span 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 /// The destination span which contains the transformed characters. /// An object that supplies culture-specific casing rules. /// If the source and destinations overlap, this method behaves as if the original values are in - /// a temporary location before the destination is overwritten. + /// a temporary location before the destination is overwritten. If is null, will be used. /// The number of characters written into the destination span. If the destination is too small, returns -1. - /// - /// Thrown when is null. - /// - public static int ToUpper(this ReadOnlySpan source, Span destination, CultureInfo culture) + public static int ToUpper(this ReadOnlySpan source, Span 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; } diff --git a/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/BStrWrapper.cs b/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/BStrWrapper.cs index f6eee34..9329b05 100644 --- a/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/BStrWrapper.cs +++ b/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/BStrWrapper.cs @@ -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; } } } diff --git a/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs b/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs index f92000f..edaa9d7 100644 --- a/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs +++ b/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs @@ -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; diff --git a/src/System.Private.CoreLib/src/System/Threading/Overlapped.cs b/src/System.Private.CoreLib/src/System/Threading/Overlapped.cs index e05611e..1f49852 100644 --- a/src/System.Private.CoreLib/src/System/Threading/Overlapped.cs +++ b/src/System.Private.CoreLib/src/System/Threading/Overlapped.cs @@ -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) { } diff --git a/src/System.Private.CoreLib/src/System/Variant.cs b/src/System.Private.CoreLib/src/System/Variant.cs index 0e8cc7d..334bdc8 100644 --- a/src/System.Private.CoreLib/src/System/Variant.cs +++ b/src/System.Private.CoreLib/src/System/Variant.cs @@ -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) diff --git a/tests/CoreFX/CoreFX.issues.rsp b/tests/CoreFX/CoreFX.issues.rsp index 40c0fd4..98af810 100644 --- a/tests/CoreFX/CoreFX.issues.rsp +++ b/tests/CoreFX/CoreFX.issues.rsp @@ -112,3 +112,5 @@ -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 -- 2.7.4