From d7f5b69dab1111f55fcc79b2e7978221bc80198a Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Wed, 23 Aug 2023 10:55:23 +0200 Subject: [PATCH 1/4] Add failing test for generic by-ref parameter constrained to `Enum` This triggers a `ArgumentException`: "Cannot create an instance of `TEnum` because `Type.ContainsGenericParameters` is true." --- .../IHaveGenericMethodWithEnumConstraint.cs | 23 +++++++++++++++++++ .../GenericConstraintsTestCase.cs | 6 +++++ 2 files changed, 29 insertions(+) create mode 100644 src/Castle.Core.Tests/DynamicProxy.Tests/GenInterfaces/IHaveGenericMethodWithEnumConstraint.cs diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/GenInterfaces/IHaveGenericMethodWithEnumConstraint.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/GenInterfaces/IHaveGenericMethodWithEnumConstraint.cs new file mode 100644 index 0000000000..c0ec122681 --- /dev/null +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/GenInterfaces/IHaveGenericMethodWithEnumConstraint.cs @@ -0,0 +1,23 @@ +// Copyright 2004-2023 Castle Project - http://www.castleproject.org/ +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +namespace Castle.DynamicProxy.Tests.GenInterfaces +{ + using System; + + public interface IHaveGenericMethodWithEnumConstraint + { + void Method(out TEnum result) where TEnum : Enum; + } +} diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/GenericConstraintsTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/GenericConstraintsTestCase.cs index efdb75f0f9..5591ed0fbd 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/GenericConstraintsTestCase.cs +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/GenericConstraintsTestCase.cs @@ -33,6 +33,12 @@ public void Generic_type_generic_method_with_struct_base_Method_base_Type_constr CreateProxyFor>(); } + [Test] + public void Non_generic_type_generic_method_with_by_ref_parameter_type_constrained_to_Enum() + { + CreateProxyFor(); + } + private T CreateProxyFor(params IInterceptor[] interceptors) where T : class { From 18b514ca469dc651c08b7291faf0945f6829923b Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Wed, 23 Aug 2023 11:35:34 +0200 Subject: [PATCH 2/4] Disable enum-specific logic for generic parameters --- .../DynamicProxy/Generators/Emitters/OpCodeUtil.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Castle.Core/DynamicProxy/Generators/Emitters/OpCodeUtil.cs b/src/Castle.Core/DynamicProxy/Generators/Emitters/OpCodeUtil.cs index 4b1ac75ed3..2a9c43bc5a 100644 --- a/src/Castle.Core/DynamicProxy/Generators/Emitters/OpCodeUtil.cs +++ b/src/Castle.Core/DynamicProxy/Generators/Emitters/OpCodeUtil.cs @@ -27,7 +27,7 @@ internal abstract class OpCodeUtil /// public static void EmitLoadIndirectOpCodeForType(ILGenerator gen, Type type) { - if (type.IsEnum) + if (type.IsEnum && !type.IsGenericParameter) { EmitLoadIndirectOpCodeForType(gen, GetUnderlyingTypeOfEnum(type)); return; @@ -107,7 +107,7 @@ public static void EmitLoadOpCodeForDefaultValueOfType(ILGenerator gen, Type typ /// public static void EmitStoreIndirectOpCodeForType(ILGenerator gen, Type type) { - if (type.IsEnum) + if (type.IsEnum && !type.IsGenericParameter) { EmitStoreIndirectOpCodeForType(gen, GetUnderlyingTypeOfEnum(type)); return; From e9c957c956dacf6bfea1b5939cc5806ff5b0c943 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Wed, 23 Aug 2023 11:38:40 +0200 Subject: [PATCH 3/4] Reorder checks so the 1st looks less special-casey * As the previous commit showed, the `IsGenericParameter` check must have priority... probably not just over enums, but over any other value types, too. * The `IsEnum` check can be put behind `IsValueType` so it does not run for non-value types. --- .../Generators/Emitters/OpCodeUtil.cs | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/Castle.Core/DynamicProxy/Generators/Emitters/OpCodeUtil.cs b/src/Castle.Core/DynamicProxy/Generators/Emitters/OpCodeUtil.cs index 2a9c43bc5a..d2ab0c3335 100644 --- a/src/Castle.Core/DynamicProxy/Generators/Emitters/OpCodeUtil.cs +++ b/src/Castle.Core/DynamicProxy/Generators/Emitters/OpCodeUtil.cs @@ -27,12 +27,6 @@ internal abstract class OpCodeUtil /// public static void EmitLoadIndirectOpCodeForType(ILGenerator gen, Type type) { - if (type.IsEnum && !type.IsGenericParameter) - { - EmitLoadIndirectOpCodeForType(gen, GetUnderlyingTypeOfEnum(type)); - return; - } - if (type.IsByRef) { throw new NotSupportedException("Cannot load ByRef values"); @@ -48,13 +42,20 @@ public static void EmitLoadIndirectOpCodeForType(ILGenerator gen, Type type) gen.Emit(opCode); } - else if (type.IsValueType) + else if (type.IsGenericParameter) { gen.Emit(OpCodes.Ldobj, type); } - else if (type.IsGenericParameter) + else if (type.IsValueType) { - gen.Emit(OpCodes.Ldobj, type); + if (type.IsEnum) + { + EmitLoadIndirectOpCodeForType(gen, GetUnderlyingTypeOfEnum(type)); + } + else + { + gen.Emit(OpCodes.Ldobj, type); + } } else { @@ -107,12 +108,6 @@ public static void EmitLoadOpCodeForDefaultValueOfType(ILGenerator gen, Type typ /// public static void EmitStoreIndirectOpCodeForType(ILGenerator gen, Type type) { - if (type.IsEnum && !type.IsGenericParameter) - { - EmitStoreIndirectOpCodeForType(gen, GetUnderlyingTypeOfEnum(type)); - return; - } - if (type.IsByRef) { throw new NotSupportedException("Cannot store ByRef values"); @@ -128,13 +123,20 @@ public static void EmitStoreIndirectOpCodeForType(ILGenerator gen, Type type) gen.Emit(opCode); } - else if (type.IsValueType) + else if (type.IsGenericParameter) { gen.Emit(OpCodes.Stobj, type); } - else if (type.IsGenericParameter) + else if (type.IsValueType) { - gen.Emit(OpCodes.Stobj, type); + if (type.IsEnum) + { + EmitStoreIndirectOpCodeForType(gen, GetUnderlyingTypeOfEnum(type)); + } + else + { + gen.Emit(OpCodes.Stobj, type); + } } else { From 1bc7f18193bd04dbcac7c5feb3be16024c458914 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Wed, 23 Aug 2023 11:45:28 +0200 Subject: [PATCH 4/4] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0309e13178..c7c205052b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Enhancements: Bugfixes: - `ArgumentException`: "Could not find method overriding method" with overridden class method having generic by-ref parameter (@stakx, #657) +- `ArgumentException`: "Cannot create an instance of `TEnum` because `Type.ContainsGenericParameters` is true" caused by `Enum` constraint on method `out` parameter (@stakx, #658) ## 5.1.1 (2022-12-30)