Skip to content

[Xamarin.Android.Tools.Bytecode] MethodParameters Attribute parsing #107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 3, 2017
Merged

[Xamarin.Android.Tools.Bytecode] MethodParameters Attribute parsing #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions src/Xamarin.Android.Tools.Bytecode/AttributeInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class AttributeInfo {
public const string Exceptions = "Exceptions";
public const string InnerClasses = "InnerClasses";
public const string LocalVariableTable = "LocalVariableTable";
public const string MethodParameters = "MethodParameters";
public const string Signature = "Signature";
public const string StackMapTable = "StackMapTable";

Expand Down Expand Up @@ -71,6 +72,7 @@ public string Name {
{ typeof (ExceptionsAttribute), Exceptions },
{ typeof (InnerClassesAttribute), InnerClasses },
{ typeof (LocalVariableTableAttribute), LocalVariableTable },
{ typeof (MethodParametersAttribute), MethodParameters },
{ typeof (SignatureAttribute), Signature },
{ typeof (StackMapTableAttribute), StackMapTable },
};
Expand Down Expand Up @@ -101,6 +103,7 @@ static AttributeInfo CreateAttribute (string name, ConstantPool constantPool, us
case Exceptions: return new ExceptionsAttribute (constantPool, nameIndex, stream);
case InnerClasses: return new InnerClassesAttribute (constantPool, nameIndex, stream);
case LocalVariableTable: return new LocalVariableTableAttribute (constantPool, nameIndex, stream);
case MethodParameters: return new MethodParametersAttribute (constantPool, nameIndex, stream);
case Signature: return new SignatureAttribute (constantPool, nameIndex, stream);
case StackMapTable: return new StackMapTableAttribute (constantPool, nameIndex, stream);
default: return new UnknownAttribute (constantPool, nameIndex, stream);
Expand Down Expand Up @@ -389,6 +392,74 @@ public override string ToString ()
}
}

public sealed class MethodParameterInfo {

internal ushort nameIndex;
public MethodParameterAccessFlags AccessFlags;

public ConstantPool ConstantPool {get; private set;}

public MethodParameterInfo (ConstantPool constantPool)
{
ConstantPool = constantPool;
}

public string Name {
get {
if (nameIndex == 0)
return null;
return ((ConstantPoolUtf8Item) ConstantPool [nameIndex]).Value;
}
}

public override string ToString ()
{
return string.Format ("MethodParameterInfo(Name='{0}', AccessFlags={1})", Name, AccessFlags);
}
}

// https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.24
public sealed class MethodParametersAttribute : AttributeInfo {

List<MethodParameterInfo> parameters;

public IList<MethodParameterInfo> ParameterInfo {
get {return parameters;}
}

public MethodParametersAttribute (ConstantPool constantPool, ushort nameIndex, Stream stream)
: base (constantPool, nameIndex, stream)
{
var length = stream.ReadNetworkUInt32 ();
var count = stream.ReadNetworkByte ();
Debug.Assert (
length == (checked ((count * 4) + 1)),
$"Unexpected `MethodParameters` length; expected {(count*4)+1}, got {length}!");
parameters = new List<MethodParameterInfo> (count);
for (int i = 0; i < count; ++i) {
var pNameIndex = stream.ReadNetworkUInt16 ();
var accessFlags = stream.ReadNetworkUInt16 ();
var p = new MethodParameterInfo (constantPool) {
nameIndex = pNameIndex,
AccessFlags = (MethodParameterAccessFlags) accessFlags,
};
parameters.Add (p);
}
}

public override string ToString ()
{
if (parameters.Count == 0)
return "MethodParametersAttribute()";
var sb = new StringBuilder ("MethodParametersAttribute(");
sb.Append (parameters [0]);
for (int i = 1; i < parameters.Count; ++i)
sb.Append (", ").Append (parameters [i]);
sb.Append (")");
return sb.ToString ();
}
}

// http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.9
public sealed class SignatureAttribute : AttributeInfo {

Expand Down
40 changes: 39 additions & 1 deletion src/Xamarin.Android.Tools.Bytecode/Methods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ public ParameterInfo[] GetParameters ()
parameters [i].Type.TypeSignature = sig.Parameters [i];
}
}
UpdateParametersFromMethodParametersAttribute (parameters);
return parameters;
}

Expand Down Expand Up @@ -204,6 +205,33 @@ public MethodTypeSignature GetSignature ()
? new MethodTypeSignature (signature.Signature)
: null;
}

void UpdateParametersFromMethodParametersAttribute (ParameterInfo[] parameters)
{
var methodParams = (MethodParametersAttribute) Attributes.SingleOrDefault (a => a.Name == AttributeInfo.MethodParameters);
if (methodParams == null)
return;

const MethodParameterAccessFlags OuterThis =
MethodParameterAccessFlags.Mandated | MethodParameterAccessFlags.Final;
var pinfo = methodParams.ParameterInfo;
int startIndex = 0;
while (startIndex < pinfo.Count &&
(pinfo [startIndex].AccessFlags & OuterThis) == OuterThis) {
startIndex++;
}
Debug.Assert (
parameters.Length == pinfo.Count - startIndex,
$"Unexpected number of method parameters; expected {parameters.Length}, got {pinfo.Count - startIndex}");
for (int i = 0; i < parameters.Length; ++i) {
var p = pinfo [i + startIndex];

parameters [i].AccessFlags = p.AccessFlags;
if (p != null) {
parameters [i].Name = p.Name;
}
}
}
}

public sealed class TypeInfo : IEquatable<TypeInfo> {
Expand Down Expand Up @@ -259,6 +287,8 @@ public sealed class ParameterInfo : IEquatable<ParameterInfo> {
public int Position;
public TypeInfo Type = new TypeInfo ();

public MethodParameterAccessFlags AccessFlags;

public ParameterInfo (string name = null, string binaryName = null, string typeSignature = null, int position = 0)
{
Name = name;
Expand Down Expand Up @@ -294,7 +324,7 @@ public bool Equals (ParameterInfo other)

public override string ToString ()
{
return string.Format ("ParameterInfo(Name={0}, Position={1}, Type={2}", Name, Position, Type);
return $"ParameterInfo(Name={Name}, Position={Position}, Type={Type}, AccessFlags={AccessFlags})";
}
}

Expand All @@ -313,4 +343,12 @@ public enum MethodAccessFlags {
Strict = 0x0800,
Synthetic = 0x1000,
}

[Flags]
public enum MethodParameterAccessFlags {
None,
Final = 0x0010,
Synthetic = 0x1000,
Mandated = 0x8000,
}
}
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Tools.Bytecode/Tests/IJavaInterface.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
synthetic="false"
jni-signature="(Ljava/lang/CharSequence;)Ljava/lang/Object;">
<parameter
name="p0"
name="value"
type="TString"
jni-type="TTString;" />
</method>
Expand Down
4 changes: 2 additions & 2 deletions src/Xamarin.Android.Tools.Bytecode/Tests/IJavaInterfaceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public void ClassFile_WithIJavaInterface_class ()
new ExpectedTypeDeclaration {
MajorVersion = 0x32,
MinorVersion = 0,
ConstantPoolCount = 21,
ConstantPoolCount = 23,
AccessFlags = ClassAccessFlags.Interface | ClassAccessFlags.Abstract,
FullName = "com/xamarin/IJavaInterface",
Superclass = new TypeInfo ("java/lang/Object", "Ljava/lang/Object;"),
Expand Down Expand Up @@ -61,7 +61,7 @@ public void ClassFile_WithIJavaInterface_class ()
ReturnDescriptor = "Ljava/lang/Object;",
ReturnGenericDescriptor = "TTReturn;",
Parameters = {
new ParameterInfo ("p0", "Ljava/lang/CharSequence;", "TTString;"),
new ParameterInfo ("value", "Ljava/lang/CharSequence;", "TTString;"),
},
},
}
Expand Down
6 changes: 3 additions & 3 deletions src/Xamarin.Android.Tools.Bytecode/Tests/JavaEnumTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public void ClassFileDescription ()
new ExpectedTypeDeclaration {
MajorVersion = 0x32,
MinorVersion = 0,
ConstantPoolCount = 50,
ConstantPoolCount = 53,
AccessFlags = ClassAccessFlags.Final | ClassAccessFlags.Super | ClassAccessFlags.Enum,
FullName = "com/xamarin/JavaEnum",
Superclass = new TypeInfo ("java/lang/Enum", "Ljava/lang/Enum<Lcom/xamarin/JavaEnum;>;"),
Expand Down Expand Up @@ -62,8 +62,8 @@ public void ClassFileDescription ()
AccessFlags = MethodAccessFlags.Private,
ReturnDescriptor = "V",
Parameters = {
new ParameterInfo ("p0", "Ljava/lang/String;", "Ljava/lang/String;"),
new ParameterInfo ("p1", "I", "I"),
new ParameterInfo ("$enum$name", "Ljava/lang/String;", "Ljava/lang/String;"),
new ParameterInfo ("$enum$ordinal", "I", "I"),
},
},
new ExpectedMethodDeclaration {
Expand Down
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Tools.Bytecode/Tests/JavaType$RNC$RPNC.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
synthetic="false"
jni-signature="(Ljava/lang/Object;)Ljava/lang/Object;">
<parameter
name="p0"
name="value"
type="E2"
jni-type="TE2;" />
</method>
Expand Down
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Tools.Bytecode/Tests/JavaType$RNC.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
synthetic="false"
jni-signature="(Ljava/lang/Object;)Ljava/lang/Object;">
<parameter
name="p0"
name="value"
type="E"
jni-type="TE;" />
</method>
Expand Down
4 changes: 2 additions & 2 deletions src/Xamarin.Android.Tools.Bytecode/Tests/JavaType.RNC.RPNCTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public void ClassFileDescription ()
new ExpectedTypeDeclaration {
MajorVersion = 0x32,
MinorVersion = 0,
ConstantPoolCount = 44,
ConstantPoolCount = 46,
AccessFlags = ClassAccessFlags.Public | ClassAccessFlags.Super | ClassAccessFlags.Abstract,
FullName = "com/xamarin/JavaType$RNC$RPNC",
Superclass = new TypeInfo ("java/lang/Object", "Ljava/lang/Object;"),
Expand Down Expand Up @@ -71,7 +71,7 @@ public void ClassFileDescription ()
ReturnDescriptor = "Ljava/lang/Object;",
ReturnGenericDescriptor = "TE3;",
Parameters = {
new ParameterInfo ("p0", "Ljava/lang/Object;", "TE2;"),
new ParameterInfo ("value", "Ljava/lang/Object;", "TE2;"),
},
},
}
Expand Down
4 changes: 2 additions & 2 deletions src/Xamarin.Android.Tools.Bytecode/Tests/JavaType.RNCTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public void ClassFileDescription ()
new ExpectedTypeDeclaration {
MajorVersion = 0x32,
MinorVersion = 0,
ConstantPoolCount = 42,
ConstantPoolCount = 44,
AccessFlags = ClassAccessFlags.Public | ClassAccessFlags.Super | ClassAccessFlags.Abstract,
FullName = "com/xamarin/JavaType$RNC",
Superclass = new TypeInfo ("java/lang/Object", "Ljava/lang/Object;"),
Expand Down Expand Up @@ -70,7 +70,7 @@ public void ClassFileDescription ()
ReturnDescriptor = "Ljava/lang/Object;",
ReturnGenericDescriptor = "TE2;",
Parameters = {
new ParameterInfo ("p0", "Ljava/lang/Object;", "TE;"),
new ParameterInfo ("value", "Ljava/lang/Object;", "TE;"),
},
},
}
Expand Down
4 changes: 2 additions & 2 deletions src/Xamarin.Android.Tools.Bytecode/Tests/JavaType.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
synthetic="true"
jni-signature="(Ljava/lang/Object;)I">
<parameter
name="p0"
name="value"
type="java.lang.Object"
jni-type="Ljava/lang/Object;" />
</method>
Expand Down Expand Up @@ -162,7 +162,7 @@
synthetic="true"
jni-signature="(Ljava/lang/CharSequence;)Ljava/lang/Object;">
<parameter
name="p0"
name="value"
type="java.lang.CharSequence"
jni-type="Ljava/lang/CharSequence;" />
</method>
Expand Down
8 changes: 4 additions & 4 deletions src/Xamarin.Android.Tools.Bytecode/Tests/JavaTypeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public void ClassFile_WithJavaType_class ()
new ExpectedTypeDeclaration {
MajorVersion = 0x32,
MinorVersion = 0,
ConstantPoolCount = 183,
ConstantPoolCount = 184,
AccessFlags = ClassAccessFlags.Public | ClassAccessFlags.Super,
FullName = "com/xamarin/JavaType",
Superclass = new TypeInfo ("java/lang/Object", "Ljava/lang/Object;"),
Expand Down Expand Up @@ -132,7 +132,7 @@ public void ClassFile_WithJavaType_class ()
Name = "STATIC_FINAL_STRING",
Descriptor = "Ljava/lang/String;",
AccessFlags = FieldAccessFlags.Public | FieldAccessFlags.Static | FieldAccessFlags.Final,
ConstantValue = "String(stringIndex=175 Utf8=\"Hello, \\\"embedded\0Nulls\" and \ud83d\udca9!\")",
ConstantValue = "String(stringIndex=176 Utf8=\"Hello, \\\"embedded\0Nulls\" and \ud83d\udca9!\")",
},
new ExpectedFieldDeclaration {
Name = "STATIC_FINAL_BOOL_FALSE",
Expand Down Expand Up @@ -309,15 +309,15 @@ public void ClassFile_WithJavaType_class ()
AccessFlags = MethodAccessFlags.Public | MethodAccessFlags.Bridge | MethodAccessFlags.Synthetic,
ReturnDescriptor = "I",
Parameters = {
new ParameterInfo ("p0", "Ljava/lang/Object;", "Ljava/lang/Object;"),
new ParameterInfo ("value", "Ljava/lang/Object;", "Ljava/lang/Object;"),
},
},
new ExpectedMethodDeclaration {
Name = "func",
AccessFlags = MethodAccessFlags.Public | MethodAccessFlags.Bridge | MethodAccessFlags.Synthetic,
ReturnDescriptor = "Ljava/lang/Object;",
Parameters = {
new ParameterInfo ("p0", "Ljava/lang/CharSequence;", "Ljava/lang/CharSequence;"),
new ParameterInfo ("value", "Ljava/lang/CharSequence;", "Ljava/lang/CharSequence;"),
},
},
new ExpectedMethodDeclaration {
Expand Down
11 changes: 9 additions & 2 deletions src/Xamarin.Android.Tools.Bytecode/Tests/Xamarin.Android.Tools.Bytecode-Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,13 @@
<Compile Include="ParameterFixupTests.cs" />
</ItemGroup>
<ItemGroup>
<TestJar Include="java\**\*.java" />
<TestJar
Include="java\**\*.java"
Exclude="java\java\util\Collection.java"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it excluded?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or better question: is it missing parameters?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's excluded/missing parameters so that the Xamarin.Android.Tools.BytecodeTests.ParameterFixupTests.XmlDeclaration_FixedUpFromApiXmlDocumentation test continues to work.

That test works by "fixing up" parameter names based on imported XML, but the import only works if the parameter names are "generated" (e.g. p1...). If Collection.java is compiled with javac -parameters, it will have correct parameter names, not generated names, which "breaks" the ParameterFixupTests.XmlDeclaration_FixedUpFromApiXmlDocumentation() tests.

This breakage might be acceptable -- i.e. we fixup the expected XML, and the test passes -- but by doing so, we're no longer testing/ensuring that parameter fixup continues to work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excluding the source means we are not testing that type either, right? Leaving that extra sources would just confuse future test coding and I don't see reason to not change the expected XML.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source isn't fully excluded. Collection.java is added to the @(TestJarNoParameters) item group. @(TestJar) is built with javac -parameters, while @(TestJarNoParameters) is built without javac -parameters.

The Collection.java source is still built.

Additionally, there are no other tests on Collection.class that check the names of parameters, so keeping it around as a file built without javac -parameters makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then the resulting state is that future test writers are likely lost when they need to add new test files because the build involves two different javac compilation steps? What if they are mutually dependent on the ones in @(TestJar) and @(TestJarNoParameters) ? I still don't think this is the way to go.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to suggestions for alternate strategies.

What if they are mutually dependent on the ones in @(TestJar) and @(TestJarNoParameters) ?

Try to avoid that? Or we could introduce a third item group which is compiled after those other item groups... It's solvable. Not necessarily "pretty" or "sane," but solvable.

/>
<TestJarNoParameters
Include="java\java\util\Collection.java"
/>
</ItemGroup>
<Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" />
<PropertyGroup>
Expand All @@ -67,7 +73,8 @@
</PropertyGroup>
<Target Name="BuildClasses" Inputs="@(TestJar)" Outputs="@(TestJar-&gt;'$(IntermediateOutputPath)classes\%(RecursiveDir)%(Filename).class')">
<MakeDir Directories="$(IntermediateOutputPath)classes" />
<Exec Command="&quot;javac&quot; -source 1.5 -target 1.6 -g -d &quot;$(IntermediateOutputPath)classes&quot; @(TestJar -&gt; '%(Identity)', ' ')" />
<Exec Command="&quot;javac&quot; -parameters -source 1.5 -target 1.6 -g -d &quot;$(IntermediateOutputPath)classes&quot; @(TestJar->'%(Identity)', ' ')" />
<Exec Command="&quot;javac&quot; -source 1.5 -target 1.6 -g -d &quot;$(IntermediateOutputPath)classes&quot; @(TestJarNoParameters->'%(Identity)', ' ')" />
</Target>
<ItemGroup>
<ProjectReference Include="..\Xamarin.Android.Tools.Bytecode.csproj">
Expand Down