Determine solution for nullable attributes and the new public API to access them

[vitek-karas]

We will need to come up with a linker solution for the new API proposed here: dotnet/runtime#29723.

[eerhardt]

I had a thought here, that I am copying below:

One thought I had here is if we could annotate these new NullabilityInfoContext APIs with something like:

[RequiresFeature("System.Reflection.NullabilityInfo")]
public NullabilityInfo Create(ParameterInfo parameterInfo) {}

And what this would mean is:

When the linker sees a method with RequiresFeature attribute being called, it would "turn on that feature switch".

Then in ILLink.LinkAttributes.xml, we would put all these RemoveAttributeInstances requests behind the System.Reflection.NullabilityInfo feature switch:

https://github.com/dotnet/runtime/blob/eeadfdb2f57e607241b9b09ada1ef08717e618f6/src/mono/System.Private.CoreLib/src/ILLink/ILLink.LinkAttributes.xml#L133-L165

This would mean, if the feature switch wasn't "on", the attributes could be trimmed safely. If the feature switch was "on", then the attributes won't be trimmed. And then the APIs would work just fine in a trimmed app.

And in the off case where someone was reading the attributes directly, not through these APIs, they could turn the feature switch on for the app in the usual way, and the attributes wouldn't be trimmed.

[vitek-karas]

Thanks for the idea @eerhardt. I like:

• It makes things just work
• Could be used in other places as well

What I don't like:

• Would introduce lot of complexity to the linker. Currently there are several places where linker assumes that feature switches are all specified up front (branch removal, constant prop, ...). To support this proposal linker would have to be able to retroactively change FS values, which would be VERY tricky to implement (or it would require two passes).
• Requires new public API

[sbomer]

Expanding on what @MichalStrehovsky mentioned in https://github.com/mono/linker/issues/2078: We should also consider the GetCustomAttributes APIs. If we put different attributes under different feature switches, then GetCustomAttributes will only be "safe" when all of those feature switches are enabled (all the attributes are kept). For example

foreach (var a in typeof(Foo).GetCustomAttributes()) // ...

This should either keep all custom attributes on Foo, or issue a warning.

[MichalStrehovsky]

@vitek-karas Can this be closed since we have an extra new feature switch to make this one API work? We still have dotnet/runtime#103934 open tracking the fact that IL Linker is breaking user code without warnings when attribute stripping is enabled in general.

[vitek-karas]

I think it can - the specific issue is resolved. And generic attribute trimming is... problematic.