Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Consider NativeIntegerAttribute in CopyTypeCustomModifiers() #47536

Merged
merged 5 commits into from
Sep 17, 2020
Merged

Consider NativeIntegerAttribute in CopyTypeCustomModifiers() #47536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b126b03
Consider NativeIntegerAttribute in CopyTypeCustomModifiers()
cston Sep 3, 2020
79a872a
Fix test
cston Sep 8, 2020
790b7a2
Misc.
cston Sep 8, 2020
79b4ed5
Add test for 44358
cston Sep 15, 2020
4fcf72d
Merge remote-tracking branch 'upstream/master' into 42500
cston Sep 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/Metadata/PE/NativeIntegerTypeDecoder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal static TypeSymbol TransformType(TypeSymbol type, EntityHandle handle, P
type;
}

private static TypeSymbol TransformType(TypeSymbol type, ImmutableArray<bool> transformFlags)
internal static TypeSymbol TransformType(TypeSymbol type, ImmutableArray<bool> transformFlags)
{
var decoder = new NativeIntegerTypeDecoder(transformFlags);
try
Expand Down Expand Up @@ -104,7 +104,12 @@ private NamedTypeSymbol TransformNamedType(NamedTypeSymbol type)
{
throw new UnsupportedSignatureContent();
}
return _transformFlags[_index++] ? type.AsNativeInteger() : type;
return (_transformFlags[_index++], type.IsNativeIntegerType) switch
{
(false, true) => type.NativeIntegerUnderlyingType,
(true, false) => type.AsNativeInteger(),
_ => type,
};
}
}

Expand Down
17 changes: 8 additions & 9 deletions src/Compilers/CSharp/Portable/Symbols/Source/CustomModifierUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@cston cston Sep 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated comments in #47576.

In reply to: 490547894 [](ancestors = 490547894)

// NOTE: ref is irrelevant here since we are just encoding/decoding the type out of the signature context
ImmutableArray<bool> flags = CSharpCompilation.DynamicTransformsEncoder.EncodeWithoutCustomModifierFlags(destinationType, refKind);
TypeSymbol typeWithDynamic = DynamicTypeDecoder.TransformTypeWithoutCustomModifierFlags(sourceType, containingAssembly, refKind, flags);
TypeSymbol resultType = DynamicTypeDecoder.TransformTypeWithoutCustomModifierFlags(sourceType, containingAssembly, refKind, flags);

var builder = ArrayBuilder<bool>.GetInstance();
Copy link
Member

@jcouv jcouv Sep 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider adding an overload of NativeIntegerTransformsEncoder.Encode that returns flags as an immutable array (similarly to dynamic and tuple decoders) #Closed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be the only use of that overload. Let's add later if there are other uses.

In reply to: 485231913 [](ancestors = 485231913)

CSharpCompilation.NativeIntegerTransformsEncoder.Encode(builder, destinationType);
resultType = NativeIntegerTypeDecoder.TransformType(resultType, builder.ToImmutableAndFree());

TypeSymbol resultType;
if (destinationType.ContainsTuple() && !sourceType.Equals(destinationType, TypeCompareKind.IgnoreCustomModifiersAndArraySizesAndLowerBounds | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes | TypeCompareKind.IgnoreDynamic))
{
// We also preserve tuple names, if present and different
ImmutableArray<string> names = CSharpCompilation.TupleNamesEncoder.Encode(destinationType);
resultType = TupleTypeDecoder.DecodeTupleTypesIfApplicable(typeWithDynamic, names);
}
else
{
resultType = typeWithDynamic;
resultType = TupleTypeDecoder.DecodeTupleTypesIfApplicable(resultType, names);
}

// Preserve nullable modifiers as well.
Expand All @@ -100,9 +99,9 @@ internal static TypeSymbol CopyTypeCustomModifiers(TypeSymbol sourceType, TypeSy
bool transformResult = resultType.ApplyNullableTransforms(defaultTransformFlag: 0, flagsBuilder.ToImmutableAndFree(), ref position, out resultType);
Debug.Assert(transformResult && position == length);

Debug.Assert(resultType.Equals(sourceType, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes)); // Same custom modifiers as source type.
Debug.Assert(resultType.Equals(sourceType, TypeCompareKind.IgnoreDynamicAndTupleNames | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes | TypeCompareKind.IgnoreNativeIntegers)); // Same custom modifiers as source type.

// Same object/dynamic, nullability and tuple names as destination type.
// Same object/dynamic, nullability, native integers, and tuple names as destination type.
Debug.Assert(resultType.Equals(destinationType, TypeCompareKind.IgnoreCustomModifiersAndArraySizesAndLowerBounds));

return resultType;
Expand Down
196 changes: 196 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NativeIntegerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12237,6 +12237,202 @@ static object Execute(Func<object> f)
{(IntPtr.Size == 4 ? "System.OverflowException" : "0")}");
}

[WorkItem(42500, "https://github.com/dotnet/roslyn/issues/42500")]
[Fact]
public void ExplicitImplementationReturnTypeDifferences()
{
string source =
@"struct S<T>
{
}
interface I
{
S<nint> F1();
S<System.IntPtr> F2();
S<nint> F3();
S<System.IntPtr> F4();
}
class C : I
{
S<System.IntPtr> I.F1() => default;
S<nint> I.F2() => default;
S<nint> I.F3() => default;
S<System.IntPtr> I.F4() => default;
}";
var comp = CreateCompilation(source, parseOptions: TestOptions.Regular9);
comp.VerifyEmitDiagnostics();

var type = comp.GetTypeByMetadataName("I");
Assert.Equal("S<nint> I.F1()", type.GetMember("F1").ToTestDisplayString());
Assert.Equal("S<System.IntPtr> I.F2()", type.GetMember("F2").ToTestDisplayString());
Assert.Equal("S<nint> I.F3()", type.GetMember("F3").ToTestDisplayString());
Assert.Equal("S<System.IntPtr> I.F4()", type.GetMember("F4").ToTestDisplayString());

type = comp.GetTypeByMetadataName("C");
Assert.Equal("S<System.IntPtr> C.I.F1()", type.GetMember("I.F1").ToTestDisplayString());
Assert.Equal("S<nint> C.I.F2()", type.GetMember("I.F2").ToTestDisplayString());
Assert.Equal("S<nint> C.I.F3()", type.GetMember("I.F3").ToTestDisplayString());
Assert.Equal("S<System.IntPtr> C.I.F4()", type.GetMember("I.F4").ToTestDisplayString());
}

[WorkItem(42500, "https://github.com/dotnet/roslyn/issues/42500")]
[WorkItem(44358, "https://github.com/dotnet/roslyn/issues/44358")]
[Fact]
public void OverrideReturnTypeDifferences()
{
string source =
@"class A
{
public virtual nint[] F1() => null;
public virtual System.IntPtr[] F2() => null;
public virtual nint[] F3() => null;
public virtual System.IntPtr[] F4() => null;
}
class B : A
{
public override System.IntPtr[] F1() => null;
public override nint[] F2() => null;
public override nint[] F3() => null;
public override System.IntPtr[] F4() => null;
}";
var comp = CreateCompilation(source, parseOptions: TestOptions.Regular9);
comp.VerifyEmitDiagnostics();

var type = comp.GetTypeByMetadataName("A");
Assert.Equal("nint[] A.F1()", type.GetMember("F1").ToTestDisplayString());
Assert.Equal("System.IntPtr[] A.F2()", type.GetMember("F2").ToTestDisplayString());
Assert.Equal("nint[] A.F3()", type.GetMember("F3").ToTestDisplayString());
Assert.Equal("System.IntPtr[] A.F4()", type.GetMember("F4").ToTestDisplayString());

type = comp.GetTypeByMetadataName("B");
Assert.Equal("System.IntPtr[] B.F1()", type.GetMember("F1").ToTestDisplayString());
Assert.Equal("nint[] B.F2()", type.GetMember("F2").ToTestDisplayString());
Assert.Equal("nint[] B.F3()", type.GetMember("F3").ToTestDisplayString());
Assert.Equal("System.IntPtr[] B.F4()", type.GetMember("F4").ToTestDisplayString());
}

[WorkItem(42500, "https://github.com/dotnet/roslyn/issues/42500")]
[Fact]
public void OverrideParameterTypeCustomModifierDifferences()
{
var sourceA =
@".class private System.Runtime.CompilerServices.NativeIntegerAttribute extends [mscorlib]System.Attribute
{
.method public hidebysig specialname rtspecialname instance void .ctor() cil managed { ret }
}
.class public A
{
.method public hidebysig specialname rtspecialname instance void .ctor() cil managed { ret }
.method public virtual void F1(native int modopt(int32) i)
{
.param [1]
.custom instance void System.Runtime.CompilerServices.NativeIntegerAttribute::.ctor() = ( 01 00 00 00 )
ret
}
.method public virtual void F2(native int modopt(int32) i)
{
ret
}
.method public virtual void F3(native int modopt(int32) i)
{
.param [1]
.custom instance void System.Runtime.CompilerServices.NativeIntegerAttribute::.ctor() = ( 01 00 00 00 )
ret
}
.method public virtual void F4(native int modopt(int32) i)
{
ret
}
}";
var refA = CompileIL(sourceA);

var sourceB =
@"class B : A
{
public override void F1(System.IntPtr i) { }
public override void F2(nint i) { }
public override void F3(nint i) { }
public override void F4(System.IntPtr i) { }
}";
Copy link
Member

@jcouv jcouv Sep 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added all four permutations. Nested cases are covered in the preceding test.

In reply to: 485234664 [](ancestors = 485234664)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

In reply to: 485245803 [](ancestors = 485245803,485242939,485239131,485234664)

var comp = CreateCompilation(sourceB, new[] { refA }, parseOptions: TestOptions.Regular9);
comp.VerifyEmitDiagnostics();

var type = comp.GetTypeByMetadataName("A");
Assert.Equal("void A.F1(nint modopt(System.Int32) i)", type.GetMember("F1").ToTestDisplayString());
Assert.Equal("void A.F2(System.IntPtr modopt(System.Int32) i)", type.GetMember("F2").ToTestDisplayString());
Assert.Equal("void A.F3(nint modopt(System.Int32) i)", type.GetMember("F3").ToTestDisplayString());
Assert.Equal("void A.F4(System.IntPtr modopt(System.Int32) i)", type.GetMember("F4").ToTestDisplayString());

type = comp.GetTypeByMetadataName("B");
Assert.Equal("void B.F1(System.IntPtr modopt(System.Int32) i)", type.GetMember("F1").ToTestDisplayString());
Assert.Equal("void B.F2(nint modopt(System.Int32) i)", type.GetMember("F2").ToTestDisplayString());
Assert.Equal("void B.F3(nint modopt(System.Int32) i)", type.GetMember("F3").ToTestDisplayString());
Assert.Equal("void B.F4(System.IntPtr modopt(System.Int32) i)", type.GetMember("F4").ToTestDisplayString());
}

[WorkItem(42500, "https://github.com/dotnet/roslyn/issues/42500")]
[Fact]
public void OverrideReturnTypeCustomModifierDifferences()
{
var sourceA =
@".class private System.Runtime.CompilerServices.NativeIntegerAttribute extends [mscorlib]System.Attribute
{
.method public hidebysig specialname rtspecialname instance void .ctor() cil managed { ret }
}
.class public A
{
.method public hidebysig specialname rtspecialname instance void .ctor() cil managed { ret }
.method public virtual native int[] modopt(int32) F1()
{
.param [0]
.custom instance void System.Runtime.CompilerServices.NativeIntegerAttribute::.ctor() = ( 01 00 00 00 )
ldnull
throw
}
.method public virtual native int[] modopt(int32) F2()
{
ldnull
throw
}
.method public virtual native int[] modopt(int32) F3()
{
.param [0]
.custom instance void System.Runtime.CompilerServices.NativeIntegerAttribute::.ctor() = ( 01 00 00 00 )
ldnull
throw
}
.method public virtual native int[] modopt(int32) F4()
{
ldnull
throw
}
}";
var refA = CompileIL(sourceA);

var sourceB =
@"class B : A
{
public override System.IntPtr[] F1() => default;
public override nint[] F2() => default;
public override nint[] F3() => default;
public override System.IntPtr[] F4() => default;
}";
var comp = CreateCompilation(sourceB, new[] { refA }, parseOptions: TestOptions.Regular9);
comp.VerifyEmitDiagnostics();

var type = comp.GetTypeByMetadataName("A");
Assert.Equal("nint[] modopt(System.Int32) A.F1()", type.GetMember("F1").ToTestDisplayString());
Assert.Equal("System.IntPtr[] modopt(System.Int32) A.F2()", type.GetMember("F2").ToTestDisplayString());
Assert.Equal("nint[] modopt(System.Int32) A.F3()", type.GetMember("F3").ToTestDisplayString());
Assert.Equal("System.IntPtr[] modopt(System.Int32) A.F4()", type.GetMember("F4").ToTestDisplayString());

type = comp.GetTypeByMetadataName("B");
Assert.Equal("System.IntPtr[] modopt(System.Int32) B.F1()", type.GetMember("F1").ToTestDisplayString());
Assert.Equal("nint[] modopt(System.Int32) B.F2()", type.GetMember("F2").ToTestDisplayString());
Assert.Equal("nint[] modopt(System.Int32) B.F3()", type.GetMember("F3").ToTestDisplayString());
Assert.Equal("System.IntPtr[] modopt(System.Int32) B.F4()", type.GetMember("F4").ToTestDisplayString());
}

[WorkItem(44810, "https://github.com/dotnet/roslyn/issues/44810")]
[Theory]
[InlineData("void*")]
Expand Down
9 changes: 4 additions & 5 deletions src/Compilers/CSharp/Test/Symbol/Symbols/InterfaceImplementationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2554,11 +2554,10 @@ static void Main()
[InlineData("(int X, int Y)", "(int X, int Y)", "(System.Int32 X, System.Int32 Y)", "System.ValueTuple`2[System.Int32,System.Int32]", false)]
[InlineData("nint", "nint", "nint", "System.IntPtr", true)]
[InlineData("nint", "nint", "nint", "System.IntPtr", false)]
// https://github.com/dotnet/roslyn/issues/42500: CopyTypeCustomModifiers() should copy NativeIntegerAttribute
//[InlineData("nint", "System.IntPtr", "System.IntPtr", "System.IntPtr", true)]
//[InlineData("nint", "System.IntPtr", "System.IntPtr", "System.IntPtr", false)]
//[InlineData("System.IntPtr", "nint", "nint", "System.IntPtr", true)]
//[InlineData("System.IntPtr", "nint", "nint", "System.IntPtr", false)]
[InlineData("nint", "System.IntPtr", "System.IntPtr", "System.IntPtr", true)]
[InlineData("nint", "System.IntPtr", "System.IntPtr", "System.IntPtr", false)]
[InlineData("System.IntPtr", "nint", "nint", "System.IntPtr", true)]
[InlineData("System.IntPtr", "nint", "nint", "System.IntPtr", false)]
public void ExplicitImplementationInBaseType_02(string interfaceTypeArg, string baseTypeArg, string expectedTypeArg, string expectedOutput, bool useCompilationReference)
{
var source0 =
Expand Down