-
Notifications
You must be signed in to change notification settings - Fork 53
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Xamarin.Android.Tools.Bytecode] Support @jvmoverloads (#651)
@moljac ran into an interesting warning from `class-parse` when binding [OfficeUIFrabric][0]: warning : class-parse: warning: method com/microsoft/officeuifabric/widget/BottomNavigationView.<init>(Landroid/content/Context;)V: Local variables array has 0 entries ('LocalVariableTableAttribute(LocalVariableTableEntry(Name='context', Descriptor='Landroid/content/Context;'))'); descriptor has 1 entries! For starters, this shouldn't be a warning; it's not actionable. There is nothing a user can do to fix this warning; it's only meaningful to the Java.Interop team. Update `Xamarin.Android.Tools.Bytecode.MethodInfo.GetParameters()` so that these messages are *debug* messages, not warnings. `class-parse` knows how many parameters are present based on the JNI method descriptor, in this case `(Landroid/content/Context;)V`, which specifies one parameter type. The JNI descriptor *doesn't* contain parameter name information; where do parameter names come from? If the `.class` file was built via `javac -parameters`, then the `MethodParametersAttribute` blob can be used; see 4273e5c. However, before checking for `MethodParametersAttribute`, `class-parse` will attempt to use the `LocalVariableTableAttribute` and `LocalVariableTableEntry` values to infer parameter names. As part of this inference, method parameters are assumed to be variables with `LocalVariableTableEntry.StartPC` is 0; from `class-parse --dump BottomNavigationView.class`: LocalVariableTableAttribute( LocalVariableTableEntry(Name='context', Descriptor='Landroid/content/Context;', StartPC=0, Index=1)) Next, `class-parse` will "skip" the first variable for instance methods. This results in "skipping" the `context` variable, resulting in the message: Local variables array has 0 entries …; descriptor has 1 entries! To fix this, `class-parse` needs a stricter "skip the `this` parameter" check: the first parameter should *only* be skipped when: 1. The method is an instance method, not a `static` method, *and* 2. The JNI descriptor of the first parameter is the same as the descriptor of the declaring type. Additionally, update the code style to use `enumValue.HasFlag(V)` instead of `(enumValue & V) == V` for readability. Finally, why was this failing in the first place? In some circumstances, when the Kotlin [`@JvmOverloads`][2] annotation is used on a `constructor`, Kotlin will emit all possible overloads for the constructor, and in many of those overloads the `this` parameter *won't* be present in the `LocalVariableTableAttribute` data, as seen above with `BottomNavigationView`. [0]: https://jcenter.bintray.com/com/microsoft/uifabric/OfficeUIFabric/ [1]: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.jvm/-jvm-overloads/
- Loading branch information
Showing
8 changed files
with
284 additions
and
7 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
98 changes: 98 additions & 0 deletions
98
tests/Xamarin.Android.Tools.Bytecode-Tests/JvmOverloadsConstructorTests.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
|
|
||
| using Xamarin.Android.Tools.Bytecode; | ||
|
|
||
| using NUnit.Framework; | ||
|
|
||
| using Assembly = System.Reflection.Assembly; | ||
|
|
||
| namespace Xamarin.Android.Tools.BytecodeTests { | ||
|
|
||
| [TestFixture] | ||
| public class JvmOverloadsConstructorTests : ClassFileFixture { | ||
|
|
||
| const string ClassFile = "JvmOverloadsConstructor.class"; | ||
| const string XmlFile = "JvmOverloadsConstructor.xml"; | ||
|
|
||
| [Test] | ||
| public void ClassFile_WithJavaType_class () | ||
| { | ||
| var c = LoadClassFile (ClassFile); | ||
| new ExpectedTypeDeclaration { | ||
| MajorVersion = 0x32, | ||
| MinorVersion = 0, | ||
| ConstantPoolCount = 59, | ||
| AccessFlags = ClassAccessFlags.Public | ClassAccessFlags.Final | ClassAccessFlags.Super, | ||
| FullName = "JvmOverloadsConstructor", | ||
| Superclass = new TypeInfo ("java/lang/Object", "Ljava/lang/Object;"), | ||
| Methods = { | ||
| new ExpectedMethodDeclaration { | ||
| Name = "<init>", | ||
| AccessFlags = MethodAccessFlags.Public, | ||
| ReturnDescriptor = "V", | ||
| Parameters = { | ||
| new ParameterInfo ("something", "LJvmOverloadsConstructor;", "LJvmOverloadsConstructor;"), | ||
| new ParameterInfo ("id", "I", "I"), | ||
| new ParameterInfo ("imageId", "I", "I"), | ||
| new ParameterInfo ("title", "Ljava/lang/String;", "Ljava/lang/String;"), | ||
| new ParameterInfo ("useDivider", "Z", "Z"), | ||
| }, | ||
| }, | ||
| new ExpectedMethodDeclaration { | ||
| Name = "<init>", | ||
| AccessFlags = MethodAccessFlags.Public | MethodAccessFlags.Synthetic, | ||
| ReturnDescriptor = "V", | ||
| Parameters = { | ||
| new ParameterInfo ("p0", "LJvmOverloadsConstructor;", "LJvmOverloadsConstructor;"), | ||
| new ParameterInfo ("p1", "I", "I"), | ||
| new ParameterInfo ("p2", "I", "I"), | ||
| new ParameterInfo ("p3", "Ljava/lang/String;", "Ljava/lang/String;"), | ||
| new ParameterInfo ("p4", "Z", "Z"), | ||
| new ParameterInfo ("p5", "I", "I"), | ||
| new ParameterInfo ("p6", "Lkotlin/jvm/internal/DefaultConstructorMarker;", "Lkotlin/jvm/internal/DefaultConstructorMarker;"), | ||
| }, | ||
| }, | ||
| new ExpectedMethodDeclaration { | ||
| Name = "<init>", | ||
| AccessFlags = MethodAccessFlags.Public, | ||
| ReturnDescriptor = "V", | ||
| Parameters = { | ||
| new ParameterInfo ("something", "LJvmOverloadsConstructor;", "LJvmOverloadsConstructor;"), | ||
| new ParameterInfo ("id", "I", "I"), | ||
| new ParameterInfo ("imageId", "I", "I"), | ||
| new ParameterInfo ("title", "Ljava/lang/String;", "Ljava/lang/String;"), | ||
| }, | ||
| }, | ||
| new ExpectedMethodDeclaration { | ||
| Name = "<init>", | ||
| AccessFlags = MethodAccessFlags.Public, | ||
| ReturnDescriptor = "V", | ||
| Parameters = { | ||
| new ParameterInfo ("something", "LJvmOverloadsConstructor;", "LJvmOverloadsConstructor;"), | ||
| new ParameterInfo ("id", "I", "I"), | ||
| new ParameterInfo ("title", "Ljava/lang/String;", "Ljava/lang/String;"), | ||
| }, | ||
| }, | ||
| new ExpectedMethodDeclaration { | ||
| Name = "<init>", | ||
| AccessFlags = MethodAccessFlags.Public, | ||
| ReturnDescriptor = "V", | ||
| Parameters = { | ||
| new ParameterInfo ("something", "LJvmOverloadsConstructor;", "LJvmOverloadsConstructor;"), | ||
| new ParameterInfo ("title", "Ljava/lang/String;", "Ljava/lang/String;"), | ||
| }, | ||
| }, | ||
| }, | ||
| }.Assert (c); | ||
| } | ||
|
|
||
| [Test] | ||
| public void XmlDeclaration_WithJavaType_class () | ||
| { | ||
| AssertXmlDeclaration (ClassFile, XmlFile); | ||
| } | ||
| } | ||
| } | ||
|
|
162 changes: 162 additions & 0 deletions
162
tests/Xamarin.Android.Tools.Bytecode-Tests/Resources/JvmOverloadsConstructor.xml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| <api | ||
| api-source="class-parse"> | ||
| <package | ||
| name="" | ||
| jni-name=""> | ||
| <class | ||
| abstract="false" | ||
| deprecated="not deprecated" | ||
| jni-extends="Ljava/lang/Object;" | ||
| extends="java.lang.Object" | ||
| extends-generic-aware="java.lang.Object" | ||
| final="true" | ||
| name="JvmOverloadsConstructor" | ||
| jni-signature="LJvmOverloadsConstructor;" | ||
| source-file-name="JvmOverloadsConstructor.kt" | ||
| static="false" | ||
| visibility="public"> | ||
| <constructor | ||
| deprecated="not deprecated" | ||
| final="false" | ||
| name="JvmOverloadsConstructor" | ||
| static="false" | ||
| visibility="public" | ||
| bridge="false" | ||
| synthetic="false" | ||
| jni-signature="(LJvmOverloadsConstructor;IILjava/lang/String;)V"> | ||
| <parameter | ||
| name="something" | ||
| type="JvmOverloadsConstructor" | ||
| jni-type="LJvmOverloadsConstructor;" | ||
| not-null="true" /> | ||
| <parameter | ||
| name="id" | ||
| type="int" | ||
| jni-type="I" /> | ||
| <parameter | ||
| name="imageId" | ||
| type="int" | ||
| jni-type="I" /> | ||
| <parameter | ||
| name="title" | ||
| type="java.lang.String" | ||
| jni-type="Ljava/lang/String;" | ||
| not-null="true" /> | ||
| </constructor> | ||
| <constructor | ||
| deprecated="not deprecated" | ||
| final="false" | ||
| name="JvmOverloadsConstructor" | ||
| static="false" | ||
| visibility="public" | ||
| bridge="false" | ||
| synthetic="false" | ||
| jni-signature="(LJvmOverloadsConstructor;IILjava/lang/String;Z)V"> | ||
| <parameter | ||
| name="something" | ||
| type="JvmOverloadsConstructor" | ||
| jni-type="LJvmOverloadsConstructor;" | ||
| not-null="true" /> | ||
| <parameter | ||
| name="id" | ||
| type="int" | ||
| jni-type="I" /> | ||
| <parameter | ||
| name="imageId" | ||
| type="int" | ||
| jni-type="I" /> | ||
| <parameter | ||
| name="title" | ||
| type="java.lang.String" | ||
| jni-type="Ljava/lang/String;" | ||
| not-null="true" /> | ||
| <parameter | ||
| name="useDivider" | ||
| type="boolean" | ||
| jni-type="Z" /> | ||
| </constructor> | ||
| <constructor | ||
| deprecated="not deprecated" | ||
| final="false" | ||
| name="JvmOverloadsConstructor" | ||
| static="false" | ||
| visibility="public" | ||
| bridge="false" | ||
| synthetic="true" | ||
| jni-signature="(LJvmOverloadsConstructor;IILjava/lang/String;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V"> | ||
| <parameter | ||
| name="p0" | ||
| type="JvmOverloadsConstructor" | ||
| jni-type="LJvmOverloadsConstructor;" /> | ||
| <parameter | ||
| name="p1" | ||
| type="int" | ||
| jni-type="I" /> | ||
| <parameter | ||
| name="p2" | ||
| type="int" | ||
| jni-type="I" /> | ||
| <parameter | ||
| name="p3" | ||
| type="java.lang.String" | ||
| jni-type="Ljava/lang/String;" /> | ||
| <parameter | ||
| name="p4" | ||
| type="boolean" | ||
| jni-type="Z" /> | ||
| <parameter | ||
| name="p5" | ||
| type="int" | ||
| jni-type="I" /> | ||
| <parameter | ||
| name="p6" | ||
| type="kotlin.jvm.internal.DefaultConstructorMarker" | ||
| jni-type="Lkotlin/jvm/internal/DefaultConstructorMarker;" /> | ||
| </constructor> | ||
| <constructor | ||
| deprecated="not deprecated" | ||
| final="false" | ||
| name="JvmOverloadsConstructor" | ||
| static="false" | ||
| visibility="public" | ||
| bridge="false" | ||
| synthetic="false" | ||
| jni-signature="(LJvmOverloadsConstructor;ILjava/lang/String;)V"> | ||
| <parameter | ||
| name="something" | ||
| type="JvmOverloadsConstructor" | ||
| jni-type="LJvmOverloadsConstructor;" | ||
| not-null="true" /> | ||
| <parameter | ||
| name="id" | ||
| type="int" | ||
| jni-type="I" /> | ||
| <parameter | ||
| name="title" | ||
| type="java.lang.String" | ||
| jni-type="Ljava/lang/String;" | ||
| not-null="true" /> | ||
| </constructor> | ||
| <constructor | ||
| deprecated="not deprecated" | ||
| final="false" | ||
| name="JvmOverloadsConstructor" | ||
| static="false" | ||
| visibility="public" | ||
| bridge="false" | ||
| synthetic="false" | ||
| jni-signature="(LJvmOverloadsConstructor;Ljava/lang/String;)V"> | ||
| <parameter | ||
| name="something" | ||
| type="JvmOverloadsConstructor" | ||
| jni-type="LJvmOverloadsConstructor;" | ||
| not-null="true" /> | ||
| <parameter | ||
| name="title" | ||
| type="java.lang.String" | ||
| jni-type="Ljava/lang/String;" | ||
| not-null="true" /> | ||
| </constructor> | ||
| </class> | ||
| </package> | ||
| </api> |
Binary file added
BIN
+1.74 KB
tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/JvmOverloadsConstructor.class
Binary file not shown.
11 changes: 11 additions & 0 deletions
11
tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/JvmOverloadsConstructor.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| class JvmOverloadsConstructor { | ||
| @JvmOverloads | ||
| constructor( | ||
| something : JvmOverloadsConstructor, | ||
| id: Int = 1, | ||
| imageId: Int = 2, | ||
| title: String, | ||
| useDivider: Boolean = false | ||
| ) { | ||
| } | ||
| } |