Skip to content

Commit

Permalink
Enable more ILLink test cases with native AOT (dotnet#110166)
Browse files Browse the repository at this point in the history
When I was working on the `DAMT.All*`, I noticed we don't run many Reflection tests, this is progress towards that.

Also fixes rare `CreateInstance` overloads - these were never worth the effort to implement honestly, but more code sharing made implementing them a breeze.

Contributes to dotnet#82447.
  • Loading branch information
MichalStrehovsky authored Dec 3, 2024
1 parent 05fa881 commit c5213de
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -638,17 +638,31 @@ private partial bool TryGetBaseType(TypeProxy type, out TypeProxy? baseType)
return false;
}

#pragma warning disable IDE0060
private partial bool TryResolveTypeNameForCreateInstanceAndMark(in MethodProxy calledMethod, string assemblyName, string typeName, out TypeProxy resolvedType)
{
// TODO: niche APIs that we probably shouldn't even have added
// We have to issue a warning, otherwise we could break the app without a warning.
// This is not the ideal warning, but it's good enough for now.
_diagnosticContext.AddDiagnostic(DiagnosticId.UnrecognizedParameterInMethodCreateInstance, calledMethod.GetParameter((ParameterIndex)(1 + (calledMethod.HasImplicitThis() ? 1 : 0))).GetDisplayName(), calledMethod.GetDisplayName());
resolvedType = default;
return false;
if (!System.Reflection.Metadata.AssemblyNameInfo.TryParse(assemblyName, out var an)
|| _callingMethod.Context.ResolveAssembly(an) is not ModuleDesc resolvedAssembly)
{
_diagnosticContext.AddDiagnostic(DiagnosticId.UnresolvedAssemblyInCreateInstance,
assemblyName,
calledMethod.GetDisplayName());
resolvedType = default;
return false;
}

if (!_reflectionMarker.TryResolveTypeNameAndMark(resolvedAssembly, typeName, _diagnosticContext, "Reflection", out TypeDesc? foundType))
{
// It's not wrong to have a reference to non-existing type - the code may well expect to get an exception in this case
// Note that we did find the assembly, so it's not a ILLink config problem, it's either intentional, or wrong versions of assemblies
// but ILLink can't know that. In case a user tries to create an array using System.Activator we should simply ignore it, the user
// might expect an exception to be thrown.
resolvedType = default;
return false;
}

resolvedType = new TypeProxy(foundType);
return true;
}
#pragma warning restore IDE0060

private partial void MarkStaticConstructor(TypeProxy type)
=> _reflectionMarker.MarkStaticConstructor(_diagnosticContext.Origin, type.Type, _reason);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,33 @@ internal bool TryResolveTypeNameAndMark(string typeName, in DiagnosticContext di
return true;
}

internal bool TryResolveTypeNameAndMark(ModuleDesc assembly, string typeName, in DiagnosticContext diagnosticContext, string reason, [NotNullWhen(true)] out TypeDesc? type)
{
List<ModuleDesc> referencedModules = new();
TypeDesc foundType = CustomAttributeTypeNameParser.GetTypeByCustomAttributeTypeNameForDataFlow(typeName, assembly, assembly.Context,
referencedModules, needsAssemblyName: false, out _);
if (foundType == null)
{
type = default;
return false;
}

if (_enabled)
{
foreach (ModuleDesc referencedModule in referencedModules)
{
// Also add module metadata in case this reference was through a type forward
if (Factory.MetadataManager.CanGenerateMetadata(referencedModule.GetGlobalModuleType()))
_dependencies.Add(Factory.ModuleMetadata(referencedModule), reason);
}

MarkType(diagnosticContext.Origin, foundType, reason);
}

type = foundType;
return true;
}

internal void MarkType(in MessageOrigin origin, TypeDesc type, string reason, AccessKind accessKind = AccessKind.Unspecified)
{
if (!_enabled)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,17 @@ public void LinkXml (string t)
[MemberData (nameof (TestDatabase.Reflection), MemberType = typeof (TestDatabase))]
public void Reflection (string t)
{
switch (t) {
case "TypeHierarchyReflectionWarnings":
case "ParametersUsedViaReflection":
case "UnsafeAccessor":
case "TypeUsedViaReflection":
case "RunClassConstructor":
case "NestedTypeUsedViaReflection":
Run (t);
switch (t)
{
case "ObjectGetType":
// Skip for now
break;
case "ObjectGetTypeLibraryMode":
case "TypeHierarchyLibraryModeSuppressions":
// No Library mode
break;
default:
// Skip the rest for now
Run (t);
break;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ static void TestArrayTypeGetType ()
// doesn't work on arrays, but the current implementation will preserve it anyway due to how it processes
// string -> Type resolution. This will only impact code which would have failed at runtime, so very unlikely to
// actually occur in real apps (and even if it does happen, it just increases size, doesn't break behavior).
[Kept (By = Tool.Trimmer)] // NativeAOT doesn't preserve array element types just due to the usage of the array type
[Kept (By = Tool.Trimmer | Tool.NativeAot)]
class ArrayCreateInstanceByNameElement
{
public ArrayCreateInstanceByNameElement ()
Expand All @@ -151,7 +151,6 @@ public ArrayCreateInstanceByNameElement ()
}

[Kept]
[ExpectedWarning ("IL2032", Tool.NativeAot, "https://github.com/dotnet/runtime/issues/82447")]
static void TestArrayCreateInstanceByName ()
{
Activator.CreateInstance ("test", "Mono.Linker.Tests.Cases.DataFlow.ComplexTypeHandling+ArrayCreateInstanceByNameElement[]");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public FromParameterOnStaticMethodTypeB (int arg) { }
}

// Small formatting difference
[ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type, Object[])", Tool.Trimmer, "")]
[ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type, Object[])", Tool.Trimmer | Tool.NativeAot, "")]
[ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type, params Object[])", Tool.Analyzer, "")]
[ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance), nameof (CultureInfo))]
[Kept]
Expand All @@ -197,7 +197,7 @@ public FromParameterOnInstanceMethodType (int arg, int arg2) { }

[ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type)")]
// Small formatting difference
[ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type, Object[])", Tool.Trimmer, "")]
[ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type, Object[])", Tool.Trimmer | Tool.NativeAot, "")]
[ExpectedWarning ("IL2067", nameof (Activator) + "." + nameof (Activator.CreateInstance) + "(Type, params Object[])", Tool.Analyzer, "")]
[Kept]
private static void FromParameterWithNonPublicConstructors (
Expand Down Expand Up @@ -552,6 +552,7 @@ private static void TestCreateInstanceOfTWithNewConstraint<
[Kept]
class TestCreateInstanceOfTWithNoConstraintType
{
[Kept(By = Tool.NativeAot /* native AOT intrinsically expands CreateInstance<T> and would keep this method, albeit not reflection-visible */)]
public TestCreateInstanceOfTWithNoConstraintType ()
{
}
Expand Down Expand Up @@ -680,7 +681,7 @@ public void TestCreateInstance ()

[Kept]
[KeptBaseType (typeof (AnnotatedBase))]
[KeptMember (".ctor()")]
[KeptMember (".ctor()", By = Tool.Trimmer /* This type is never allocated so there's no reason to keep ctor due to a GetType call */)]
class Derived : AnnotatedBase
{
[Kept]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ public int InstancePropertyViaReflection {
}

[Kept]
[ExpectedWarning ("IL2026", nameof (StaticPropertyExpressionAccess), Tool.Trimmer, "https://github.com/dotnet/linker/issues/2669")]
[ExpectedWarning ("IL2026", nameof (StaticPropertyExpressionAccess), Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/linker/issues/2669")]
[ExpectedWarning ("IL2026", nameof (StaticPropertyViaReflection))]
[ExpectedWarning ("IL2026", nameof (StaticPropertyViaRuntimeMethod))]
[ExpectedWarning ("IL2026", nameof (InstancePropertyExpressionAccess), Tool.Trimmer, "https://github.com/dotnet/linker/issues/2669")]
[ExpectedWarning ("IL2026", nameof (InstancePropertyExpressionAccess), Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/linker/issues/2669")]
[ExpectedWarning ("IL2026", nameof (InstancePropertyViaReflection))]
public static void Test ()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ private void PrivateMethodWithRUC () { }
}

[Kept]
[ExpectedWarning ("IL2026", Tool.Trimmer, "https://github.com/dotnet/linker/issues/2638")]
[ExpectedWarning ("IL2026", Tool.Trimmer | Tool.NativeAot, "https://github.com/dotnet/linker/issues/2638")]
public static void Test ()
{
BindingFlags left = BindingFlags.Instance | BindingFlags.Static;
Expand Down

0 comments on commit c5213de

Please sign in to comment.