Consider NativeIntegerAttribute in CopyTypeCustomModifiers() #47536
Conversation
| return _transformFlags[_index++] ? type.AsNativeInteger() : type; | ||
| return _transformFlags[_index++] ? | ||
| type.AsNativeInteger() : | ||
| (type.NativeIntegerUnderlyingType ?? type); |
There was a problem hiding this comment.
Is it correct that this change is so that we can handle all four cases: type is nint or IntPtr, and transform flag is set or not?
If so, I don't understand why we would get a nint type. I would expect only IntPtr here. #Closed
There was a problem hiding this comment.
Yes, all four cases are possible from CopyTypeCustomModifiers(). Updated.
In reply to: 485230638 [](ancestors = 485230638)
| TypeSymbol typeWithDynamic = DynamicTypeDecoder.TransformTypeWithoutCustomModifierFlags(sourceType, containingAssembly, refKind, flags); | ||
| TypeSymbol resultType = DynamicTypeDecoder.TransformTypeWithoutCustomModifierFlags(sourceType, containingAssembly, refKind, flags); | ||
|
|
||
| var builder = ArrayBuilder<bool>.GetInstance(); |
There was a problem hiding this comment.
nit: consider adding an overload of NativeIntegerTransformsEncoder.Encode that returns flags as an immutable array (similarly to dynamic and tuple decoders) #Closed
There was a problem hiding this comment.
This would be the only use of that overload. Let's add later if there are other uses.
In reply to: 485231913 [](ancestors = 485231913)
| { | ||
| public override void F1(System.IntPtr i) { } | ||
| public override void F2(nint i) { } | ||
| }"; |
There was a problem hiding this comment.
Could we complete the four permutations : base has modified IntPtr vs. nint, and override has unmodified IntPt vs. nint?
Could we also add one nested case? #Closed
There was a problem hiding this comment.
Added all four permutations. Nested cases are covered in the preceding test.
In reply to: 485234664 [](ancestors = 485234664)
There was a problem hiding this comment.
The problem with the preceding case is it doesn't have modifiers to copy. Are we even entering the CopyTypeCustomModifiers method?
In reply to: 485239131 [](ancestors = 485239131,485234664)
There was a problem hiding this comment.
CopyTypeCustomModifiers() is called for overrides and explicit implementations, even without custom modifiers. I've changed the last test here to cover nested cases as well.
In reply to: 485242939 [](ancestors = 485242939,485239131,485234664)
There was a problem hiding this comment.
|
@dotnet/roslyn-compiler, please review, thanks. |
| @@ -74,18 +74,17 @@ internal static TypeSymbol CopyTypeCustomModifiers(TypeSymbol sourceType, TypeSy | |||
| // code gen uses and then passing them to the dynamic type decoder that metadata reading uses. | |||
There was a problem hiding this comment.
nit: Consider revising the <param name="destinationType"> comment on line 63 to delete its last sentence "May differ in object/dynamic.", since there are several other scenarios that this method handles, possibly more in the future, and we don't want to give an impression that object/dynamic is all that we're handling here.
ghost
modified the milestones:
Fixes #42500.
Fixes #44358.