Replace RuntimeHelpers.CreateSpan<T>(LdMemberToken) with new T[] { }
#3380
Conversation
| #if !NET40 && ROSLYN | ||
| public static ReadOnlySpan<byte> ReadOnlySpanInitializer_ByteArray() | ||
| { | ||
| return new byte[] { 1, 2, 3 }; | ||
| } | ||
|
|
||
| public static ReadOnlySpan<int> ReadOnlySpanInitializer_Int32Array() | ||
| { | ||
| return new int[] { 1, 2, 3 }; | ||
| } | ||
| #endif | ||
|
|
There was a problem hiding this comment.
Should I have put these in a new file and modified the options? It currently fails for Roslyn v1.
There was a problem hiding this comment.
Is there a reason why these tests are "correctness" tests and not "pretty" tests?
It seems like this is a relatively new language feature... you can simply use the CSx preprocessor symbol that matches the earliest version of C# where this feature is supported.
| @@ -143,6 +143,40 @@ internal static bool TransformSpanTArrayInitialization(NewObj inst, StatementTra | |||
| return false; | |||
| } | |||
|
|
|||
| internal static bool TransformRuntimeHelpersCreateSpanInitialization(Call inst, StatementTransformContext context, out ILInstruction replacement) | |||
There was a problem hiding this comment.
Not really, I am bad at naming things... 😅 ... luckily I had nine months to name my children...
| if (context.Settings.Utf8StringLiterals && | ||
| elementType.IsKnownType(KnownTypeCode.Byte) && | ||
| DecodeUTF8String(initialValue, size, out string text)) | ||
| { | ||
| replacement = new LdStrUtf8(text); | ||
| return true; | ||
| } | ||
| if (DecodeArrayInitializer(elementType, initialValue, new[] { size }, valuesList)) | ||
| { | ||
| var tempStore = context.Function.RegisterVariable(VariableKind.InitializerTarget, new ArrayType(context.TypeSystem, elementType)); | ||
| replacement = BlockFromInitializer(tempStore, elementType, new[] { size }, valuesList.ToArray()); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This is the same as for Span<T>(void*, int) handler. Should split this out into a shared function?
There was a problem hiding this comment.
You could do that, yes...
| field = default; | ||
| elementType = null; | ||
| IType type = inst.Method.DeclaringType; | ||
| if (type.Namespace != "System.Runtime.CompilerServices" || type.Name != "RuntimeHelpers" || type.TypeParameterCount != 0) |
There was a problem hiding this comment.
Is this the right way to check? Should I have created a "known" type?
There was a problem hiding this comment.
There already exists
Although that's missing the check for TypeParameterCount, but you could add it.
I don't think it's necessary to add a known type for this...
| if (inst.Arguments.Count != 1) | ||
| return false; | ||
| IMethod method = inst.Method; | ||
| if (method.Name != "CreateSpan" || method.TypeArguments.Count != 1) | ||
| return false; |
There was a problem hiding this comment.
Should I be checking the full method signature? I noticed that the other handler doesn't check argument types.
There was a problem hiding this comment.
checking more is usually better than checking less... note that the code in that file is very old and thus is not using some of the helpers and is not following the style of later transform implementations
There was a problem hiding this comment.
Addendum: for methods we usually check the name, the number of parameters and then validate all the arguments, so checking the parameter types is usually not necessary.
Problem
Resolves #3379
Solution
Span<T>(void*, int).