-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Leaf level AOT annotation for CoreLib library #66976
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Diagnostics; | ||
using System.Diagnostics.CodeAnalysis; | ||
using static System.RuntimeTypeHandle; | ||
|
||
namespace System.Collections.Generic | ||
|
@@ -25,6 +26,8 @@ internal static class ComparerHelpers | |
/// The logic in this method is replicated in vm/compile.cpp to ensure that NGen saves the right instantiations, | ||
/// and in vm/jitinterface.cpp so the jit can model the behavior of this method. | ||
/// </remarks> | ||
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file is not used with NativeAOT - src/coreclr/System.Private.Corelib are files specific to the JIT-based CoreCLR. It would not be safe - NativeAOT approaches comparers differently and does some work in the compiler to support them. I don't know how best to approach this - suppressing this runs a risk that if we ever move this code under src/libraries/System.Private.CoreLib and for whatever reason it gets activated under NativeAOT, we'll have a bug. Maybe the suppression is the best we can do, but it gives me a bad feeling. As an option to consider, we could also disable the analyzer for CoreCLR's CoreLib. |
||
Justification = "MakeGenericType is safe in this method")] | ||
internal static object CreateDefaultComparer(Type type) | ||
{ | ||
Debug.Assert(type != null && type is RuntimeType); | ||
|
@@ -59,6 +62,8 @@ internal static object CreateDefaultComparer(Type type) | |
/// Creates the default <see cref="Comparer{T}"/> for a nullable type. | ||
/// </summary> | ||
/// <param name="nullableType">The nullable type to create the default comparer for.</param> | ||
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern", | ||
Justification = "MakeGenericType is safe in this method")] | ||
private static object? TryCreateNullableComparer(RuntimeType nullableType) | ||
{ | ||
Debug.Assert(nullableType != null); | ||
|
@@ -113,6 +118,8 @@ internal static object CreateDefaultComparer(Type type) | |
/// <remarks> | ||
/// The logic in this method is replicated in vm/compile.cpp to ensure that NGen saves the right instantiations. | ||
/// </remarks> | ||
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern", | ||
Justification = "MakeGenericType is safe in this method")] | ||
internal static object CreateDefaultEqualityComparer(Type type) | ||
{ | ||
Debug.Assert(type != null && type is RuntimeType); | ||
|
@@ -157,6 +164,8 @@ internal static object CreateDefaultEqualityComparer(Type type) | |
/// Creates the default <see cref="EqualityComparer{T}"/> for a nullable type. | ||
/// </summary> | ||
/// <param name="nullableType">The nullable type to create the default equality comparer for.</param> | ||
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern", | ||
Justification = "MakeGenericType is safe in this method")] | ||
private static object? TryCreateNullableEqualityComparer(RuntimeType nullableType) | ||
{ | ||
Debug.Assert(nullableType != null); | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -55,6 +55,8 @@ public void Bake(ModuleBuilder module, int token) | |||||
#region Public Static Methods | ||||||
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2055:UnrecognizedReflectionPattern", | ||||||
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")] | ||||||
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern", | ||||||
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")] | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Similar comments apply below too - the mention of trimming doesn't describe the reason well enough)
Suggested change
|
||||||
public static MethodInfo GetMethod(Type type, MethodInfo method) | ||||||
{ | ||||||
if (type is not TypeBuilder && type is not TypeBuilderInstantiation) | ||||||
|
@@ -90,6 +92,8 @@ public static MethodInfo GetMethod(Type type, MethodInfo method) | |||||
|
||||||
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2055:UnrecognizedReflectionPattern", | ||||||
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")] | ||||||
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern", | ||||||
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")] | ||||||
public static ConstructorInfo GetConstructor(Type type, ConstructorInfo constructor) | ||||||
{ | ||||||
if (type is not TypeBuilder && type is not TypeBuilderInstantiation) | ||||||
|
@@ -1461,6 +1465,8 @@ public ConstructorBuilder DefineDefaultConstructor(MethodAttributes attributes) | |||||
Justification = "MakeGenericType is only called on a TypeBuilderInstantiation which is not subject to trimming")] | ||||||
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern", | ||||||
Justification = "GetConstructor is only called on a TypeBuilderInstantiation which is not subject to trimming")] | ||||||
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern", | ||||||
Justification = "MakeGenericType is only called on a TypeBuilder which is not subject to trimming")] | ||||||
private ConstructorBuilder DefineDefaultConstructorNoLock(MethodAttributes attributes) | ||||||
{ | ||||||
ConstructorBuilder constBuilder; | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -557,6 +557,7 @@ private static RuntimeType ResolveType(RuntimeModule scope, string typeName) | |
} | ||
#endregion | ||
|
||
[RequiresDynamicCode("The code for an array of the specified type might not be available.")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as ComparerHelpers applies - this is not used for NativeAOT and the compiler otherwise ensures custom attribute blobs are constructable. |
||
internal CustomAttributeTypedArgument(RuntimeModule scope, CustomAttributeEncodedArgument encodedArg) | ||
{ | ||
CustomAttributeEncoding encodedType = encodedArg.CustomAttributeType.EncodedType; | ||
|
@@ -1433,6 +1434,7 @@ internal static AttributeUsageAttribute GetAttributeUsage(RuntimeType decoratedA | |
return attributeUsageAttribute ?? AttributeUsageAttribute.Default; | ||
} | ||
|
||
[RequiresDynamicCode("The code for an array of the specified type might not be available.")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be suppressed because the type used in CreateInstance is a reference type (we handle IsValueType with an object array). |
||
internal static object[] CreateAttributeArrayHelper(RuntimeType caType, int elementCount) | ||
{ | ||
bool useAttributeArray = false; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1015,6 +1015,10 @@ private void AddSpecialInterface( | |
Justification = "Calls to GetInterfaces technically require all interfaces on ReflectedType" + | ||
"But this is not a public API to enumerate reflection items, all the public APIs which do that" + | ||
"should be annotated accordingly.")] | ||
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:UnrecognizedReflectionPattern", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would not be safe (and the justification is off) - this code is not active for NativeAOT. Similar comments to the other place applies. I really wonder if it would be better to disable the analyzer for CoreCLR's corelib. |
||
Justification = "Calls to GetInterfaces technically require all interfaces on ReflectedType" + | ||
"But this is not a public API to enumerate reflection items, all the public APIs which do that" + | ||
"should be annotated accordingly.")] | ||
private RuntimeType[] PopulateInterfaces(Filter filter) | ||
{ | ||
ListBuilder<RuntimeType> list = default; | ||
|
@@ -1601,6 +1605,7 @@ internal TypeCode TypeCode | |
return m_defaultMemberName; | ||
} | ||
|
||
[RequiresDynamicCode("The code for an array of the specified type might not be available.")] | ||
internal object[] GetEmptyArray() => _emptyArray ??= (object[])Array.CreateInstance(m_runtimeType, 0); | ||
#endregion | ||
|
||
|
@@ -3336,6 +3341,7 @@ public override Type[] GetGenericArguments() | |
} | ||
|
||
[RequiresUnreferencedCode("If some of the generic arguments are annotated (either with DynamicallyAccessedMembersAttribute, or generic constraints), trimming can't validate that the requirements of those annotations are met.")] | ||
[RequiresDynamicCode("The native code for this instantiation might not be available at runtime.")] | ||
public override Type MakeGenericType(Type[] instantiation!!) | ||
{ | ||
if (!IsGenericTypeDefinition) | ||
|
@@ -3438,8 +3444,10 @@ public override Type[] GetGenericParameterConstraints() | |
|
||
public override Type MakeByRefType() => new RuntimeTypeHandle(this).MakeByRef(); | ||
|
||
[RequiresDynamicCode("The code for an array of the specified type might not be available.")] | ||
public override Type MakeArrayType() => new RuntimeTypeHandle(this).MakeSZArray(); | ||
|
||
[RequiresDynamicCode("The code for an array of the specified type might not be available.")] | ||
public override Type MakeArrayType(int rank) | ||
{ | ||
if (rank <= 0) | ||
|
@@ -3860,6 +3868,7 @@ private extern object InvokeDispMethod( | |
return ret; | ||
} | ||
|
||
[RequiresDynamicCode("The code for an array of the specified type might not be available.")] | ||
private static void WrapArgsForInvokeCall(object[] aArgs, int[] aArgsWrapperTypes) | ||
{ | ||
int cArgs = aArgs.Length; | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -302,6 +302,7 @@ public static Type GetUnderlyingType(Type enumType!!) => | |||||||||||||||
enumType.GetEnumUnderlyingType(); | ||||||||||||||||
|
||||||||||||||||
#if !CORERT | ||||||||||||||||
[RequiresDynamicCode("It might not be possible to create an array of the enum type at runtime. Use the GetValues<TEnum> overload instead.")] | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The implementation for NativeAOT of this is AOT friendly - we don't want to bubble this out like this because the only form factor that currently cares is safe: runtime/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Enum.CoreRT.cs Lines 116 to 122 in b2dc37b
|
||||||||||||||||
public static TEnum[] GetValues<TEnum>() where TEnum : struct, Enum => | ||||||||||||||||
(TEnum[])GetValues(typeof(TEnum)); | ||||||||||||||||
#endif | ||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be a suppression - we're creating an instance of a reference type array (
System.Attribute
is a reference type andType
is expected to be deriving fromAttribute
or the cast would fail).Array.CreateInstance
with a reference type element is safe.