Implement support for Inline Array conversions #68019
Conversation
ghost
added
the
untriaged
|
|
||
| // Neither Span<T>, nor ReadOnlySpan<T> can be wrapped into a Nullable<T>, therefore, there is no point to check for an attempt to convert to Nullable types here. | ||
| if ((destination.OriginalDefinition.Equals(Compilation.GetWellKnownType(WellKnownType.System_Span_T), TypeCompareKind.AllIgnoreOptions) || | ||
| destination.OriginalDefinition.Equals(Compilation.GetWellKnownType(WellKnownType.System_ReadOnlySpan_T), TypeCompareKind.AllIgnoreOptions)) && |
There was a problem hiding this comment.
This condition seems relatively expensive, at least compared to the next condition. Consider checking source?.HasInlineArrayAttribute(out _) == true first. #Resolved
There was a problem hiding this comment.
This condition seems relatively expensive, at least compared to the next condition. Consider checking
source?.HasInlineArrayAttribute(out _) == truefirst.
The order is intentional. Checking source?.HasInlineArrayAttribute(out _) == true first causes a cycle. I do not plan to address it in context of this PR. Besides, symbols for well known types are cached and, since we are comparing definitions, this condition shouldn't have noticeable impact on performance.
| createSpan = _factory.ModuleBuilderOpt.EnsureInlineArrayAsSpanExists(syntax, spanType.OriginalDefinition, _factory.SpecialType(SpecialType.System_Int32), _diagnostics.DiagnosticBag); | ||
| } | ||
|
|
||
| createSpan = createSpan.Construct(rewrittenOperand.Type, spanType.TypeArgumentsWithAnnotationsNoUseSiteDiagnostics.Single().Type); |
There was a problem hiding this comment.
Perhaps just pass in the array
I do not think there is a Construct method that takes a TypeSymbol and an array.
| Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "b4").WithArguments("Buffer10<string>", "System.Span<string?>").WithLocation(26, 34), | ||
| // (27,42): warning CS8619: Nullability of reference types in value of type 'Buffer10<string>' doesn't match target type 'ReadOnlySpan<string?>'. | ||
| // System.ReadOnlySpan<string?> y = b4; | ||
| Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "b4").WithArguments("Buffer10<string>", "System.ReadOnlySpan<string?>").WithLocation(27, 42) |
There was a problem hiding this comment.
Should we test something similar separately for nullable value types (like int?)? #Resolved
There was a problem hiding this comment.
Should we test something similar separately for nullable value types (like
int?)?
A scenario like that wouldn't be any different than the one tested inConversion_ElementTypeMismatch. That is because int? and int are different types, whereas string? and string are the same type, but with different nullability.
| } | ||
|
|
||
| private static Conversion GenerateConversion(Conversions conversions, BoundExpression? sourceExpression, TypeSymbol? sourceType, TypeSymbol destinationType, bool fromExplicitCast, bool extensionMethodThisArgument, bool isChecked) | ||
| private static Conversion GenerateConversion(Conversions conversions, BoundExpression? sourceExpression, TypeSymbol? sourceType, TypeSymbol destinationType, bool fromExplicitCast, bool extensionMethodThisArgument, bool isChecked, bool? fromExpression = null) |
There was a problem hiding this comment.
Does caller VisitArgumentConversionAndInboundAssignmentsAndPreConditions care if it doesn't set fromExpression: true for inline array conversions? #Resolved
There was a problem hiding this comment.
Switched to a more robust approach that doesn't need the parameter.
3 participants
https://github.com/dotnet/csharplang/blob/main/proposals/inline-arrays.md#conversions