Make it possible to trim custom attribute artifacts

[MichalStrehovsky]

Pretty much any app uses custom attribute metadata due to FlagsAttribute. But not every app needs to activate custom attributes. We currently root unreachable code when custom attributes are not actually reflected on (such as constructors, property setters, MethodTables, etc.). The code reachable from there has potentially a big closure. This adds a check to make sure we only do this if there's a chance the attributes are looked at.

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

[jkotas]

Are there examples of where this kicks in and how much does it save?

[MichalStrehovsky]

Are there examples of where this kicks in and how much does it save?

This is a stepping stone. I'm trying to get rid of boxed enums (e.g. MethodTable for AttributeTargets enum).

MethodTable for Enum has a ton of dependencies due to all the generic specialization that was done to make stringification fast.

MichalStrehovsky/rt-sz#10
MichalStrehovsky/rt-sz#9

Has the stats for the two pull requests I opened.

If I combine the two pull requests with the below patch (haven't really decided how I can make that one look more acceptable), we can get hello-minimal to 801,280 bytes (so getting rid of Enums with the two pull requests and this patch is almost 10% saving, but we need all of this to kick in to reap the benefit).

Maybe this can wait for whether the below diff can be shaped into something acceptable.

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/MethodTable.Runtime.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/MethodTable.Runtime.cs
index 899ac448d5f..c0c196713aa 100644
--- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/MethodTable.Runtime.cs
+++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/MethodTable.Runtime.cs
@@ -37,7 +37,7 @@ internal bool IsEnum
 
                 // Generic type definitions that return true for IsPrimitive are type definitions of generic enums.
                 // Otherwise check the base type.
-                return IsPrimitive && (IsGenericTypeDefinition || NonArrayBaseType == MethodTable.Of<Enum>());
+                return IsPrimitive && (IsGenericTypeDefinition || NonArrayBaseType != MethodTable.Of<ValueType>());
             }
         }
 
diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/GC.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/GC.NativeAot.cs
index 0ee16609157..8263fc7cab8 100644
--- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/GC.NativeAot.cs
+++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/GC.NativeAot.cs
@@ -777,8 +777,8 @@ public static GCMemoryInfo GetGCMemoryInfo(GCKind kind)
                 throw new ArgumentOutOfRangeException(nameof(kind),
                                       SR.Format(
                                           SR.ArgumentOutOfRange_Bounds_Lower_Upper,
-                                          GCKind.Any,
-                                          GCKind.Background));
+                                          nameof(GCKind.Any),
+                                          nameof(GCKind.Background)));
             }
 
             var data = new GCMemoryInfoData();
diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs
index d6ea54412e1..8d8d86215b3 100644
--- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs
+++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs
@@ -289,7 +289,12 @@ private bool SetApartmentStateUnchecked(ApartmentState state, bool throwOnError)
             {
                 if (throwOnError)
                 {
-                    string msg = SR.Format(SR.Thread_ApartmentState_ChangeFailed, retState);
+                    string msg = SR.Format(SR.Thread_ApartmentState_ChangeFailed, retState switch
+                    {
+                        ApartmentState.STA => "STA",
+                        ApartmentState.MTA => "MTA",
+                        _ => "Unknown",
+                    });
                     throw new InvalidOperationException(msg);
                 }
 
diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.cs
index 189ce06de70..b7f814ccf72 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.cs
@@ -628,7 +628,7 @@ private static CultureData CreateCultureWithInvariantData()
             invariant._iFirstWeekOfYear = 0;                      // first week of year
 
             // all available calendar type(s).  The first one is the default calendar
-            invariant._waCalendars = new CalendarId[] { CalendarId.GREGORIAN };
+            invariant._waCalendars = (CalendarId[])(object)new ushort[] { (ushort)CalendarId.GREGORIAN };
 
             if (!GlobalizationMode.Invariant)
             {
diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/GregorianCalendar.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/GregorianCalendar.cs
index edb65a80557..87092870126 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Globalization/GregorianCalendar.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/GregorianCalendar.cs
@@ -50,8 +50,8 @@ public GregorianCalendar(GregorianCalendarTypes type)
             {
                 throw new ArgumentOutOfRangeException(
                     nameof(type),
-                    type,
-                    SR.Format(SR.ArgumentOutOfRange_Range, GregorianCalendarTypes.Localized, GregorianCalendarTypes.TransliteratedFrench));
+                    (int)type,
+                    SR.Format(SR.ArgumentOutOfRange_Range, nameof(GregorianCalendarTypes.Localized), nameof(GregorianCalendarTypes.TransliteratedFrench)));
             }
 
             _type = type;

[jkotas]

How does GetGCMemoryInfo get pulled into console hello world?

[MichalStrehovsky]

How does GetGCMemoryInfo get pulled into console hello world?

This one looks easily fixable by just skipping the validation of the enum range since we know we used a valid enum.

It's from string interpolation:

[jkotas]

Nit: Sort usings

[jkotas]

Maybe this can wait for whether the below diff can be shaped into something acceptable.

I have been looking at this one on and off. My worry with this diff is that requires writing unnatural code in random places. It means that we would likely see regressions regularly and that the benefits are unlikely to apply to anything beyond the trivial Console.WriteLine("Hello world"). I am not sure we want to go ahead with a change like that. Though, I agree it would be nice to make the enum formatting more pay-for-play.

[MichalStrehovsky]

I have been looking at this one on and off. My worry with this diff is that requires writing unnatural code in random places.

Yeah. Eventually, the changes to argument validation may not be needed. This is exactly the kind of information we already have in the whole program view (values of all possible parameters to a method), so codegen could optimize those validation checks away. It just doesn't, because it's more work to actually collect and apply that information. But argument validation could go away in the future.

The change not to create enum array is the annoying one. I cannot think of a way how it would be problematic, but it is unnatural. Unfortunately, glob/loc is not pay for play in .NET, even with invariant globalization. There's just too much junk needed to decide what is the negative minus sign to use when formatting integers (I don't don't think it's needed for much else).

Unfortunately even things that are supposed to be lightweight depend on custom attribute reflection (e.g. source generated interop), so maybe very few would benefit from this.