Add RequiresUnreferencedCodeAttribute

[MichalStrehovsky]

This is in support of dotnet/linker#988 - see the spec there for motivation.

Potentially might want to call this something like RequiresFormFactorAttribute with a FormFactor enum. We'll want similar attributes to mark things that are unfriendly to AOT or unfriendly to single-file deployments.

namespace System.Runtime.CompilerServices
{
    [AttributeUsage(
        AttributeTargets.Method |
        AttributeTargets.Class |
        AttributeTargets.Struct |
        AttributeTargets.Constructor |
        AttributeTargets.Assembly)]
    public sealed class RequiresUnreferencedCodeAttribute : Attribute
    {
        public RequiresUnreferencedCodeAttribute(string message)
        {
            Message = message;
        }

        public string Message { get; }
    }
}

[vitek-karas]

We definitely need a better name... "FormFactor" sounds OK to me, but I'm not sure about "unsafe".

I would also start with only allowing this on methods. If we see a need to put this on other things we can expand later on. I just fear that defining the correct behavior for this on things like assemblies will be relatively tricky.

[vitek-karas]

/cc @eerhardt

[stephentoub]

For naming, I'd rather the attribute describe what the condition is rather than its effects, e.g. [ReflectsOnInput(parameterName)]. That then can be interpreted both by tools and by developers calling this method and themselves needing to understand what about the target they need to then protect so that their method can be considered linker safe.

[vitek-karas]

For naming, I'd rather the attribute describe what the condition is rather than its effects, e.g. [ReflectsOnInput(parameterName)].

That's what the proposal for DynamicallyAccessedMembersAttribute is about - #33861.
This attribute here is for cases where it's not possible to point to a parameter/data or it simply doesn't make sense. It's also not only about reflection going forward. Examples:

• Assembly.Location for single-file correctness
• Assembly.LoadFrom for linker friendliness (since it loads code linker has never seen, it is potentially problematic)
• Reflection.Emit for AOT scenarios without a JIT/interpreter
• ...

We also plan to use this as a band aid for cases which the linker doesn't support yet, or for cases where the DynamicallyAccessedMembersAttribute will not be good enough.

There should also eventually be "LinkerFriendly" attribute - among other things also as a band-aid for cases where the linker finds something wrong, but it's not.

[stephentoub]

Ok, but that's missing my point. And your examples highlight that, as several of them are not about the linker, and "form factor" is ambiguous (that can, for example, refer to whether an app's UI is designed for a phone, a tablet, a desktop, etc.)

[vitek-karas]

I absolutely agree that it's not just about a linker and that we need to find a better name if we're going to use the same attribute for all of these cases. We could also have multiple attributes each covering one of these scenarios.

[stephentoub]

We could also have multiple attributes each covering one of these scenarios.

I think we need to. e.g. If all I care about is trimming to make my app smaller, why would I want warnings about things that might not work only if I was targeting AOT? This also speaks to why I think attributes should be named very specifically; they can describe the what, and a tool can then decide whether that condition is relevant to its needs. If the attribute instead speaks to a specific tool, other tools need to guess why and whether it's relevant to them.

[vitek-karas]

If all I care about is trimming to make my app smaller, why would I want warnings about things that might not work only if I was targeting AOT?
Linker should only warn about things which are applicable to the current configuration. So if the app being built is not asking for AOT or Single-File linker should not warn about those. That said adding the ability to detect issues with AOT/Single-File/... is still very much in "thinking about it" as we don't have a specific designs.

About the trim-only attribute (if we decide to go with that) - I still think we should have a generic "band aid" attribute for cases which we don't have better attributes for. But maybe it can be something relatively specific like [HasDynamicBehavior("Loading assemblies can lead to breaks static analysis is required for the app")] (I don't know, bad with names) - something which would not be tool specific, but would also be "generic" enough.

[stephentoub]

Linker should only warn about things which are applicable to the current configuration. So if the app being built is not asking for AOT or Single-File linker should not warn about those.

If I put this LinkerUnfriendly/FormFactor attribute on my method, how does the linker know I did that because I had AOT concerns rather than because I had trimming concerns?

[MichalStrehovsky]

The problem is that the attribute (if we go only with the linker-unfriendliness, forgoing single-file unfriendly or AOT unfriendly) is that the attribute is tool specific.

It means "the patterns in this method are very hard/impossible to analyze, don't analyze it". The "very hard" part is IL Linker specific. There could be another tool (cough, .NET Native dependency reducer, cough) that would have no trouble analyzing the pattern.

But we need something that is IL Linker specific to produce actionable warnings from the linker. Otherwise the warning will be about some reflection pattern deep within the callstack that started with a method that the end user could still reason about.

It's kind of like the myriad of custom attributes in CoreLib with C# compiler/Visual C++ compiler -specific semantics and mean nothing to any other tool.

[eerhardt]

The way I was thinking about this was to make some sort of Pass Through attribute, that says - "I know I am doing something that is unsafe. But don't complain about me. Instead when someone calls me, complain to them that they shouldn't be calling me."

We can annotate some public API in CoreLib with this attribute. And if a caller is calling that API, they would get the warning from the linker that they shouldn't be calling that API. If asked, we can analyze inside the public API in CoreLib to know "what" is unsafe about it.

Thinking more about this, we probably wouldn't need an attribute at all for that. Instead, we would just need to know what is considered "the developer's code", and what is considered "external". At that point, any public, "external" API that the developer calls that does something unsafe, the developer gets a warning about the public, "external" API. Not about what is inside it.

For example, take System.Linq.Expressions. It uses a bunch of Reflection. If a developer is calling System.Linq.Expressions, they wouldn't get warnings that inside System.Linq.Expressions it uses a bunch of Reflection. Instead, the developer would get a warning that it shouldn't be using System.Linq.Expressions. And if they want to dig and find out "why" - then the linker can give the full reasoning.

Similarly with a single-file deployment-unfriendly instance: RuntimeEnvironment::GetRuntimeDirectory() calls Assembly::get_Location(), which is unfriendly for single-file deployments. If a developer is calling RuntimeEnvironment::GetRuntimeDirectory(), and is single-file linking, then the developer would get a warning about RuntimeEnvironment::GetRuntimeDirectory() is not going to work for single-file deployments.

The same reasoning could be used for 3rd party dependencies as well, outside of the framework.

[MichalStrehovsky]

Thinking more about this, we probably wouldn't need an attribute at all for that. Instead, we would just need to know what is considered "the developer's code", and what is considered "external".

This would miss out on the "message" part where the library author can suggest workarounds.

I look at this as an ObsoleteAttribute - the API annotated with this is no longer recommended to use once one opts into linking.

[vitek-karas]

/cc @marek-safar

[eerhardt]

I think I understand @stephentoub's position better here, and agree that we shouldn't have a single [LinkerUnfriendly] attribute. But instead, we should express what "capability" the code requires and create different attributes for each of those capabilities.

For example, we could have an attribute like [RequiresDynamicCode] that we could add to anything that uses Reflection.Emit. This is unsafe when targeting AOT scenarios that don't allow dynamically generated code. But it is perfectly safe for a "trimmed" micro-service application that runs on the server.

This allows the linker to suppress any errors inside this code using Reflection.Emit because it is annotated as saying "I know I require dynamic code". But if that same method also calls Assembly.LoadFrom, the linker can still warn about that, since it isn't annotated for [RequiresLoadingAssemblies] (or whatever we name that one).

This also allows other tools to reason about the capabilities required by the method, not just the linker.

These "capability"-driven attributes can take a message as well, to address @MichalStrehovsky's comment above.

[MichalStrehovsky]

Renamed this to RequiresUnreferencedCodeAttribute per discussion with Vitek. I expect this to change again during the review, but this is what we'll try to go with.

[bartonjs]

• Since this is supposed to be applied by hand, the namespace should be System.Diagnostics.CodeAnalysis
  • We may want to bulk review and move the linker-inspired new attributes for .net 5 into a new namespace.
• Add a Url parameter
• The AttributeTargets should either only include methods (and ctors) or all of the container types above methods, not just some of them.
  • It sounds like trimming to just methods is preferred by the tooling team.

namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(
        AttributeTargets.Method |
        AttributeTargets.Constructor,
      Inherited = false,
      AllowMultiple = false)]
    public sealed class RequiresUnreferencedCodeAttribute : Attribute
    {
        public RequiresUnreferencedCodeAttribute(string message)
        {
            Message = message;
        }

        public string Message { get; }
        public string Url { get; set; }
    }
}