From 2ed5e040d45186aeb19c0d166bb97037c1b838d3 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 c9bd7ae6146f040c0f8826463ab1551dfac77c8b 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 bc949189a3c125..e9dc6997d89e7b 100644 --- a/src/mono/mono/metadata/class-setup-vtable.c +++ b/src/mono/mono/metadata/class-setup-vtable.c @@ -1656,6 +1656,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 e1de7ccfdffae56f9c689510b38ccab987258e47 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 75a8d65de4d003b77dd85e2015c9b34695b6d278 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; }