Skip to content

Commit

Permalink
Add new Xml Type parser (dotnet#82145)
Browse files Browse the repository at this point in the history
Use new type parser for xml processing
Add tests in nativeAOT
Port XML testing changes from NativeAOT to illink
Port logic to distinguish who needs to Kept an element in illink
  • Loading branch information
tlakollo authored Feb 15, 2023
1 parent 4ddfd7b commit 1c4cc74
Show file tree
Hide file tree
Showing 15 changed files with 185 additions and 26 deletions.
58 changes: 53 additions & 5 deletions src/coreclr/tools/Common/Compiler/ProcessLinkerXmlBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,7 @@ protected virtual void ProcessTypes(ModuleDesc assembly, XPathNavigator nav, boo

// TODO: Process exported types

// TODO: Semantics differ and xml format is cecil specific, therefore they are discrepancies on things like nested types
// for now just hack replacing / for + to support basic resolving of nested types
// https://github.com/dotnet/runtime/issues/73083
fullname = fullname.Replace("/", "+");
TypeDesc type = CustomAttributeTypeNameParser.GetTypeByCustomAttributeTypeName(assembly, fullname, throwIfNotFound: false);
TypeDesc? type = CecilCompatibleTypeParser.GetType(assembly, fullname);

if (type == null)
{
Expand Down Expand Up @@ -753,4 +749,56 @@ public bool TryConvertValue(string value, TypeDesc type, out object? result)
#endif
}
}

public class CecilCompatibleTypeParser
{
public static TypeDesc? GetType(ModuleDesc assembly, string fullName)
{
Debug.Assert(!string.IsNullOrEmpty(fullName));
var position = fullName.IndexOf('/');
if (position > 0)
return GetNestedType(assembly, fullName);
string @namespace, name;
SplitFullName(fullName, out @namespace, out name);

return assembly.GetType(@namespace, name, throwIfNotFound: false);
}

private static MetadataType? GetNestedType(ModuleDesc assembly, string fullName)
{
var names = fullName.Split('/');
var type = GetType(assembly, names[0]);

if (type == null)
return null;

MetadataType typeReference = (MetadataType)type;
for (int i = 1; i < names.Length; i++)
{
var nested_type = typeReference.GetNestedType(names[i]);
if (nested_type == null)
return null;

typeReference = nested_type;
}

return typeReference;
}

public static void SplitFullName(string fullName, out string @namespace, out string name)
{
var last_dot = fullName.LastIndexOf('.');

if (last_dot == -1)
{
@namespace = string.Empty;
name = fullName;
}
else
{
@namespace = fullName.Substring(0, last_dot);
name = fullName.Substring(last_dot + 1);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ namespace Mono.Linker.Tests.Cases.LinkXml
[ExpectedWarning ("IL2017", "NonExistentProperty", "TypeWithNoProperties", FileName = "LinkXmlErrorCases.xml", SourceLine = 21, SourceColumn = 8)]
[ExpectedWarning ("IL2018", "SetOnlyProperty", "TypeWithProperties", FileName = "LinkXmlErrorCases.xml", SourceLine = 25, SourceColumn = 8)]
[ExpectedWarning ("IL2019", "GetOnlyProperty", "TypeWithProperties", FileName = "LinkXmlErrorCases.xml", SourceLine = 26, SourceColumn = 8)]
[ExpectedWarning ("IL2025", "Method", FileName = "LinkXmlErrorCases.xml", SourceLine = 39, SourceColumn = 8, ProducedBy = ProducedBy.Trimmer)]
[ExpectedWarning ("IL2025", "Event", FileName = "LinkXmlErrorCases.xml", SourceLine = 40, SourceColumn = 8, ProducedBy = ProducedBy.Trimmer)]
[ExpectedWarning ("IL2025", "Field", FileName = "LinkXmlErrorCases.xml", SourceLine = 41, SourceColumn = 8, ProducedBy = ProducedBy.Trimmer)]
[ExpectedWarning ("IL2025", "Property", FileName = "LinkXmlErrorCases.xml", SourceLine = 42, SourceColumn = 8, ProducedBy = ProducedBy.Trimmer)]
// NativeAOT doesn't support wildcard * and will skip usages of it, including if they would warn
// https://github.com/dotnet/runtime/issues/80466
[ExpectedWarning ("IL2100", FileName = "LinkXmlErrorCases.xml", SourceLine = 50, SourceColumn = 4, ProducedBy = ProducedBy.Trimmer)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,20 @@ public static void Main ()
class UnusedTypePreservedByLinkXmlIsKeptUnusedType
{
}
}

[Kept]
public class UnusedGenericTypePreservedByLinkXmlIsKept<T>
{
[Kept]
public class UnusedNestedTypePreservedByLinkXmlIsKept { }

[Kept]
public class UnusedNestedGenericTypePreservedByLinkXmlIsKept<U, V>
{
[Kept]
public class UnusedNestedNestedGenericTypePreservedByLinkXmlIsKept<W> { }
}

public class UnusedNestedTypeNotPreserved { }
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
<linker>
<assembly fullname="test, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null">
<type fullname="Mono.Linker.Tests.Cases.LinkXml.UnusedTypePreservedByLinkXmlIsKeptUnusedType" />
<type fullname="Mono.Linker.Tests.Cases.LinkXml.UnusedGenericTypePreservedByLinkXmlIsKept`1" preserve="none" />
<type fullname="Mono.Linker.Tests.Cases.LinkXml.UnusedGenericTypePreservedByLinkXmlIsKept`1/UnusedNestedTypePreservedByLinkXmlIsKept" preserve="none" />
<type fullname="Mono.Linker.Tests.Cases.LinkXml.UnusedGenericTypePreservedByLinkXmlIsKept`1/UnusedNestedGenericTypePreservedByLinkXmlIsKept`2" preserve="none" />
<type fullname="Mono.Linker.Tests.Cases.LinkXml.UnusedGenericTypePreservedByLinkXmlIsKept`1/UnusedNestedGenericTypePreservedByLinkXmlIsKept`2/UnusedNestedNestedGenericTypePreservedByLinkXmlIsKept`1" preserve="none" />
</assembly>
</linker>
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,11 @@ namespace Mono.Linker.Tests.Cases.Expectations.Assertions
[AttributeUsage (AttributeTargets.All, Inherited = false)]
public class KeptAttribute : BaseExpectedLinkedBehaviorAttribute
{
/// <summary>
/// By default the target should be kept by all platforms
/// This property can override that by setting only the platforms
/// which are expected to keep the target.
/// </summary>
public ProducedBy By { get; set; } = ProducedBy.TrimmerAnalyzerAndNativeAot;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ namespace Mono.Linker.Tests.Cases.LinkXml
[ExpectedWarning ("IL2017", "NonExistentProperty", "TypeWithNoProperties", FileName = "LinkXmlErrorCases.xml", SourceLine = 21, SourceColumn = 8)]
[ExpectedWarning ("IL2018", "SetOnlyProperty", "TypeWithProperties", FileName = "LinkXmlErrorCases.xml", SourceLine = 25, SourceColumn = 8)]
[ExpectedWarning ("IL2019", "GetOnlyProperty", "TypeWithProperties", FileName = "LinkXmlErrorCases.xml", SourceLine = 26, SourceColumn = 8)]

[ExpectedWarning ("IL2025", "Method", FileName = "LinkXmlErrorCases.xml", SourceLine = 39, SourceColumn = 8)]
[ExpectedWarning ("IL2025", "Event", FileName = "LinkXmlErrorCases.xml", SourceLine = 40, SourceColumn = 8)]
[ExpectedWarning ("IL2025", "Field", FileName = "LinkXmlErrorCases.xml", SourceLine = 41, SourceColumn = 8)]
[ExpectedWarning ("IL2025", "Property", FileName = "LinkXmlErrorCases.xml", SourceLine = 42, SourceColumn = 8)]
[ExpectedWarning ("IL2100", FileName = "LinkXmlErrorCases.xml", SourceLine = 50, SourceColumn = 4)]
[ExpectedWarning ("IL2025", "Method", FileName = "LinkXmlErrorCases.xml", SourceLine = 39, SourceColumn = 8, ProducedBy = ProducedBy.Trimmer)]
[ExpectedWarning ("IL2025", "Event", FileName = "LinkXmlErrorCases.xml", SourceLine = 40, SourceColumn = 8, ProducedBy = ProducedBy.Trimmer)]
[ExpectedWarning ("IL2025", "Field", FileName = "LinkXmlErrorCases.xml", SourceLine = 41, SourceColumn = 8, ProducedBy = ProducedBy.Trimmer)]
[ExpectedWarning ("IL2025", "Property", FileName = "LinkXmlErrorCases.xml", SourceLine = 42, SourceColumn = 8, ProducedBy = ProducedBy.Trimmer)]
// NativeAOT doesn't support wildcard * and will skip usages of it, including if they would warn
// https://github.com/dotnet/runtime/issues/80466
[ExpectedWarning ("IL2100", FileName = "LinkXmlErrorCases.xml", SourceLine = 50, SourceColumn = 4, ProducedBy = ProducedBy.Trimmer)]
class LinkXmlErrorCases
{
public static void Main ()
Expand Down Expand Up @@ -76,4 +77,4 @@ public void Method () { }
public int Property { [Kept] get; [Kept] set; }
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class Unused : IFoo<int>, IFoo<string>, IFoo<Cat>, IFoo2<int>, IFoo3<int, string
int IFoo<IFoo<int>>.Bar { get; set; }
}

[Kept (By = ProducedBy.NativeAot)]
interface IDog
{
string Name { get; set; }
Expand All @@ -52,16 +53,19 @@ interface IFoo<T>
int Bar { get; set; }
}

[Kept (By = ProducedBy.NativeAot)]
interface IFoo2<T>
{
int Bar2 { get; set; }
}

[Kept (By = ProducedBy.NativeAot)]
interface IFoo3<T, K, J>
{
int Bar3 { get; set; }
}

[Kept (By = ProducedBy.NativeAot)]
class Cat
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public static void Main ()
{
}

[Kept (By = ProducedBy.NativeAot)]
interface IFoo
{
}
Expand All @@ -19,4 +20,4 @@ class Bar : IFoo
{
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,12 @@ private void NotPreservedMethod ()
{
}

[Kept]
// NativeAOT fails to translate Cecil generic methods
// https://github.com/dotnet/runtime/issues/80462
[Kept (By = ProducedBy.Trimmer)]
private void PreservedMethod5<T> (T arg)
{
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Mono.Linker.Tests.Cases.Expectations.Assertions;
using Mono.Linker.Tests.Cases.Expectations.Metadata;

namespace Mono.Linker.Tests.Cases.LinkXml
Expand All @@ -9,8 +10,12 @@ public static void Main ()
{
}

// NativeAOT should generate conditional dependencies for the required tag
// https://github.com/dotnet/runtime/issues/80464
[Kept (By = ProducedBy.NativeAot)]
[KeptMember (".ctor()", By = ProducedBy.NativeAot)]
class Unused
{
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<linker>
<assembly fullname="test, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null">
<assembly fullname="test, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null">
<type fullname="Mono.Linker.Tests.Cases.LinkXml.UnusedNonRequiredTypeIsRemoved/Unused" preserve="all" required="0"/>
</assembly>
</linker>
</linker>
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,20 @@ public static void Main ()
class UnusedTypePreservedByLinkXmlIsKeptUnusedType
{
}
}

[Kept]
public class UnusedGenericTypePreservedByLinkXmlIsKept<T>
{
[Kept]
public class UnusedNestedTypePreservedByLinkXmlIsKept { }

[Kept]
public class UnusedNestedGenericTypePreservedByLinkXmlIsKept<U, V>
{
[Kept]
public class UnusedNestedNestedGenericTypePreservedByLinkXmlIsKept<W> { }
}

public class UnusedNestedTypeNotPreserved { }
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
<linker>
<assembly fullname="test, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null">
<type fullname="Mono.Linker.Tests.Cases.LinkXml.UnusedTypePreservedByLinkXmlIsKeptUnusedType" />
<type fullname="Mono.Linker.Tests.Cases.LinkXml.UnusedGenericTypePreservedByLinkXmlIsKept`1" preserve="none" />
<type fullname="Mono.Linker.Tests.Cases.LinkXml.UnusedGenericTypePreservedByLinkXmlIsKept`1/UnusedNestedTypePreservedByLinkXmlIsKept" preserve="none" />
<type fullname="Mono.Linker.Tests.Cases.LinkXml.UnusedGenericTypePreservedByLinkXmlIsKept`1/UnusedNestedGenericTypePreservedByLinkXmlIsKept`2" preserve="none" />
<type fullname="Mono.Linker.Tests.Cases.LinkXml.UnusedGenericTypePreservedByLinkXmlIsKept`1/UnusedNestedGenericTypePreservedByLinkXmlIsKept`2/UnusedNestedNestedGenericTypePreservedByLinkXmlIsKept`1" preserve="none" />
</assembly>
</linker>
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,27 @@ public SecondLevel (int arg)
}
}

// NativeAOT should generate conditional dependencies for the tag required
// https://github.com/dotnet/runtime/issues/80464
[Kept (By = ProducedBy.NativeAot)]
class ReallyUnused
{
[Kept (By = ProducedBy.NativeAot)]
private void PreservedMethod ()
{
new SecondLevelUnused (2);
}
}

// NativeAOT should generate conditional dependencies for the tag required
// https://github.com/dotnet/runtime/issues/80464
[Kept (By = ProducedBy.NativeAot)]
class SecondLevelUnused
{
[Kept (By = ProducedBy.NativeAot)]
public SecondLevelUnused (int arg)
{
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ protected virtual void VerifyTypeDefinition (TypeDefinition original, TypeDefini
// - It contains at least one member which has [Kept] attribute (not recursive)
//
bool expectedKept =
original.HasAttributeDerivedFrom (nameof (KeptAttribute)) ||
HasActiveKeptDerivedAttribute (original) ||
(linked != null && linkedModule.Assembly.EntryPoint?.DeclaringType == linked) ||
original.AllMembers ().Any (l => l.HasAttribute (nameof (KeptAttribute)));
original.AllMembers ().Any (HasActiveKeptDerivedAttribute);

if (!expectedKept) {
if (linked != null)
Expand Down Expand Up @@ -1061,14 +1061,54 @@ protected virtual bool ShouldMethodBeKept (MethodDefinition method)

protected virtual bool ShouldBeKept<T> (T member, string signature = null) where T : MemberReference, ICustomAttributeProvider
{
if (member.HasAttribute (nameof (KeptAttribute)) || member.HasAttribute (nameof (KeptByAttribute)))
if (HasActiveKeptAttribute (member) || member.HasAttribute (nameof (KeptByAttribute)))
return true;

ICustomAttributeProvider cap = (ICustomAttributeProvider) member.DeclaringType;
if (cap == null)
return false;

return GetCustomAttributeCtorValues<string> (cap, nameof (KeptMemberAttribute)).Any (a => a == (signature ?? member.Name));
return GetActiveKeptAttributes (cap, nameof (KeptMemberAttribute)).Any (ca => {
if (ca.Constructor.Parameters.Count != 1 ||
ca.ConstructorArguments[0].Value is not string a)
return false;

return a == (signature ?? member.Name);
});
}

private static IEnumerable<CustomAttribute> GetActiveKeptAttributes (ICustomAttributeProvider provider, string attributeName)
{
return provider.CustomAttributes.Where(ca => {
if (ca.AttributeType.Name != attributeName) {
return false;
}

object keptBy = ca.GetPropertyValue (nameof (KeptAttribute.By));
return keptBy is null ? true : ((ProducedBy) keptBy).HasFlag (ProducedBy.Trimmer);
});
}

private static bool HasActiveKeptAttribute (ICustomAttributeProvider provider)
{
return GetActiveKeptAttributes (provider, nameof (KeptAttribute)).Any ();
}

private static IEnumerable<CustomAttribute> GetActiveKeptDerivedAttributes (ICustomAttributeProvider provider)
{
return provider.CustomAttributes.Where (ca => {
if (!ca.AttributeType.Resolve ().DerivesFrom (nameof (KeptAttribute))) {
return false;
}

object keptBy = ca.GetPropertyValue (nameof (KeptAttribute.By));
return keptBy is null ? true : ((ProducedBy) keptBy).HasFlag (ProducedBy.Trimmer);
});
}

private static bool HasActiveKeptDerivedAttribute (ICustomAttributeProvider provider)
{
return GetActiveKeptDerivedAttributes (provider).Any ();
}

protected static uint GetExpectedPseudoAttributeValue (ICustomAttributeProvider provider, uint sourceValue)
Expand Down

0 comments on commit 1c4cc74

Please sign in to comment.