From e1cfc2dccf56fbaa206e3d4ec091c9555c59f726 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 28 Sep 2022 14:39:35 -0400 Subject: [PATCH 1/4] Add test for covariant reabstraction --- .../UnitTest/OverrideReabstracted.cs | 55 +++++++++++++++++++ .../UnitTest/OverrideReabstracted.csproj | 8 +++ 2 files changed, 63 insertions(+) create mode 100644 src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.cs create mode 100644 src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.csproj diff --git a/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.cs b/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.cs new file mode 100644 index 00000000000000..1b790cae1992fb --- /dev/null +++ b/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.cs @@ -0,0 +1,55 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + + +namespace ReproMAUI6811; + +public static class Program +{ + public static int Main() + { + Leaf l = new Leaf(); + + if (l.getI().ToString() != "Leaf") + return 1; + if (((Intermediate)l).getI().ToString() != "Leaf") + return 2; + if (((PseudoBase)l).getI().ToString() != "Leaf") + return 3; + if (((Base)l).getI().ToString() != "Leaf") + return 4; + return 100; + } +} + +public abstract class Base { + public abstract I getI(); +} + +public class PseudoBase : Base { + public override I getI() => new C ("PseudoBase"); +} + +public abstract class Intermediate : PseudoBase { + public override abstract I getI(); +} + +public class Leaf : Intermediate { + public Leaf() {} + public override C getI() { return new C ("Leaf"); } +} + +public interface I {} + +public class C : I { + private readonly string _repr; + public C(string s) { _repr = s; } + public override string ToString() => _repr; +} + + diff --git a/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.csproj b/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.csproj new file mode 100644 index 00000000000000..570644f1dbcb7e --- /dev/null +++ b/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.csproj @@ -0,0 +1,8 @@ + + + Exe + + + + + From 306055403cbf0e2f840425961b7067b0e5e1f4bc Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 28 Sep 2022 14:40:47 -0400 Subject: [PATCH 2/4] [metadata] Skip null vtable entires when checking covariant overrides When there are covariant return overrides, we check the vtable slot in every parent class for signature compatability with the proposed override. However if one of the ancestor classes is abstract, it could have an abstract override for hte method: class Base { public virtual Base Method() => this; } public abstract Intermediate : Base { public override abstract Base Method(); } public Leaf : Intermediate { public override Leaf Method() => this; } In this case when we're checking that Leaf.Method is compatible with the vtable slot in Intermediate, we will see a null method (since Intermediate is abstract). In such cases we can skip over the class and continue with its parent. Fixes https://github.com/dotnet/runtime/issues/76312 --- src/mono/mono/metadata/class-setup-vtable.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/mono/mono/metadata/class-setup-vtable.c b/src/mono/mono/metadata/class-setup-vtable.c index bbd6b6a2f2c27e..2c56437ae97a52 100644 --- a/src/mono/mono/metadata/class-setup-vtable.c +++ b/src/mono/mono/metadata/class-setup-vtable.c @@ -1643,6 +1643,10 @@ check_vtable_covariant_override_impls (MonoClass *klass, MonoMethod **vtable, in break; MonoMethod *prev_impl = cur_class->vtable[slot]; + // if the current class re-abstracted the method, it may not be there. + if (!prev_impl) + continue; + if (prev_impl != last_checked_prev_override) { /* * the new impl should be subsumed by the prior one, ie this From 8cd17418e5ae1810431b996cf30c93960eb8b90f Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 28 Sep 2022 15:01:09 -0400 Subject: [PATCH 3/4] Fix copypaste --- .../CovariantReturns/UnitTest/OverrideReabstracted.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.cs b/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.cs index 1b790cae1992fb..afc62ad708e6f6 100644 --- a/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.cs +++ b/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.cs @@ -1,9 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - using System; From 3e4640aa3d858be4f894a57c9dd64515371ed8d7 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 28 Sep 2022 15:03:18 -0400 Subject: [PATCH 4/4] fix whitespace --- .../CovariantReturns/UnitTest/OverrideReabstracted.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.cs b/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.cs index afc62ad708e6f6..f0f461777e7002 100644 --- a/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.cs +++ b/src/tests/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/OverrideReabstracted.cs @@ -29,7 +29,7 @@ public abstract class Base { } public class PseudoBase : Base { - public override I getI() => new C ("PseudoBase"); + public override I getI() => new C ("PseudoBase"); } public abstract class Intermediate : PseudoBase { @@ -44,9 +44,9 @@ public Leaf() {} public interface I {} public class C : I { - private readonly string _repr; - public C(string s) { _repr = s; } - public override string ToString() => _repr; + private readonly string _repr; + public C(string s) { _repr = s; } + public override string ToString() => _repr; }