From 65f6b620048ab17a7992b5ede75ad7d31a8f489d Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 5 Feb 2024 14:46:44 -0800 Subject: [PATCH 1/8] ByRefLike generic/interface follow-up work --- docs/design/features/byreflike-generics.md | 126 +++++++++++++++ .../CompilerTypeSystemContext.Validation.cs | 10 ++ .../ReadyToRun/TypeValidationChecker.cs | 7 +- src/coreclr/vm/siginfo.cpp | 6 +- .../constrainedcall/constrained2_brl.il | 153 ++++++++++++++++++ .../constrainedcall/constrained2_brl.ilproj | 5 + .../ByRefLike/GenericTypeSubstitution.cs | 13 +- .../generics/ByRefLike/InvalidCSharp.il | 68 +++++++- 8 files changed, 377 insertions(+), 11 deletions(-) create mode 100644 src/tests/Loader/classloader/DefaultInterfaceMethods/constrainedcall/constrained2_brl.il create mode 100644 src/tests/Loader/classloader/DefaultInterfaceMethods/constrainedcall/constrained2_brl.ilproj diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index d644a25e7f3f22..99aa905ebf0f58 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -127,3 +127,129 @@ The following are IL sequences involving the `box` instruction. They are used fo `box` ; `isinst` ; `unbox.any` – The box, `isint`, and unbox target types are all equal. `box` ; `isinst` ; `br_true/false` – The box target type is equal to the unboxed target type or the box target type is `Nullable` and target type equalities can be computed. + +## Examples + +Below are valid and invalid examples of ByRefLike as Generic parameters. All examples use the **not official** syntax, `allows ref struct`, for indicating the Generic permits ByRefLike types. + +**1) Valid** +```csharp +class A where T1: allows ref struct +{ + public void M(); +} + +// The derived class is okay to lack the 'allows' +// because the base permits non-ByRefLike (default) +// _and_ ByRefLike types. +class B : A +{ + public void N() + => M(); // Any T2 satisfies the constraints from A<> +} +``` + +**2) Invalid** +```csharp +class A +{ + public void M(); +} + +// The derived class cannot push up the allows +// constraint for ByRefLike types. +class B : A where T2: allows ref struct +{ + public void N() + => M(); // A<> may not permit a T2 +} +``` + +**3) Valid** +```csharp +interface IA +{ + void M(); +} + +ref struct A : IA +{ + public void M() { } +} + +class B +{ + // This call is permitted because no boxing is needed + // to dispatch to the method - it is implemented on A. + public static void C(T t) where T: IA, allows ref struct + => t.M(); +} +``` + +**4) Invalid** +```csharp +interface IA +{ + public void M() { } +} + +ref struct A : IA +{ + // Relies on IA::M() implementation. +} + +class B +{ + // Reliance on a DIM forces the generic parameter + // to be boxed, which is invalid for ByRefLike types. + public static void C(T t) where T: IA, allows ref struct + => t.M(); +} +``` + +**5) Valid** +```csharp +class A where T1: allows ref struct +{ +} + +class B +{ + // The type parameter is okay to lack the 'allows' + // because the field permits non-ByRefLike (default) + // _and_ ByRefLike types. + A Field; +} +``` + +**6) Invalid** +```csharp +class A +{ +} + +class B where T2: allows ref struct +{ + // The type parameter can be passed to + // the field type, but will fail if + // T2 is a ByRefLike type. + A Field; +} +``` + +**7) Invalid** +```csharp +class A +{ + virtual void M() where T1: allows ref struct; +} + +class B : A +{ + // Override methods need to match be at least + // as restrictive with respect to constraints. + // If a user has an instance of A, they are + // not aware they could be calling B. + override void M(); +} +``` \ No newline at end of file diff --git a/src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Validation.cs b/src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Validation.cs index 9326fe5978d6fc..60d276ea0f7a05 100644 --- a/src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Validation.cs +++ b/src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Validation.cs @@ -368,6 +368,16 @@ private static TypeDesc EnsureLoadableTypeUncached(TypeDesc type) ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type); } + // Validate fields meet the constraints of the enclosing type. + foreach (FieldDesc field in defType.GetFields()) + { + if (!field.FieldType.IsCanonicalSubtype(CanonicalFormKind.Any) + && !field.FieldType.CheckConstraints()) + { + ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, field.FieldType); + } + } + // Check the type doesn't have bogus MethodImpls or overrides and we can get the finalizer. defType.GetFinalizer(); } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs index 88882e12f1f5b1..d8a5649c674bc3 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs @@ -107,7 +107,7 @@ Task ValidateTypeWorkerHelper(TypeDesc typeToCheckForSkipValidation) // The runtime has a number of checks in the type loader which it will skip running if the SkipValidation flag is set // This function attempts to document all of them, and implement *some* of them. - // This function performs a portion of the validation skipping that has been found to have some importance, or to serve as + // This function performs a portion of the validation skipping that has been found to have some importance, or to serve as // In addition, there are comments about all validation skipping activities that the runtime will perform. try { @@ -488,8 +488,9 @@ static bool CompareGenericParameterConstraint(MethodDesc declMethod, GenericPara if (!parameterOfDecl.HasReferenceTypeConstraint) return false; - if (parameterOfDecl.HasAcceptByRefLikeConstraint) - if (!parameterOfImpl.HasAcceptByRefLikeConstraint) + // Constraints that 'allow' must check the impl first + if (parameterOfImpl.HasAcceptByRefLikeConstraint) + if (!parameterOfDecl.HasAcceptByRefLikeConstraint) return false; HashSet constraintsOnDecl = new HashSet(); diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp index 719073867caa63..558cb896fb7c27 100644 --- a/src/coreclr/vm/siginfo.cpp +++ b/src/coreclr/vm/siginfo.cpp @@ -4813,9 +4813,11 @@ BOOL MetaSig::CompareVariableConstraints(const Substitution *pSubst1, if ((specialConstraints2 & (gpDefaultConstructorConstraint | gpNotNullableValueTypeConstraint)) == 0) return FALSE; } - if ((specialConstraints1 & gpAcceptByRefLike) != 0) + + // Constraints that 'allow' must check the overridden first + if ((specialConstraints2 & gpAcceptByRefLike) != 0) { - if ((specialConstraints2 & gpAcceptByRefLike) == 0) + if ((specialConstraints1 & gpAcceptByRefLike) == 0) return FALSE; } } diff --git a/src/tests/Loader/classloader/DefaultInterfaceMethods/constrainedcall/constrained2_brl.il b/src/tests/Loader/classloader/DefaultInterfaceMethods/constrainedcall/constrained2_brl.il new file mode 100644 index 00000000000000..9382ff66730c2b --- /dev/null +++ b/src/tests/Loader/classloader/DefaultInterfaceMethods/constrainedcall/constrained2_brl.il @@ -0,0 +1,153 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +.assembly extern System.Console { .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) } +.assembly extern System.Runtime { .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) } +.assembly extern Microsoft.DotNet.XUnitExtensions {} +.assembly extern xunit.core {} +.assembly constrained2_brl { } + +.class interface private auto ansi abstract + IAdder +{ + .method public hidebysig newslot virtual + instance int32 Add(int32) cil managed + { + ldstr "Calling DIM from ByRefLike type is invalid" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw + } +} + +.class private sequential ansi sealed beforefieldinit Adder + extends [System.Runtime]System.ValueType + implements IAdder +{ + .custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = ( + 01 00 00 00 + ) + + .field private int32 _field + + .method public hidebysig specialname rtspecialname + instance void .ctor (int32) cil managed + { + ldarg.0 + ldarg.1 + stfld int32 Adder::_field + ret + } + + .method public hidebysig newslot virtual + instance int32 Add(int32) cil managed + { + // Load field and add with argument + ldarg.0 + dup + ldfld int32 valuetype Adder::_field + ldarg.1 + add + + // Update the field + stfld int32 valuetype Adder::_field + + // Return the field value + ldarg.0 + ldfld int32 valuetype Adder::_field + ret + } +} + +.class private sequential ansi sealed beforefieldinit Adder_Invalid + extends [System.Runtime]System.ValueType + implements IAdder +{ + .custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = ( + 01 00 00 00 + ) + + .method public hidebysig specialname rtspecialname + instance void .ctor (int32) cil managed + { + ret + } + + // + // Deferring to the DIM on IAdder + // +} + +.method public hidebysig static int32 Check(!!0, int32) +{ + ldarga.s 0 + ldarg.1 + constrained. !!0 + callvirt instance int32 IAdder::Add(int32) + ret +} + +.class public auto ansi abstract sealed beforefieldinit constrained2_brl + extends [System.Runtime]System.Object +{ + .method public hidebysig static int32 Main() + { + .custom instance void [xunit.core]Xunit.FactAttribute::.ctor() = ( + 01 00 00 00 + ) + .custom instance void [Microsoft.DotNet.XUnitExtensions]Xunit.SkipOnMonoAttribute::.ctor(string, valuetype [Microsoft.DotNet.XUnitExtensions]Xunit.TestPlatforms) = ( + 01 00 2c 4d 6f 6e 6f 20 64 6f 65 73 20 6e 6f 74 + 20 73 75 70 70 6f 72 74 20 42 79 52 65 66 4c 69 + 6b 65 20 67 65 6e 65 72 69 63 73 20 79 65 74 ff + ff ff ff 00 00 + ) + .entrypoint + + .locals init ( + valuetype Adder, + valuetype Adder_Invalid + ) + + // Initialize Adder instance + ldloca.s 0 + ldc.i4 10 + call instance void Adder::.ctor(int32) + + ldstr "Validate constrained call of ByRefLike interface method passes" + call void [System.Console]System.Console::WriteLine(string) + ldloc.0 + ldc.i4 20 + call int32 Check(!!0, int32) + ldc.i4 30 + ceq + brfalse FAIL + + // Initialize Adder_Invalid instance + ldloca.s 1 + ldc.i4 10 + call instance void Adder_Invalid::.ctor(int32) + + .try + { + ldstr "Validate constrained call of ByRefLike interface DIM fails" + call void [System.Console]System.Console::WriteLine(string) + + ldloc.1 + ldc.i4 20 + call int32 Check(!!0, int32) + leave FAIL + } + catch [System.Runtime]System.Exception + { + pop + leave ExpectedFailure + } + + ExpectedFailure: + ldc.i4 100 + ret + + FAIL: + ldc.i4 101 + ret + } +} diff --git a/src/tests/Loader/classloader/DefaultInterfaceMethods/constrainedcall/constrained2_brl.ilproj b/src/tests/Loader/classloader/DefaultInterfaceMethods/constrainedcall/constrained2_brl.ilproj new file mode 100644 index 00000000000000..8aceddffe8f5ef --- /dev/null +++ b/src/tests/Loader/classloader/DefaultInterfaceMethods/constrainedcall/constrained2_brl.ilproj @@ -0,0 +1,5 @@ + + + + + diff --git a/src/tests/Loader/classloader/generics/ByRefLike/GenericTypeSubstitution.cs b/src/tests/Loader/classloader/generics/ByRefLike/GenericTypeSubstitution.cs index 3453b2a1d3e5ee..2fef5e7eacb9b3 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/GenericTypeSubstitution.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/GenericTypeSubstitution.cs @@ -16,7 +16,7 @@ public class GenericTypeSubstitution public static void AllowByRefLike_Substituted_For_AllowByRefLike() { Console.WriteLine($"{nameof(AllowByRefLike_Substituted_For_AllowByRefLike)}..."); - + Console.WriteLine($" -- Instantiate: {Exec.TypeSubstitutionInterfaceImplementationAllowByRefLike()}"); Console.WriteLine($" -- Instantiate: {Exec.TypeSubstitutionInheritanceAllowByRefLike()}"); Console.WriteLine($" -- Instantiate: {Exec.TypeSubstitutionFieldAllowByRefLike()}"); @@ -26,18 +26,25 @@ public static void AllowByRefLike_Substituted_For_AllowByRefLike() [SkipOnMono("Mono does not support ByRefLike generics yet")] public static void NonByRefLike_Substituted_For_AllowByRefLike() { + Console.WriteLine($"{nameof(NonByRefLike_Substituted_For_AllowByRefLike)}..."); + Console.WriteLine($" -- Instantiate: {Exec.TypeSubstitutionInterfaceImplementationNonByRefLike()}"); Console.WriteLine($" -- Instantiate: {Exec.TypeSubstitutionInheritanceNonByRefLike()}"); Console.WriteLine($" -- Instantiate: {Exec.TypeSubstitutionFieldNonByRefLike()}"); } [Fact] - [ActiveIssue("To be created", TestRuntimes.CoreCLR)] [SkipOnMono("Mono does not support ByRefLike generics yet")] - public static void AllowByRefLike_Substituted_For_NonByRefLike_Invalid() + public static void AllowByRefLike_Substituted_For_NonByRefLike() { + Console.WriteLine($"{nameof(AllowByRefLike_Substituted_For_NonByRefLike)}..."); + Assert.Throws(() => { Exec.TypeSubstitutionInterfaceImplementationAllowByRefLikeIntoNonByRefLike(); }); Assert.Throws(() => { Exec.TypeSubstitutionInheritanceAllowByRefLikeIntoNonByRefLike(); }); Assert.Throws(() => { Exec.TypeSubstitutionFieldAllowByRefLikeIntoNonByRefLike(); }); + + Exec.TypeSubstitutionFieldAllowNonByRefLikeIntoNonByRefLike(); + + Assert.Throws(() => { Exec.OverrideMethodNotByRefLike(); }); } } \ No newline at end of file diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il index d81452d398af9d..9a2d9ce7954693 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il @@ -455,6 +455,46 @@ .field public valuetype InvalidCSharp.GenericValueType_Invalid`1 fld; } +.class public auto ansi beforefieldinit InvalidCSharp.BaseClassWithGenericMethod + extends [System.Runtime]System.Object +{ + .method public hidebysig newslot virtual + instance void AcceptsByRefLike () cil managed + { + ret + } + .method public hidebysig specialname rtspecialname + instance void .ctor () cil managed + { + ldarg.0 + call instance void [System.Runtime]System.Object::.ctor() + ret + } +} + +.class public auto ansi beforefieldinit InvalidCSharp.DerivedClassWithGenericMethod_Invalid + extends InvalidCSharp.BaseClassWithGenericMethod +{ + .method public hidebysig static + class InvalidCSharp.BaseClassWithGenericMethod Create () cil managed noinlining + { + newobj instance void InvalidCSharp.DerivedClassWithGenericMethod_Invalid::.ctor() + ret + } + .method public hidebysig virtual + instance void AcceptsByRefLike () cil managed // Missing constraint + { + ret + } + .method private hidebysig specialname rtspecialname + instance void .ctor () cil managed + { + ldarg.0 + call instance void InvalidCSharp.BaseClassWithGenericMethod::.ctor() + ret + } +} + // Entry points .class public auto ansi abstract sealed beforefieldinit Exec @@ -798,11 +838,11 @@ callvirt instance string [System.Runtime]System.Object::ToString() ret } - + .method public hidebysig static string TypeSubstitutionInterfaceImplementationAllowByRefLikeIntoNonByRefLike() cil managed { - ldtoken class InvalidCSharp.GenericDerivedInterface_Invalid`1 + ldtoken class InvalidCSharp.GenericDerivedInterface_Invalid`1 call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) callvirt instance string [System.Runtime]System.Object::ToString() ret @@ -811,7 +851,7 @@ .method public hidebysig static string TypeSubstitutionInheritanceAllowByRefLikeIntoNonByRefLike() cil managed { - ldtoken class InvalidCSharp.GenericDerivedClass_Invalid`1 + ldtoken class InvalidCSharp.GenericDerivedClass_Invalid`1 call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) callvirt instance string [System.Runtime]System.Object::ToString() ret @@ -819,10 +859,32 @@ .method public hidebysig static string TypeSubstitutionFieldAllowByRefLikeIntoNonByRefLike() cil managed + { + ldtoken valuetype InvalidCSharp.GenericValueTypeWrapper_Invalid`1 + call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) + callvirt instance string [System.Runtime]System.Object::ToString() + ret + } + + .method public hidebysig static + string TypeSubstitutionFieldAllowNonByRefLikeIntoNonByRefLike() cil managed { ldtoken valuetype InvalidCSharp.GenericValueTypeWrapper_Invalid`1 call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) callvirt instance string [System.Runtime]System.Object::ToString() ret } + + .method public hidebysig static + void OverrideMethodNotByRefLike() cil managed + { + .locals init ( + [0] class InvalidCSharp.BaseClassWithGenericMethod + ) + call class InvalidCSharp.BaseClassWithGenericMethod InvalidCSharp.DerivedClassWithGenericMethod_Invalid::Create() + stloc.0 + ldloc.0 + callvirt instance void InvalidCSharp.BaseClassWithGenericMethod::AcceptsByRefLike() + ret + } } \ No newline at end of file From 98b85dfe4d376c38c3a9458b97e390f280b59288 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 6 Feb 2024 14:37:26 -0800 Subject: [PATCH 2/8] Create negative tests --- .../ByRefLike/GenericTypeSubstitution.cs | 7 ------ .../generics/ByRefLike/Negative.cs | 25 +++++++++++++++++++ .../ByRefLike/ValidateNegative.csproj | 18 +++++++++++++ 3 files changed, 43 insertions(+), 7 deletions(-) create mode 100644 src/tests/Loader/classloader/generics/ByRefLike/Negative.cs create mode 100644 src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.csproj diff --git a/src/tests/Loader/classloader/generics/ByRefLike/GenericTypeSubstitution.cs b/src/tests/Loader/classloader/generics/ByRefLike/GenericTypeSubstitution.cs index 2fef5e7eacb9b3..b35e59576abba4 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/GenericTypeSubstitution.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/GenericTypeSubstitution.cs @@ -38,13 +38,6 @@ public static void NonByRefLike_Substituted_For_AllowByRefLike() public static void AllowByRefLike_Substituted_For_NonByRefLike() { Console.WriteLine($"{nameof(AllowByRefLike_Substituted_For_NonByRefLike)}..."); - - Assert.Throws(() => { Exec.TypeSubstitutionInterfaceImplementationAllowByRefLikeIntoNonByRefLike(); }); - Assert.Throws(() => { Exec.TypeSubstitutionInheritanceAllowByRefLikeIntoNonByRefLike(); }); - Assert.Throws(() => { Exec.TypeSubstitutionFieldAllowByRefLikeIntoNonByRefLike(); }); - Exec.TypeSubstitutionFieldAllowNonByRefLikeIntoNonByRefLike(); - - Assert.Throws(() => { Exec.OverrideMethodNotByRefLike(); }); } } \ No newline at end of file diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Negative.cs b/src/tests/Loader/classloader/generics/ByRefLike/Negative.cs new file mode 100644 index 00000000000000..4434a9d247bdd7 --- /dev/null +++ b/src/tests/Loader/classloader/generics/ByRefLike/Negative.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using InvalidCSharp; + +using Xunit; + +public class Negative +{ + [Fact] + [SkipOnMono("Mono does not support ByRefLike generics yet")] + public static void AllowByRefLike_Substituted_For_NonByRefLike_Invalid() + { + Console.WriteLine($"{nameof(AllowByRefLike_Substituted_For_NonByRefLike_Invalid)}..."); + + Assert.Throws(() => { Exec.TypeSubstitutionInterfaceImplementationAllowByRefLikeIntoNonByRefLike(); }); + Assert.Throws(() => { Exec.TypeSubstitutionInheritanceAllowByRefLikeIntoNonByRefLike(); }); + Assert.Throws(() => { Exec.TypeSubstitutionFieldAllowByRefLikeIntoNonByRefLike(); }); + Assert.Throws(() => { Exec.OverrideMethodNotByRefLike(); }); + } +} \ No newline at end of file diff --git a/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.csproj b/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.csproj new file mode 100644 index 00000000000000..b04c6b2134db1c --- /dev/null +++ b/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.csproj @@ -0,0 +1,18 @@ + + + + true + true + + + false + true + + + + + + + + + From dc192b65c11bfe0b93a9943f8bfa028a2aa86903 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 7 Feb 2024 12:06:02 -0800 Subject: [PATCH 3/8] Feedback --- .../Compiler/CompilerTypeSystemContext.Validation.cs | 10 ---------- .../XUnitWrapperGenerator/XUnitWrapperGenerator.cs | 4 ++++ .../ByRefLike/{Negative.cs => ValidateNegative.cs} | 2 +- .../generics/ByRefLike/ValidateNegative.csproj | 2 +- 4 files changed, 6 insertions(+), 12 deletions(-) rename src/tests/Loader/classloader/generics/ByRefLike/{Negative.cs => ValidateNegative.cs} (97%) diff --git a/src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Validation.cs b/src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Validation.cs index 60d276ea0f7a05..9326fe5978d6fc 100644 --- a/src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Validation.cs +++ b/src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.Validation.cs @@ -368,16 +368,6 @@ private static TypeDesc EnsureLoadableTypeUncached(TypeDesc type) ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, type); } - // Validate fields meet the constraints of the enclosing type. - foreach (FieldDesc field in defType.GetFields()) - { - if (!field.FieldType.IsCanonicalSubtype(CanonicalFormKind.Any) - && !field.FieldType.CheckConstraints()) - { - ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, field.FieldType); - } - } - // Check the type doesn't have bogus MethodImpls or overrides and we can get the finalizer. defType.GetFinalizer(); } diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index c5a94972b7098f..ec133b20fcbff0 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -806,6 +806,10 @@ private static IEnumerable GetTestMethodInfosForMethod(IMethodSymbol // If we're building tests not for Mono, we can skip handling the specifics of the SkipOnMonoAttribute. continue; } + if (filterAttribute.ConstructorArguments.Length == 1) + { + return ImmutableArray.Empty; + } testInfos = DecorateWithSkipOnPlatform(testInfos, (int)filterAttribute.ConstructorArguments[1].Value!, options); break; case "Xunit.SkipOnPlatformAttribute": diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Negative.cs b/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.cs similarity index 97% rename from src/tests/Loader/classloader/generics/ByRefLike/Negative.cs rename to src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.cs index 4434a9d247bdd7..9c85561c2aa558 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/Negative.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.cs @@ -9,7 +9,7 @@ using Xunit; -public class Negative +public class ValidateNegative { [Fact] [SkipOnMono("Mono does not support ByRefLike generics yet")] diff --git a/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.csproj b/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.csproj index b04c6b2134db1c..8b7d80bd09f607 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.csproj +++ b/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.csproj @@ -9,7 +9,7 @@ true - + From 49601bd39ebbe03d46d38dd59484337071b4589c Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 7 Feb 2024 15:39:40 -0800 Subject: [PATCH 4/8] Fix source generator --- src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs index ec133b20fcbff0..9fad87e9bf4e14 100644 --- a/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs +++ b/src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs @@ -806,7 +806,7 @@ private static IEnumerable GetTestMethodInfosForMethod(IMethodSymbol // If we're building tests not for Mono, we can skip handling the specifics of the SkipOnMonoAttribute. continue; } - if (filterAttribute.ConstructorArguments.Length == 1) + if (filterAttribute.ConstructorArguments.Length <= 1) { return ImmutableArray.Empty; } From a084641d62dba692feb4ff240b55223522590dbc Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 8 Feb 2024 09:03:56 -0800 Subject: [PATCH 5/8] Create new IL negative test assembly. --- .../generics/ByRefLike/InvalidCSharp.il | 89 ----------- .../ByRefLike/InvalidCSharpNegative.il | 143 ++++++++++++++++++ .../ByRefLike/InvalidCSharpNegative.ilproj | 10 ++ .../generics/ByRefLike/ValidateNegative.cs | 2 +- .../ByRefLike/ValidateNegative.csproj | 2 +- 5 files changed, 155 insertions(+), 91 deletions(-) create mode 100644 src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharpNegative.il create mode 100644 src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharpNegative.ilproj diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il index 9a2d9ce7954693..8deff773fb2440 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il @@ -439,15 +439,6 @@ } // Invalid generic substitution of non-allow-byreflike with allow-byreflike -.class interface public auto ansi abstract InvalidCSharp.GenericDerivedInterface_Invalid`1 - implements class InvalidCSharp.GenericInterface_Invalid`1 -{ -} - -.class public sequential ansi sealed beforefieldinit InvalidCSharp.GenericDerivedClass_Invalid`1 - extends class InvalidCSharp.GenericClass_Invalid`1 -{ -} .class public sequential ansi sealed beforefieldinit InvalidCSharp.GenericValueTypeWrapper_Invalid`1 extends [System.Runtime]System.ValueType @@ -455,46 +446,6 @@ .field public valuetype InvalidCSharp.GenericValueType_Invalid`1 fld; } -.class public auto ansi beforefieldinit InvalidCSharp.BaseClassWithGenericMethod - extends [System.Runtime]System.Object -{ - .method public hidebysig newslot virtual - instance void AcceptsByRefLike () cil managed - { - ret - } - .method public hidebysig specialname rtspecialname - instance void .ctor () cil managed - { - ldarg.0 - call instance void [System.Runtime]System.Object::.ctor() - ret - } -} - -.class public auto ansi beforefieldinit InvalidCSharp.DerivedClassWithGenericMethod_Invalid - extends InvalidCSharp.BaseClassWithGenericMethod -{ - .method public hidebysig static - class InvalidCSharp.BaseClassWithGenericMethod Create () cil managed noinlining - { - newobj instance void InvalidCSharp.DerivedClassWithGenericMethod_Invalid::.ctor() - ret - } - .method public hidebysig virtual - instance void AcceptsByRefLike () cil managed // Missing constraint - { - ret - } - .method private hidebysig specialname rtspecialname - instance void .ctor () cil managed - { - ldarg.0 - call instance void InvalidCSharp.BaseClassWithGenericMethod::.ctor() - ret - } -} - // Entry points .class public auto ansi abstract sealed beforefieldinit Exec @@ -839,33 +790,6 @@ ret } - .method public hidebysig static - string TypeSubstitutionInterfaceImplementationAllowByRefLikeIntoNonByRefLike() cil managed - { - ldtoken class InvalidCSharp.GenericDerivedInterface_Invalid`1 - call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) - callvirt instance string [System.Runtime]System.Object::ToString() - ret - } - - .method public hidebysig static - string TypeSubstitutionInheritanceAllowByRefLikeIntoNonByRefLike() cil managed - { - ldtoken class InvalidCSharp.GenericDerivedClass_Invalid`1 - call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) - callvirt instance string [System.Runtime]System.Object::ToString() - ret - } - - .method public hidebysig static - string TypeSubstitutionFieldAllowByRefLikeIntoNonByRefLike() cil managed - { - ldtoken valuetype InvalidCSharp.GenericValueTypeWrapper_Invalid`1 - call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) - callvirt instance string [System.Runtime]System.Object::ToString() - ret - } - .method public hidebysig static string TypeSubstitutionFieldAllowNonByRefLikeIntoNonByRefLike() cil managed { @@ -874,17 +798,4 @@ callvirt instance string [System.Runtime]System.Object::ToString() ret } - - .method public hidebysig static - void OverrideMethodNotByRefLike() cil managed - { - .locals init ( - [0] class InvalidCSharp.BaseClassWithGenericMethod - ) - call class InvalidCSharp.BaseClassWithGenericMethod InvalidCSharp.DerivedClassWithGenericMethod_Invalid::Create() - stloc.0 - ldloc.0 - callvirt instance void InvalidCSharp.BaseClassWithGenericMethod::AcceptsByRefLike() - ret - } } \ No newline at end of file diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharpNegative.il b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharpNegative.il new file mode 100644 index 00000000000000..08767e372ea460 --- /dev/null +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharpNegative.il @@ -0,0 +1,143 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +.assembly extern System.Console { .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) } +.assembly extern System.Runtime { .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) } + +.assembly InvalidCSharpNegative { } + +.class public sequential ansi sealed beforefieldinit ByRefLikeType + extends [System.Runtime]System.ValueType +{ + .custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = ( + 01 00 00 00 + ) +} + +// +// Begin invalid +// + +.class public sequential ansi sealed beforefieldinit InvalidCSharpNegative.GenericClass_Invalid`1 + extends [System.Runtime]System.Object +{ + .method public hidebysig specialname rtspecialname + instance void .ctor () cil managed + { + ldarg.0 + call instance void [System.Runtime]System.Object::.ctor() + ret + } +} + +.class interface public auto ansi abstract InvalidCSharpNegative.GenericInterface_Invalid`1 +{ +} + +.class public sequential ansi sealed beforefieldinit InvalidCSharpNegative.GenericValueType_Invalid`1 + extends [System.Runtime]System.ValueType +{ +} + +// Invalid generic substitution of non-allow-byreflike with allow-byreflike +.class interface public auto ansi abstract InvalidCSharpNegative.GenericDerivedInterface_Invalid`1 + implements class InvalidCSharpNegative.GenericInterface_Invalid`1 +{ +} + +.class public sequential ansi sealed beforefieldinit InvalidCSharpNegative.GenericDerivedClass_Invalid`1 + extends class InvalidCSharpNegative.GenericClass_Invalid`1 +{ +} + +.class public sequential ansi sealed beforefieldinit InvalidCSharpNegative.GenericValueTypeWrapper_Invalid`1 + extends [System.Runtime]System.ValueType +{ + .field public valuetype InvalidCSharpNegative.GenericValueType_Invalid`1 fld; +} + +.class public auto ansi beforefieldinit InvalidCSharpNegative.BaseClassWithGenericMethod + extends [System.Runtime]System.Object +{ + .method public hidebysig newslot virtual + instance void AcceptsByRefLike () cil managed + { + ret + } + .method public hidebysig specialname rtspecialname + instance void .ctor () cil managed + { + ldarg.0 + call instance void [System.Runtime]System.Object::.ctor() + ret + } +} + +.class public auto ansi beforefieldinit InvalidCSharpNegative.DerivedClassWithGenericMethod_Invalid + extends InvalidCSharpNegative.BaseClassWithGenericMethod +{ + .method public hidebysig static + class InvalidCSharpNegative.BaseClassWithGenericMethod Create () cil managed noinlining + { + newobj instance void InvalidCSharpNegative.DerivedClassWithGenericMethod_Invalid::.ctor() + ret + } + .method public hidebysig virtual + instance void AcceptsByRefLike () cil managed // Missing constraint + { + ret + } + .method private hidebysig specialname rtspecialname + instance void .ctor () cil managed + { + ldarg.0 + call instance void InvalidCSharpNegative.BaseClassWithGenericMethod::.ctor() + ret + } +} + +// Entry points + +.class public auto ansi abstract sealed beforefieldinit Exec + extends [System.Runtime]System.Object +{ + .method public hidebysig static + string TypeSubstitutionInterfaceImplementationAllowByRefLikeIntoNonByRefLike() cil managed + { + ldtoken class InvalidCSharpNegative.GenericDerivedInterface_Invalid`1 + call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) + callvirt instance string [System.Runtime]System.Object::ToString() + ret + } + + .method public hidebysig static + string TypeSubstitutionInheritanceAllowByRefLikeIntoNonByRefLike() cil managed + { + ldtoken class InvalidCSharpNegative.GenericDerivedClass_Invalid`1 + call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) + callvirt instance string [System.Runtime]System.Object::ToString() + ret + } + + .method public hidebysig static + string TypeSubstitutionFieldAllowByRefLikeIntoNonByRefLike() cil managed + { + ldtoken valuetype InvalidCSharpNegative.GenericValueTypeWrapper_Invalid`1 + call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) + callvirt instance string [System.Runtime]System.Object::ToString() + ret + } + + .method public hidebysig static + void OverrideMethodNotByRefLike() cil managed + { + .locals init ( + [0] class InvalidCSharpNegative.BaseClassWithGenericMethod + ) + call class InvalidCSharpNegative.BaseClassWithGenericMethod InvalidCSharpNegative.DerivedClassWithGenericMethod_Invalid::Create() + stloc.0 + ldloc.0 + callvirt instance void InvalidCSharpNegative.BaseClassWithGenericMethod::AcceptsByRefLike() + ret + } +} \ No newline at end of file diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharpNegative.ilproj b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharpNegative.ilproj new file mode 100644 index 00000000000000..6b8ac61f3da1e4 --- /dev/null +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharpNegative.ilproj @@ -0,0 +1,10 @@ + + + library + true + true + + + + + \ No newline at end of file diff --git a/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.cs b/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.cs index 9c85561c2aa558..7660130f253d6a 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.cs @@ -5,7 +5,7 @@ using System.IO; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using InvalidCSharp; +using InvalidCSharpNegative; using Xunit; diff --git a/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.csproj b/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.csproj index 8b7d80bd09f607..433b007bbca07e 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.csproj +++ b/src/tests/Loader/classloader/generics/ByRefLike/ValidateNegative.csproj @@ -13,6 +13,6 @@ - + From 54888dcf64e85a9f982a43575d80426682fdaaf3 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 9 Feb 2024 11:08:08 -0800 Subject: [PATCH 6/8] Implement API --- .../Reflection/Execution/TypeLoader/ConstraintValidator.cs | 2 +- .../src/System/Reflection/GenericParameterAttributes.cs | 2 ++ .../System.Private.CoreLib/src/System/RuntimeType.cs | 4 ++-- src/libraries/System.Runtime/ref/System.Runtime.cs | 2 ++ 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/ConstraintValidator.cs b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/ConstraintValidator.cs index ccb1a9279f0a61..cff25deff2af00 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/ConstraintValidator.cs +++ b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/ConstraintValidator.cs @@ -13,7 +13,7 @@ internal static partial class ConstraintValidator { private static bool SatisfiesConstraints(this Type genericVariable, SigTypeContext typeContextOfConstraintDeclarer, Type typeArg) { - GenericParameterAttributes specialConstraints = genericVariable.GenericParameterAttributes & GenericParameterAttributes.SpecialConstraintMask; + GenericParameterAttributes specialConstraints = genericVariable.GenericParameterAttributes; if ((specialConstraints & GenericParameterAttributes.NotNullableValueTypeConstraint) != 0) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/GenericParameterAttributes.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/GenericParameterAttributes.cs index c05d0ec6f2000d..b1bb4ac7353619 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/GenericParameterAttributes.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/GenericParameterAttributes.cs @@ -10,9 +10,11 @@ public enum GenericParameterAttributes VarianceMask = 0x0003, Covariant = 0x0001, Contravariant = 0x0002, + [Obsolete("No longer represents all special constraints")] SpecialConstraintMask = 0x001C, ReferenceTypeConstraint = 0x0004, NotNullableValueTypeConstraint = 0x0008, DefaultConstructorConstraint = 0x0010, + AcceptByRefLike = 0x0020, } } diff --git a/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs b/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs index bd6a6f89bb1353..17fbb2950c6c20 100644 --- a/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs +++ b/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs @@ -692,7 +692,7 @@ public override bool IsAssignableFrom([NotNullWhen(true)] Type? c) if (constraint.IsGenericParameter) { - GenericParameterAttributes special = constraint.GenericParameterAttributes & GenericParameterAttributes.SpecialConstraintMask; + GenericParameterAttributes special = constraint.GenericParameterAttributes; if ((special & GenericParameterAttributes.ReferenceTypeConstraint) == 0 && (special & GenericParameterAttributes.NotNullableValueTypeConstraint) == 0) @@ -704,7 +704,7 @@ public override bool IsAssignableFrom([NotNullWhen(true)] Type? c) if (baseType == ObjectType) { - GenericParameterAttributes special = GenericParameterAttributes & GenericParameterAttributes.SpecialConstraintMask; + GenericParameterAttributes special = GenericParameterAttributes; if ((special & GenericParameterAttributes.NotNullableValueTypeConstraint) != 0) baseType = ValueType; } diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 4c6ffd326a686b..22339656b7d427 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -11587,7 +11587,9 @@ public enum GenericParameterAttributes ReferenceTypeConstraint = 4, NotNullableValueTypeConstraint = 8, DefaultConstructorConstraint = 16, + [Obsolete("No longer represents all special constraints")] SpecialConstraintMask = 28, + AcceptByRefLike = 32, } public partial interface ICustomAttributeProvider { From 1b676c999f070433481af736fa59fdb89e785e92 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 13 Feb 2024 11:19:31 -0800 Subject: [PATCH 7/8] Apply API review feedback. Add FeatureFlag. --- src/coreclr/ilasm/asmparse.y | 2 +- src/coreclr/ilasm/prebuilt/asmparse.cpp | 2 +- src/coreclr/ildasm/dasm.cpp | 2 +- src/coreclr/inc/corhdr.h | 2 +- .../Execution/TypeLoader/ConstraintValidator.cs | 2 +- .../Common/TypeSystem/Common/GenericParameterDesc.cs | 8 ++++---- .../TypeSystem/Common/TypeSystemConstraintsHelpers.cs | 2 +- .../tools/Common/TypeSystem/Ecma/EcmaGenericParameter.cs | 2 +- .../ReadyToRun/TypeGenericInfoMapNode.cs | 2 +- .../ReadyToRun/TypeValidationChecker.cs | 4 ++-- .../CoreTestAssembly/Platform.cs | 1 + src/coreclr/vm/siginfo.cpp | 4 ++-- src/coreclr/vm/typedesc.cpp | 2 +- .../src/System/Reflection/GenericParameterAttributes.cs | 3 +-- .../src/System/Runtime/CompilerServices/RuntimeFeature.cs | 6 ++++++ src/libraries/System.Runtime/ref/System.Runtime.cs | 4 ++-- 16 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/coreclr/ilasm/asmparse.y b/src/coreclr/ilasm/asmparse.y index 73ef9a892b5efb..c9861d58d79740 100644 --- a/src/coreclr/ilasm/asmparse.y +++ b/src/coreclr/ilasm/asmparse.y @@ -486,7 +486,7 @@ typarAttrib : '+' { $$ = gpCovariant; | '-' { $$ = gpContravariant; } | CLASS_ { $$ = gpReferenceTypeConstraint; } | VALUETYPE_ { $$ = gpNotNullableValueTypeConstraint; } - | BYREFLIKE_ { $$ = gpAcceptByRefLike; } + | BYREFLIKE_ { $$ = gpAllowByRefLike; } | _CTOR { $$ = gpDefaultConstructorConstraint; } | FLAGS_ '(' int32 ')' { $$ = (CorGenericParamAttr)$3; } ; diff --git a/src/coreclr/ilasm/prebuilt/asmparse.cpp b/src/coreclr/ilasm/prebuilt/asmparse.cpp index 6bf91f56c57f44..08f686f290187e 100644 --- a/src/coreclr/ilasm/prebuilt/asmparse.cpp +++ b/src/coreclr/ilasm/prebuilt/asmparse.cpp @@ -2523,7 +2523,7 @@ case 152: { yyval.int32 = gpNotNullableValueTypeConstraint; } break; case 153: #line 489 "asmparse.y" -{ yyval.int32 = gpAcceptByRefLike; } break; +{ yyval.int32 = gpAllowByRefLike; } break; case 154: #line 490 "asmparse.y" { yyval.int32 = gpDefaultConstructorConstraint; } break; diff --git a/src/coreclr/ildasm/dasm.cpp b/src/coreclr/ildasm/dasm.cpp index 860b36f2e93666..c72100211d7d26 100644 --- a/src/coreclr/ildasm/dasm.cpp +++ b/src/coreclr/ildasm/dasm.cpp @@ -3081,7 +3081,7 @@ char *DumpGenericPars(_Inout_updates_(SZSTRING_SIZE) char* szString, mdToken tok if ((attr & gpNotNullableValueTypeConstraint) != 0) szptr += sprintf_s(szptr,SZSTRING_REMAINING_SIZE(szptr), "valuetype "); CHECK_REMAINING_SIZE; - if ((attr & gpAcceptByRefLike) != 0) + if ((attr & gpAllowByRefLike) != 0) szptr += sprintf_s(szptr,SZSTRING_REMAINING_SIZE(szptr), "byreflike "); CHECK_REMAINING_SIZE; if ((attr & gpDefaultConstructorConstraint) != 0) diff --git a/src/coreclr/inc/corhdr.h b/src/coreclr/inc/corhdr.h index 3f67b33da9162a..c12c1cfdd4f710 100644 --- a/src/coreclr/inc/corhdr.h +++ b/src/coreclr/inc/corhdr.h @@ -847,7 +847,7 @@ typedef enum CorGenericParamAttr gpReferenceTypeConstraint = 0x0004, // type argument must be a reference type gpNotNullableValueTypeConstraint = 0x0008, // type argument must be a value type but not Nullable gpDefaultConstructorConstraint = 0x0010, // type argument must have a public default constructor - gpAcceptByRefLike = 0x0020, // type argument can be ByRefLike + gpAllowByRefLike = 0x0020, // type argument can be ByRefLike } CorGenericParamAttr; // structures and enums moved from COR.H diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/ConstraintValidator.cs b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/ConstraintValidator.cs index cff25deff2af00..82a2f1b5cbbbc2 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/ConstraintValidator.cs +++ b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/ConstraintValidator.cs @@ -42,7 +42,7 @@ private static bool SatisfiesConstraints(this Type genericVariable, SigTypeConte return false; } - if (typeArg.IsByRefLike && (specialConstraints & (GenericParameterAttributes)0x20 /* GenericParameterAttributes.AcceptByRefLike */) == 0) + if (typeArg.IsByRefLike && (specialConstraints & (GenericParameterAttributes)0x20 /* GenericParameterAttributes.AllowByRefLike */) == 0) return false; // Now check general subtype constraints diff --git a/src/coreclr/tools/Common/TypeSystem/Common/GenericParameterDesc.cs b/src/coreclr/tools/Common/TypeSystem/Common/GenericParameterDesc.cs index e1bf789b1123ed..c361733737d4ff 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/GenericParameterDesc.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/GenericParameterDesc.cs @@ -61,7 +61,7 @@ public enum GenericConstraints /// /// A type is permitted to be ByRefLike. /// - AcceptByRefLike = 0x20, + AllowByRefLike = 0x20, } public abstract partial class GenericParameterDesc : TypeDesc @@ -159,13 +159,13 @@ public bool HasDefaultConstructorConstraint } /// - /// Does this generic parameter have the AcceptByRefLike flag + /// Does this generic parameter have the AllowByRefLike flag /// - public bool HasAcceptByRefLikeConstraint + public bool HasAllowByRefLikeConstraint { get { - return (Constraints & GenericConstraints.AcceptByRefLike) != 0; + return (Constraints & GenericConstraints.AllowByRefLike) != 0; } } diff --git a/src/coreclr/tools/Common/TypeSystem/Common/TypeSystemConstraintsHelpers.cs b/src/coreclr/tools/Common/TypeSystem/Common/TypeSystemConstraintsHelpers.cs index acd8e291cebd40..97bceb5b1b8362 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/TypeSystemConstraintsHelpers.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/TypeSystemConstraintsHelpers.cs @@ -50,7 +50,7 @@ private static bool VerifyGenericParamConstraint(InstantiationContext genericPar } // Check for ByRefLike support - if (instantiationParam.IsByRefLike && (constraints & GenericConstraints.AcceptByRefLike) == 0) + if (instantiationParam.IsByRefLike && (constraints & GenericConstraints.AllowByRefLike) == 0) return false; var instantiatedConstraints = default(ArrayBuilder); diff --git a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaGenericParameter.cs b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaGenericParameter.cs index ac6cd4307ca7c5..8ffa09bc12eb78 100644 --- a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaGenericParameter.cs +++ b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaGenericParameter.cs @@ -113,7 +113,7 @@ public override GenericConstraints Constraints { Debug.Assert((int)GenericConstraints.DefaultConstructorConstraint == (int)GenericParameterAttributes.DefaultConstructorConstraint); GenericParameter parameter = _module.MetadataReader.GetGenericParameter(_handle); - const GenericParameterAttributes mask = GenericParameterAttributes.SpecialConstraintMask | (GenericParameterAttributes)GenericConstraints.AcceptByRefLike; + const GenericParameterAttributes mask = GenericParameterAttributes.SpecialConstraintMask | (GenericParameterAttributes)GenericConstraints.AllowByRefLike; return (GenericConstraints)(parameter.Attributes & mask); } } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeGenericInfoMapNode.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeGenericInfoMapNode.cs index e6b4762433dfc2..8905c2aa31d701 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeGenericInfoMapNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeGenericInfoMapNode.cs @@ -59,7 +59,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) if ((genericParameter.Attributes & GenericParameterAttributes.VarianceMask) != GenericParameterAttributes.None) hasVariance = true; - if ((genericParameter.Attributes & (GenericParameterAttributes.SpecialConstraintMask | (GenericParameterAttributes)GenericConstraints.AcceptByRefLike)) != default(GenericParameterAttributes) || + if ((genericParameter.Attributes & (GenericParameterAttributes.SpecialConstraintMask | (GenericParameterAttributes)GenericConstraints.AllowByRefLike)) != default(GenericParameterAttributes) || (genericParameter.GetConstraints().Count > 0)) { hasConstraints = true; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs index d8a5649c674bc3..a257dbbe967fd7 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs @@ -489,8 +489,8 @@ static bool CompareGenericParameterConstraint(MethodDesc declMethod, GenericPara return false; // Constraints that 'allow' must check the impl first - if (parameterOfImpl.HasAcceptByRefLikeConstraint) - if (!parameterOfDecl.HasAcceptByRefLikeConstraint) + if (parameterOfImpl.HasAllowByRefLikeConstraint) + if (!parameterOfDecl.HasAllowByRefLikeConstraint) return false; HashSet constraintsOnDecl = new HashSet(); diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs index fda0f02f563065..a2c1a1e06ad603 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/CoreTestAssembly/Platform.cs @@ -244,6 +244,7 @@ public class CallConvSuppressGCTransition { } public static class RuntimeFeature { public const string ByRefFields = nameof(ByRefFields); + public const string ByRefLikeGenerics = nameof(ByRefLikeGenerics); public const string UnmanagedSignatureCallingConvention = nameof(UnmanagedSignatureCallingConvention); public const string VirtualStaticsInInterfaces = nameof(VirtualStaticsInInterfaces); } diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp index 558cb896fb7c27..f1d8f652b9e1aa 100644 --- a/src/coreclr/vm/siginfo.cpp +++ b/src/coreclr/vm/siginfo.cpp @@ -4815,9 +4815,9 @@ BOOL MetaSig::CompareVariableConstraints(const Substitution *pSubst1, } // Constraints that 'allow' must check the overridden first - if ((specialConstraints2 & gpAcceptByRefLike) != 0) + if ((specialConstraints2 & gpAllowByRefLike) != 0) { - if ((specialConstraints1 & gpAcceptByRefLike) == 0) + if ((specialConstraints1 & gpAllowByRefLike) == 0) return FALSE; } } diff --git a/src/coreclr/vm/typedesc.cpp b/src/coreclr/vm/typedesc.cpp index 2c43168f3aed2a..bac09165fae728 100644 --- a/src/coreclr/vm/typedesc.cpp +++ b/src/coreclr/vm/typedesc.cpp @@ -1512,7 +1512,7 @@ BOOL TypeVarTypeDesc::SatisfiesConstraints(SigTypeContext *pTypeContextOfConstra return FALSE; } - if (thArg.IsByRefLike() && (specialConstraints & gpAcceptByRefLike) == 0) + if (thArg.IsByRefLike() && (specialConstraints & gpAllowByRefLike) == 0) return FALSE; } diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/GenericParameterAttributes.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/GenericParameterAttributes.cs index b1bb4ac7353619..26bb5d929a4fdd 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/GenericParameterAttributes.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/GenericParameterAttributes.cs @@ -10,11 +10,10 @@ public enum GenericParameterAttributes VarianceMask = 0x0003, Covariant = 0x0001, Contravariant = 0x0002, - [Obsolete("No longer represents all special constraints")] SpecialConstraintMask = 0x001C, ReferenceTypeConstraint = 0x0004, NotNullableValueTypeConstraint = 0x0008, DefaultConstructorConstraint = 0x0010, - AcceptByRefLike = 0x0020, + AllowByRefLike = 0x0020, } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeFeature.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeFeature.cs index e694364942513f..392a4902337ab2 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeFeature.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeFeature.cs @@ -32,6 +32,11 @@ public static partial class RuntimeFeature /// public const string ByRefFields = nameof(ByRefFields); + /// + /// Represents a runtime feature where byref-like types can be used in Generic parameters. + /// + public const string ByRefLikeGenerics = nameof(ByRefLikeGenerics); + /// /// Indicates that this version of runtime supports virtual static members of interfaces. /// @@ -52,6 +57,7 @@ public static bool IsSupported(string feature) case PortablePdb: case CovariantReturnsOfClasses: case ByRefFields: + case ByRefLikeGenerics: case UnmanagedSignatureCallingConvention: case DefaultImplementationsOfInterfaces: case VirtualStaticsInInterfaces: diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 22339656b7d427..f755a4310e058a 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -11587,9 +11587,8 @@ public enum GenericParameterAttributes ReferenceTypeConstraint = 4, NotNullableValueTypeConstraint = 8, DefaultConstructorConstraint = 16, - [Obsolete("No longer represents all special constraints")] SpecialConstraintMask = 28, - AcceptByRefLike = 32, + AllowByRefLike = 32, } public partial interface ICustomAttributeProvider { @@ -13104,6 +13103,7 @@ public RuntimeCompatibilityAttribute() { } public static partial class RuntimeFeature { public const string ByRefFields = "ByRefFields"; + public const string ByRefLikeGenerics = "ByRefLikeGenerics"; public const string CovariantReturnsOfClasses = "CovariantReturnsOfClasses"; public const string DefaultImplementationsOfInterfaces = "DefaultImplementationsOfInterfaces"; public const string NumericIntPtr = "NumericIntPtr"; From d7c0190608bc1900d5cec9a10e1cad3bc7ebe33e Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 29 Feb 2024 14:02:06 -0800 Subject: [PATCH 8/8] Review feedback --- .../Execution/TypeLoader/ConstraintValidator.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/ConstraintValidator.cs b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/ConstraintValidator.cs index 82a2f1b5cbbbc2..d347b6d0f4af74 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/ConstraintValidator.cs +++ b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/ConstraintValidator.cs @@ -13,9 +13,9 @@ internal static partial class ConstraintValidator { private static bool SatisfiesConstraints(this Type genericVariable, SigTypeContext typeContextOfConstraintDeclarer, Type typeArg) { - GenericParameterAttributes specialConstraints = genericVariable.GenericParameterAttributes; + GenericParameterAttributes attributes = genericVariable.GenericParameterAttributes; - if ((specialConstraints & GenericParameterAttributes.NotNullableValueTypeConstraint) != 0) + if ((attributes & GenericParameterAttributes.NotNullableValueTypeConstraint) != 0) { if (!typeArg.IsValueType) { @@ -30,19 +30,19 @@ private static bool SatisfiesConstraints(this Type genericVariable, SigTypeConte } } - if ((specialConstraints & GenericParameterAttributes.ReferenceTypeConstraint) != 0) + if ((attributes & GenericParameterAttributes.ReferenceTypeConstraint) != 0) { if (typeArg.IsValueType) return false; } - if ((specialConstraints & GenericParameterAttributes.DefaultConstructorConstraint) != 0) + if ((attributes & GenericParameterAttributes.DefaultConstructorConstraint) != 0) { if (!typeArg.HasExplicitOrImplicitPublicDefaultConstructor()) return false; } - if (typeArg.IsByRefLike && (specialConstraints & (GenericParameterAttributes)0x20 /* GenericParameterAttributes.AllowByRefLike */) == 0) + if (typeArg.IsByRefLike && (attributes & (GenericParameterAttributes)0x20 /* GenericParameterAttributes.AllowByRefLike */) == 0) return false; // Now check general subtype constraints