From e4848bd8be10a84ee0ea79ea7067b0a523c0b7a1 Mon Sep 17 00:00:00 2001 From: Radek Doulik Date: Mon, 18 Sep 2017 15:23:38 +0200 Subject: [PATCH] [linker] improve method detection in FixAbstractMethodsStep The forms team run into an issue with debugger in VS [1]. During investigation it turned out that our FixAbstractMethodsStep injects 2 methods, which were not needed, as the methods were already implemented. The bug itself looks like an issue, where original mdb file was used in the apk, instead of the one written by our linker. This change improves the method detection, where explicit interface methods name starts with `global::` [2]. In this particular case it were these methods: global::Android.Views.View.IOnTouchListener.OnTouch global::Android.Views.View.IOnClickListener.OnClick This is possibly compiler specific, so we rather look in MethodDefinition's Overrides to find the interface method and compare it to the interface method we are looking for. [1] https://bugzilla.xamarin.com/show_bug.cgi?id=59293 [2] excerpt from ikdasm output: .method private hidebysig newslot virtual final instance void 'global::Android.Views.View.IOnClickListener.OnClick'(class [Mono.Android]Android.Views.View v) cil managed { .override [Mono.Android]Android.Views.View/IOnClickListener::OnClick --- .../MonoDroid.Tuner/FixAbstractMethodsStep.cs | 15 +++++++++++++++ .../java/test/bindings/Cursor.java | 1 + .../java/test/bindings/Cursor.java | 1 + .../MyClrCursor.cs | 9 ++++++++- .../Xamarin.Android.JcwGen-Tests/BindingTests.cs | 5 +++++ 5 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs index aa6878fb9ec..101ecc5052b 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs @@ -95,8 +95,23 @@ static bool CompareTypes (TypeReference iType, TypeReference tType) return true; } + bool IsInOverrides (MethodDefinition iMethod, MethodDefinition tMethod) + { + if (!tMethod.HasOverrides) + return false; + + foreach (var o in tMethod.Overrides) + if (o != null && iMethod == o.Resolve ()) + return true; + + return false; + } + bool HaveSameSignature (TypeReference iface, MethodDefinition iMethod, MethodDefinition tMethod) { + if (IsInOverrides (iMethod, tMethod)) + return true; + if (iMethod.Name != tMethod.Name && (iMethod.DeclaringType == null || (iMethod.DeclaringType.DeclaringType == null ? (string.Format ("{0}.{1}", iface.FullName, iMethod.Name) != tMethod.Name) : (string.Format ("{0}.{1}.{2}", iMethod.DeclaringType.DeclaringType, iface.Name, iMethod.Name) != tMethod.Name)))) return false; diff --git a/tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-APIv1Binding/java/test/bindings/Cursor.java b/tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-APIv1Binding/java/test/bindings/Cursor.java index bafe90efd21..23dce026b57 100644 --- a/tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-APIv1Binding/java/test/bindings/Cursor.java +++ b/tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-APIv1Binding/java/test/bindings/Cursor.java @@ -3,6 +3,7 @@ public interface Cursor { void method(); int methodWithRT (); + int methodWithCursor (Cursor cursor); int methodWithParams (int number, String text); int methodWithParams (int number, String text, float real); } diff --git a/tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-APIv2Binding/java/test/bindings/Cursor.java b/tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-APIv2Binding/java/test/bindings/Cursor.java index 659a06409f7..19bbc42dbf5 100644 --- a/tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-APIv2Binding/java/test/bindings/Cursor.java +++ b/tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-APIv2Binding/java/test/bindings/Cursor.java @@ -5,6 +5,7 @@ public interface Cursor { void newMethod(); int methodWithRT (); int newMethodWithRT (); + int methodWithCursor (Cursor cursor); int methodWithParams (int number, String text); int newMethodWithParams (int number, String text); int methodWithParams (int number, String text, float real); diff --git a/tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-Library/MyClrCursor.cs b/tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-Library/MyClrCursor.cs index 5843e1116c9..eb38eb58d23 100644 --- a/tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-Library/MyClrCursor.cs +++ b/tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-Library/MyClrCursor.cs @@ -3,7 +3,7 @@ namespace Library { - public class MyClrCursor : Java.Lang.Object, ICursor + public class MyClrCursor : Java.Lang.Object, global::Test.Bindings.ICursor { public void Method () { @@ -23,6 +23,13 @@ public int MethodWithRT () { return 3; } + + int global::Test.Bindings.ICursor.MethodWithCursor (global::Test.Bindings.ICursor cursor) + { + var a = 2; + var b = 2; + return a + b; + } } } diff --git a/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs b/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs index c6cfa594bbb..d6cb60dbb9a 100644 --- a/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs +++ b/tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs @@ -179,6 +179,11 @@ public void JavaAbstractMethodTest () if (e.GetType ().ToString () != "Java.Lang.AbstractMethodError") throw e; } + + var mi = ic.GetType ().GetMethod ("MethodWithCursor"); + + if (mi != null && mi.GetMethodBody ().LocalVariables.Count == 0) + throw new Exception ("FixAbstractMethodStep broken, MethodWithRT added, while it should not be"); } // Context https://bugzilla.xamarin.com/show_bug.cgi?id=36036