Skip to content

Commit 4b266fa

Browse files
jonpryorjonpryor
authored
[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/
1 parent b00e644 commit 4b266fa

File tree

8 files changed

+284
-7
lines changed

8 files changed

+284
-7
lines changed

‎src/Xamarin.Android.Tools.Bytecode/AttributeInfo.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ public string Descriptor {
429429

430430
public override string ToString ()
431431
{
432-
return string.Format ("LocalVariableTableEntry(Name='{0}', Descriptor='{1}')", Name, Descriptor);
432+
return $"LocalVariableTableEntry(Name='{Name}', Descriptor='{Descriptor}', StartPC={StartPC}, Index={Index})";
433433
}
434434
}
435435

‎src/Xamarin.Android.Tools.Bytecode/ClassFile.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ public string PackageName {
9898
}
9999
}
100100

101+
public string FullJniName => "L" + ThisClass.Name.Value + ";";
102+
101103
public string SourceFileName {
102104
get {
103105
var sourceFile = Attributes.Get<SourceFileAttribute> ();

‎src/Xamarin.Android.Tools.Bytecode/Methods.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,17 +90,21 @@ public ParameterInfo[] GetParameters ()
9090
if (locals != null) {
9191
var names = locals.LocalVariables.Where (p => p.StartPC == 0).ToList ();
9292
int start = 0;
93-
if ((AccessFlags & MethodAccessFlags.Static) == 0)
93+
if (names.Count != parameters.Length &&
94+
!AccessFlags.HasFlag (MethodAccessFlags.Static) &&
95+
names.Count > start &&
96+
names [start].Descriptor == DeclaringType.FullJniName) {
9497
start++; // skip `this` parameter
98+
}
9599
if (!DeclaringType.IsStatic &&
96100
names.Count > start &&
97101
(parameters.Length == 0 || parameters [0].Type.BinaryName != names [start].Descriptor)) {
98102
start++; // JDK 8?
99103
}
100-
if (((AccessFlags & MethodAccessFlags.Synthetic) != MethodAccessFlags.Synthetic) &&
104+
if (!AccessFlags.HasFlag (MethodAccessFlags.Synthetic) &&
101105
((names.Count - start) != parameters.Length) &&
102106
!enumCtor) {
103-
Log.Warning (1,"class-parse: warning: method {0}.{1}{2}: " +
107+
Log.Debug ("class-parse: method {0}.{1}{2}: " +
104108
"Local variables array has {3} entries ('{4}'); descriptor has {5} entries!",
105109
DeclaringType.ThisClass.Name.Value, Name, Descriptor,
106110
names.Count - start,
@@ -111,7 +115,7 @@ public ParameterInfo[] GetParameters ()
111115
for (int i = 0; i < max; ++i) {
112116
parameters [i].Name = names [start+i].Name;
113117
if (parameters [i].Type.BinaryName != names [start + i].Descriptor) {
114-
Log.Warning (1, "class-parse: warning: method {0}.{1}{2}: " +
118+
Log.Debug ("class-parse: method {0}.{1}{2}: " +
115119
"Local variable type descriptor mismatch! Got '{3}'; expected '{4}'.",
116120
DeclaringType.ThisClass.Name.Value, Name, Descriptor,
117121
parameters [i].Type.BinaryName,
@@ -122,7 +126,7 @@ public ParameterInfo[] GetParameters ()
122126
var sig = GetSignature ();
123127
if (sig != null) {
124128
if ((sig.Parameters.Count != parameters.Length) && !enumCtor) {
125-
Log.Warning (1,"class-parse: warning: method {0}.{1}{2}: " +
129+
Log.Debug ("class-parse: method {0}.{1}{2}: " +
126130
"Signature ('{3}') has {4} entries; Descriptor '{5}' has {6} entries!",
127131
DeclaringType.ThisClass.Name.Value, Name, Descriptor,
128132
Attributes.Get<SignatureAttribute>(),

‎src/Xamarin.Android.Tools.Bytecode/XmlClassDeclarationBuilder.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public XElement ToXElement ()
3939
GetExtendsGenericAware (),
4040
new XAttribute ("final", (classFile.AccessFlags & ClassAccessFlags.Final) != 0),
4141
new XAttribute ("name", GetThisClassName ()),
42-
new XAttribute ("jni-signature", "L" + classFile.ThisClass.Name.Value + ";"),
42+
new XAttribute ("jni-signature", classFile.FullJniName),
4343
GetSourceFile (),
4444
new XAttribute ("static", classFile.IsStatic),
4545
new XAttribute ("visibility", GetVisibility (classFile.Visibility)),

‎tests/Xamarin.Android.Tools.Bytecode-Tests/JvmOverloadsConstructorTests.cs

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
5+
using Xamarin.Android.Tools.Bytecode;
6+
7+
using NUnit.Framework;
8+
9+
using Assembly = System.Reflection.Assembly;
10+
11+
namespace Xamarin.Android.Tools.BytecodeTests {
12+
13+
[TestFixture]
14+
public class JvmOverloadsConstructorTests : ClassFileFixture {
15+
16+
const string ClassFile = "JvmOverloadsConstructor.class";
17+
const string XmlFile = "JvmOverloadsConstructor.xml";
18+
19+
[Test]
20+
public void ClassFile_WithJavaType_class ()
21+
{
22+
var c = LoadClassFile (ClassFile);
23+
new ExpectedTypeDeclaration {
24+
MajorVersion = 0x32,
25+
MinorVersion = 0,
26+
ConstantPoolCount = 59,
27+
AccessFlags = ClassAccessFlags.Public | ClassAccessFlags.Final | ClassAccessFlags.Super,
28+
FullName = "JvmOverloadsConstructor",
29+
Superclass = new TypeInfo ("java/lang/Object", "Ljava/lang/Object;"),
30+
Methods = {
31+
new ExpectedMethodDeclaration {
32+
Name = "<init>",
33+
AccessFlags = MethodAccessFlags.Public,
34+
ReturnDescriptor = "V",
35+
Parameters = {
36+
new ParameterInfo ("something", "LJvmOverloadsConstructor;", "LJvmOverloadsConstructor;"),
37+
new ParameterInfo ("id", "I", "I"),
38+
new ParameterInfo ("imageId", "I", "I"),
39+
new ParameterInfo ("title", "Ljava/lang/String;", "Ljava/lang/String;"),
40+
new ParameterInfo ("useDivider", "Z", "Z"),
41+
},
42+
},
43+
new ExpectedMethodDeclaration {
44+
Name = "<init>",
45+
AccessFlags = MethodAccessFlags.Public | MethodAccessFlags.Synthetic,
46+
ReturnDescriptor = "V",
47+
Parameters = {
48+
new ParameterInfo ("p0", "LJvmOverloadsConstructor;", "LJvmOverloadsConstructor;"),
49+
new ParameterInfo ("p1", "I", "I"),
50+
new ParameterInfo ("p2", "I", "I"),
51+
new ParameterInfo ("p3", "Ljava/lang/String;", "Ljava/lang/String;"),
52+
new ParameterInfo ("p4", "Z", "Z"),
53+
new ParameterInfo ("p5", "I", "I"),
54+
new ParameterInfo ("p6", "Lkotlin/jvm/internal/DefaultConstructorMarker;", "Lkotlin/jvm/internal/DefaultConstructorMarker;"),
55+
},
56+
},
57+
new ExpectedMethodDeclaration {
58+
Name = "<init>",
59+
AccessFlags = MethodAccessFlags.Public,
60+
ReturnDescriptor = "V",
61+
Parameters = {
62+
new ParameterInfo ("something", "LJvmOverloadsConstructor;", "LJvmOverloadsConstructor;"),
63+
new ParameterInfo ("id", "I", "I"),
64+
new ParameterInfo ("imageId", "I", "I"),
65+
new ParameterInfo ("title", "Ljava/lang/String;", "Ljava/lang/String;"),
66+
},
67+
},
68+
new ExpectedMethodDeclaration {
69+
Name = "<init>",
70+
AccessFlags = MethodAccessFlags.Public,
71+
ReturnDescriptor = "V",
72+
Parameters = {
73+
new ParameterInfo ("something", "LJvmOverloadsConstructor;", "LJvmOverloadsConstructor;"),
74+
new ParameterInfo ("id", "I", "I"),
75+
new ParameterInfo ("title", "Ljava/lang/String;", "Ljava/lang/String;"),
76+
},
77+
},
78+
new ExpectedMethodDeclaration {
79+
Name = "<init>",
80+
AccessFlags = MethodAccessFlags.Public,
81+
ReturnDescriptor = "V",
82+
Parameters = {
83+
new ParameterInfo ("something", "LJvmOverloadsConstructor;", "LJvmOverloadsConstructor;"),
84+
new ParameterInfo ("title", "Ljava/lang/String;", "Ljava/lang/String;"),
85+
},
86+
},
87+
},
88+
}.Assert (c);
89+
}
90+
91+
[Test]
92+
public void XmlDeclaration_WithJavaType_class ()
93+
{
94+
AssertXmlDeclaration (ClassFile, XmlFile);
95+
}
96+
}
97+
}
98+

‎tests/Xamarin.Android.Tools.Bytecode-Tests/Resources/JvmOverloadsConstructor.xml

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
<api
2+
api-source="class-parse">
3+
<package
4+
name=""
5+
jni-name="">
6+
<class
7+
abstract="false"
8+
deprecated="not deprecated"
9+
jni-extends="Ljava/lang/Object;"
10+
extends="java.lang.Object"
11+
extends-generic-aware="java.lang.Object"
12+
final="true"
13+
name="JvmOverloadsConstructor"
14+
jni-signature="LJvmOverloadsConstructor;"
15+
source-file-name="JvmOverloadsConstructor.kt"
16+
static="false"
17+
visibility="public">
18+
<constructor
19+
deprecated="not deprecated"
20+
final="false"
21+
name="JvmOverloadsConstructor"
22+
static="false"
23+
visibility="public"
24+
bridge="false"
25+
synthetic="false"
26+
jni-signature="(LJvmOverloadsConstructor;IILjava/lang/String;)V">
27+
<parameter
28+
name="something"
29+
type="JvmOverloadsConstructor"
30+
jni-type="LJvmOverloadsConstructor;"
31+
not-null="true" />
32+
<parameter
33+
name="id"
34+
type="int"
35+
jni-type="I" />
36+
<parameter
37+
name="imageId"
38+
type="int"
39+
jni-type="I" />
40+
<parameter
41+
name="title"
42+
type="java.lang.String"
43+
jni-type="Ljava/lang/String;"
44+
not-null="true" />
45+
</constructor>
46+
<constructor
47+
deprecated="not deprecated"
48+
final="false"
49+
name="JvmOverloadsConstructor"
50+
static="false"
51+
visibility="public"
52+
bridge="false"
53+
synthetic="false"
54+
jni-signature="(LJvmOverloadsConstructor;IILjava/lang/String;Z)V">
55+
<parameter
56+
name="something"
57+
type="JvmOverloadsConstructor"
58+
jni-type="LJvmOverloadsConstructor;"
59+
not-null="true" />
60+
<parameter
61+
name="id"
62+
type="int"
63+
jni-type="I" />
64+
<parameter
65+
name="imageId"
66+
type="int"
67+
jni-type="I" />
68+
<parameter
69+
name="title"
70+
type="java.lang.String"
71+
jni-type="Ljava/lang/String;"
72+
not-null="true" />
73+
<parameter
74+
name="useDivider"
75+
type="boolean"
76+
jni-type="Z" />
77+
</constructor>
78+
<constructor
79+
deprecated="not deprecated"
80+
final="false"
81+
name="JvmOverloadsConstructor"
82+
static="false"
83+
visibility="public"
84+
bridge="false"
85+
synthetic="true"
86+
jni-signature="(LJvmOverloadsConstructor;IILjava/lang/String;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V">
87+
<parameter
88+
name="p0"
89+
type="JvmOverloadsConstructor"
90+
jni-type="LJvmOverloadsConstructor;" />
91+
<parameter
92+
name="p1"
93+
type="int"
94+
jni-type="I" />
95+
<parameter
96+
name="p2"
97+
type="int"
98+
jni-type="I" />
99+
<parameter
100+
name="p3"
101+
type="java.lang.String"
102+
jni-type="Ljava/lang/String;" />
103+
<parameter
104+
name="p4"
105+
type="boolean"
106+
jni-type="Z" />
107+
<parameter
108+
name="p5"
109+
type="int"
110+
jni-type="I" />
111+
<parameter
112+
name="p6"
113+
type="kotlin.jvm.internal.DefaultConstructorMarker"
114+
jni-type="Lkotlin/jvm/internal/DefaultConstructorMarker;" />
115+
</constructor>
116+
<constructor
117+
deprecated="not deprecated"
118+
final="false"
119+
name="JvmOverloadsConstructor"
120+
static="false"
121+
visibility="public"
122+
bridge="false"
123+
synthetic="false"
124+
jni-signature="(LJvmOverloadsConstructor;ILjava/lang/String;)V">
125+
<parameter
126+
name="something"
127+
type="JvmOverloadsConstructor"
128+
jni-type="LJvmOverloadsConstructor;"
129+
not-null="true" />
130+
<parameter
131+
name="id"
132+
type="int"
133+
jni-type="I" />
134+
<parameter
135+
name="title"
136+
type="java.lang.String"
137+
jni-type="Ljava/lang/String;"
138+
not-null="true" />
139+
</constructor>
140+
<constructor
141+
deprecated="not deprecated"
142+
final="false"
143+
name="JvmOverloadsConstructor"
144+
static="false"
145+
visibility="public"
146+
bridge="false"
147+
synthetic="false"
148+
jni-signature="(LJvmOverloadsConstructor;Ljava/lang/String;)V">
149+
<parameter
150+
name="something"
151+
type="JvmOverloadsConstructor"
152+
jni-type="LJvmOverloadsConstructor;"
153+
not-null="true" />
154+
<parameter
155+
name="title"
156+
type="java.lang.String"
157+
jni-type="Ljava/lang/String;"
158+
not-null="true" />
159+
</constructor>
160+
</class>
161+
</package>
162+
</api>

‎tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/JvmOverloadsConstructor.class

1.74 KB
Binary file not shown.

‎tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/JvmOverloadsConstructor.kt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
class JvmOverloadsConstructor {
2+
@JvmOverloads
3+
constructor(
4+
something : JvmOverloadsConstructor,
5+
id: Int = 1,
6+
imageId: Int = 2,
7+
title: String,
8+
useDivider: Boolean = false
9+
) {
10+
}
11+
}

0 commit comments

Comments
 (0)