Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] introduce BannedApiAnalyzers for Mono.C…
Browse files Browse the repository at this point in the history
…ecil

Context: https://github.com/dotnet/roslyn-analyzers/blob/68c643b4667c6808bd21910ef32f7e2f7bd776c5/src/Microsoft.CodeAnalysis.BannedApiAnalyzers/BannedApiAnalyzers.Help.md

In 526172b, we had some reasonable performance wins just by using
`TypeDefinitionCache`. Let's take this a step further by "banning"
usage of:

    Mono.Cecil.FieldReference.Resolve()
    Mono.Cecil.MethodReference.Resolve()
    Mono.Cecil.TypeReference.Resolve()

I applied this change to `Xamarin.Android.Build.Tasks.csproj` for now.

In various places, I had to pass `TypeDefinitionCache` around or use
an existing one.

In a future PR, I'll make a small change in xamarin/java.interop so we
can apply the same treatment to `Microsoft.Android.Sdk.ILLink.csproj`.
  • Loading branch information
jonathanpeppers committed Feb 9, 2024
1 parent b812506 commit 6754a50
Show file tree
Hide file tree
Showing 38 changed files with 183 additions and 208 deletions.
3 changes: 3 additions & 0 deletions build-tools/banned-apis/BannedSymbols.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
M:Mono.Cecil.FieldReference.Resolve();Use IMetadataResolver.Resolve() instead so the result is cached
M:Mono.Cecil.MethodReference.Resolve();Use IMetadataResolver.Resolve() instead so the result is cached
M:Mono.Cecil.TypeReference.Resolve();Use IMetadataResolver.Resolve() instead so the result is cached
9 changes: 9 additions & 0 deletions build-tools/banned-apis/banned-apis.targets
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project>
<PropertyGroup>
<WarningsAsErrors>RS0030</WarningsAsErrors>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.BannedApiAnalyzers" Version="3.3.4" PrivateAssets="All" />
<AdditionalFiles Include="$(MSBuildThisFileDirectory)BannedSymbols.txt" />
</ItemGroup>
</Project>
10 changes: 5 additions & 5 deletions src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ public void ProcessType (TypeDefinition type)
{
// If this isn't a JLO or IJavaObject implementer,
// then we don't need to MarkJavaObjects
if (!type.ImplementsIJavaObject ())
if (!type.ImplementsIJavaObject (cache))
return;

PreserveJavaObjectImplementation (type);

if (IsImplementor (type))
if (IsImplementor (type, cache))
PreserveImplementor (type);

// If a user overrode a method, we need to preserve it,
Expand Down Expand Up @@ -195,7 +195,7 @@ void PreserveInterfaces (TypeDefinition type)

foreach (var iface in type.Interfaces) {
var td = iface.InterfaceType.Resolve ();
if (!td.ImplementsIJavaPeerable ())
if (!td.ImplementsIJavaPeerable (cache))
continue;
Annotations.Mark (td);
}
Expand Down Expand Up @@ -302,9 +302,9 @@ void PreserveConstructors (TypeDefinition type, TypeDefinition helper)
PreserveMethod (type, ctor);
}

static bool IsImplementor (TypeDefinition type)
static bool IsImplementor (TypeDefinition type, IMetadataResolver cache)
{
return type.Name.EndsWith ("Implementor", StringComparison.Ordinal) && type.Inherits ("Java.Lang.Object");
return type.Name.EndsWith ("Implementor", StringComparison.Ordinal) && type.Inherits ("Java.Lang.Object", cache);
}

static bool IsUserType (TypeDefinition type)
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Android.Sdk.ILLink/PreserveApplications.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public void ProcessType (TypeDefinition type)
{
if (!IsActiveFor (type.Module.Assembly))
return;
if (!type.Inherits ("Android.App.Application"))
if (!type.Inherits ("Android.App.Application", cache))
return;

ProcessAttributeProvider (type);
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Android.Sdk.ILLink/PreserveJavaExceptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public override void Initialize (LinkContext context, MarkContext markContext)

public void ProcessType (TypeDefinition type)
{
if (type.IsJavaException ())
if (type.IsJavaException (cache))
PreserveJavaException (type);
}

Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Android.Sdk.ILLink/PreserveJavaInterfaces.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ void ProcessType (TypeDefinition type)
return;

// Mono.Android interfaces will always inherit IJavaObject
if (!type.ImplementsIJavaObject ())
if (!type.ImplementsIJavaObject (cache))
return;

foreach (MethodReference method in type.Methods)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,5 @@ public bool IsOverrideOfInterfaceMember
return Base.DeclaringType.IsInterface;
}
}

public TypeDefinition InterfaceType
{
get
{
if (!IsOverrideOfInterfaceMember)
return null;

if (MatchingInterfaceImplementation != null)
return MatchingInterfaceImplementation.InterfaceType.Resolve ();

return Base.DeclaringType;
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -453,67 +453,5 @@ public static ParameterDefinition GetParameter (this MethodBody self, int index)

return parameters [index];
}

public static bool Implements (this TypeReference self, string interfaceName)
{
if (interfaceName == null)
throw new ArgumentNullException ("interfaceName");
if (self == null)
return false;

TypeDefinition type = self.Resolve ();
if (type == null)
return false; // not enough information available

// special case, check if we implement ourselves
if (type.IsInterface && (type.FullName == interfaceName))
return true;

return Implements (type, interfaceName, (interfaceName.IndexOf ('`') >= 0));
}

public static bool Implements (TypeDefinition type, string interfaceName, bool generic)
{
while (type != null) {
// does the type implements it itself
if (type.HasInterfaces) {
foreach (var iface in type.Interfaces) {
string fullname = (generic) ? iface.InterfaceType.GetElementType ().FullName : iface.InterfaceType.FullName;
if (fullname == interfaceName)
return true;
//if not, then maybe one of its parent interfaces does
if (Implements (iface.InterfaceType.Resolve (), interfaceName, generic))
return true;
}
}

type = type.BaseType != null ? type.BaseType.Resolve () : null;
}
return false;
}

public static bool Inherits (this TypeReference self, string @namespace, string name)
{
if (@namespace == null)
throw new ArgumentNullException ("namespace");
if (name == null)
throw new ArgumentNullException ("name");
if (self == null)
return false;

TypeReference current = self.Resolve ();
while (current != null) {
if (current.Is (@namespace, name))
return true;
if (current.Is ("System", "Object"))
return false;

TypeDefinition td = current.Resolve ();
if (td == null)
return false; // could not resolve type
current = td.BaseType;
}
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,30 @@ static class Extensions {
const string IJavaPeerable = "Java.Interop.IJavaPeerable";
const string JavaThrowable = "Java.Lang.Throwable";

public static bool IsJavaObject (this TypeDefinition type)
public static bool IsJavaObject (this TypeDefinition type, IMetadataResolver resolver)
{
return type.Inherits (JavaObject);
return type.Inherits (JavaObject, resolver);
}

public static bool IsJavaException (this TypeDefinition type)
public static bool IsJavaException (this TypeDefinition type, IMetadataResolver resolver)
{
return type.Inherits (JavaThrowable);
return type.Inherits (JavaThrowable, resolver);
}

public static bool ImplementsIJavaObject (this TypeDefinition type)
public static bool ImplementsIJavaObject (this TypeDefinition type, IMetadataResolver resolver)
{
return type.Implements (IJavaObject);
return type.Implements (IJavaObject, resolver);
}

public static bool ImplementsIJavaPeerable (this TypeDefinition type)
public static bool ImplementsIJavaPeerable (this TypeDefinition type, IMetadataResolver resolver)
{
return type.Implements (IJavaPeerable);
return type.Implements (IJavaPeerable, resolver);
}

public static object GetSettableValue (this CustomAttributeArgument arg)
public static object GetSettableValue (this CustomAttributeArgument arg, IMetadataResolver cache)
{
TypeReference tr = arg.Value as TypeReference;
TypeDefinition td = tr != null ? tr.Resolve () : null;
TypeDefinition td = tr != null ? cache.Resolve (tr) : null;
return td != null ? td.FullName + "," + td.Module.Assembly.FullName : arg.Value;
}

Expand Down Expand Up @@ -119,25 +119,25 @@ public static TypeDefinition GetType (AssemblyDefinition assembly, string typeNa
return assembly.MainModule.GetType (typeName);
}

public static bool Implements (this TypeReference self, string interfaceName)
public static bool Implements (this TypeReference self, string interfaceName, IMetadataResolver resolver)
{
if (interfaceName == null)
throw new ArgumentNullException ("interfaceName");
if (self == null)
return false;

TypeDefinition type = self.Resolve ();
TypeDefinition type = resolver.Resolve (self);
if (type == null)
return false; // not enough information available

// special case, check if we implement ourselves
if (type.IsInterface && (type.FullName == interfaceName))
return true;

return Implements (type, interfaceName, (interfaceName.IndexOf ('`') >= 0));
return Implements (type, interfaceName, (interfaceName.IndexOf ('`') >= 0), resolver);
}

public static bool Implements (TypeDefinition type, string interfaceName, bool generic)
public static bool Implements (TypeDefinition type, string interfaceName, bool generic, IMetadataResolver resolver)
{
while (type != null) {
// does the type implements it itself
Expand All @@ -148,32 +148,36 @@ public static bool Implements (TypeDefinition type, string interfaceName, bool g
if (fullname == interfaceName)
return true;
//if not, then maybe one of its parent interfaces does
if (Implements (iface.Resolve (), interfaceName, generic))
if (Implements (resolver.Resolve (iface), interfaceName, generic, resolver))
return true;
}
}

type = type.BaseType != null ? type.BaseType.Resolve () : null;
if (type.BaseType != null) {
type = resolver.Resolve (type.BaseType);
} else {
type = null;
}
}
return false;
}

public static bool Inherits (this TypeReference self, string className)
public static bool Inherits (this TypeReference self, string className, IMetadataResolver resolver)
{
if (className == null)
throw new ArgumentNullException ("className");
if (self == null)
return false;

TypeReference current = self.Resolve ();
TypeReference current = resolver.Resolve (self);
while (current != null) {
string fullname = current.FullName;
if (fullname == className)
return true;
if (fullname == "System.Object")
return false;

TypeDefinition td = current.Resolve ();
TypeDefinition td = resolver.Resolve (current);
if (td == null)
return false; // could not resolve type
current = td.BaseType;
Expand Down Expand Up @@ -285,7 +289,7 @@ public static bool TryGetBaseOrInterfaceRegisterMember (this MethodDefinition me
if (iface.InterfaceType.IsGenericInstance)
continue;

var itype = iface.InterfaceType.Resolve ();
var itype = resolver.Resolve (iface.InterfaceType);
if (itype == null || !itype.HasMethods)
continue;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ public class FixLegacyResourceDesignerStep : LinkDesignerBase
internal const string DesignerAssemblyName = "_Microsoft.Android.Resource.Designer";
internal const string DesignerAssemblyNamespace = "_Microsoft.Android.Resource.Designer";

#if !ILLINK
public FixLegacyResourceDesignerStep (IMetadataResolver cache) : base (cache) { }
#endif

bool designerLoaded = false;
AssemblyDefinition designerAssembly = null;
TypeDefinition designerType = null;
Expand Down Expand Up @@ -93,7 +97,7 @@ internal override bool ProcessAssemblyDesigner (AssemblyDefinition assembly)

LogMessage ($" Adding reference {designerAssembly.Name.Name}.");
assembly.MainModule.AssemblyReferences.Add (designerAssembly.Name);
var importedDesignerType = assembly.MainModule.ImportReference (designerType.Resolve ());
var importedDesignerType = assembly.MainModule.ImportReference (Cache.Resolve (designerType));

LogMessage ($" FixupAssemblyTypes {assembly.Name.Name}.");
// now replace all ldsfld with a call to the property get_ method.
Expand Down Expand Up @@ -150,7 +154,7 @@ string GetFixupKey (Instruction instruction, string designerFullName)
(fieldRef.DeclaringType?.ToString()?.Contains (".Resource/") ?? false)) {
var canResolve = false;
try {
var resolved = fieldRef.Resolve ();
var resolved = Cache.Resolve (fieldRef);
canResolve = resolved != null;
} catch (Exception) {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,19 @@


namespace MonoDroid.Tuner {
public abstract class LinkDesignerBase : BaseStep {
public abstract class LinkDesignerBase : BaseStep
{
#if ILLINK
protected IMetadataResolver Cache => Context;
#else // !ILLINK
public LinkDesignerBase (IMetadataResolver cache)
{
Cache = cache;
}

protected IMetadataResolver Cache { get; private set; }
#endif // !ILLINK

public virtual void LogMessage (string message)
{
Context.LogMessage (message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ public class RemoveResourceDesignerStep : LinkDesignerBase
CustomAttribute mainDesignerAttribute;
Dictionary<string, int> designerConstants;
Regex opCodeRegex = new Regex (@"([\w]+): ([\w]+) ([\w.]+) ([\w:./]+)");

#if !ILLINK
public RemoveResourceDesignerStep (IMetadataResolver cache) : base (cache) { }
#endif

protected override void LoadDesigner ()
{
if (mainAssembly != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ partial class ActivityAttribute {
TypeDefinition type;
ICollection<string> specified;

public static ActivityAttribute FromTypeDefinition (TypeDefinition type)
public static ActivityAttribute FromTypeDefinition (TypeDefinition type, TypeDefinitionCache cache)
{
CustomAttribute attr = type.GetCustomAttributes ("Android.App.ActivityAttribute")
.SingleOrDefault ();
Expand All @@ -343,7 +343,7 @@ public static ActivityAttribute FromTypeDefinition (TypeDefinition type)
ActivityAttribute self = new ActivityAttribute () {
type = type,
};
self.specified = mapping.Load (self, attr);
self.specified = mapping.Load (self, attr, cache);
return self;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ partial class ApplicationAttribute {

ICollection<string> specified;

public static ApplicationAttribute FromCustomAttributeProvider (ICustomAttributeProvider provider)
public static ApplicationAttribute FromCustomAttributeProvider (ICustomAttributeProvider provider, TypeDefinitionCache cache)
{
CustomAttribute attr = provider.GetCustomAttributes ("Android.App.ApplicationAttribute")
.SingleOrDefault ();
Expand All @@ -249,7 +249,7 @@ public static ApplicationAttribute FromCustomAttributeProvider (ICustomAttribute
ApplicationAttribute self = new ApplicationAttribute () {
provider = provider,
};
self.specified = mapping.Load (self, attr);
self.specified = mapping.Load (self, attr, cache);
if (provider is TypeDefinition) {
self.specified.Add ("Name");
}
Expand Down
Loading

0 comments on commit 6754a50

Please sign in to comment.