From 0ee58ab0580e348af734127851bc58a4a0a400e6 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 975b17fb42fc17b2caa414be8145df67dc78f7ac 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 9c02eb324691e08c16d86216858831c714b97816 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 473902cb3409a11fceeed8c7547d98f6891213b4 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; }