[API Proposal]: CompilerLoweringPreserveAttribute

sbomer commented 4 months ago

Background and motivation

Context: https://github.com/dotnet/roslyn/issues/73920

In short, we need a way to indicate that the compiler should flow attributes from C# code to compiler-generated symbols, to aid downstream IL-based analysis tools. This behavior will be opt-in per attribute type, when the attribute type is annotated with CompilerLoweringPreserveAttribute.

API Proposal

namespace System.Runtime.CompilerServices;

[AttributeUsage(AttributeTargets.Class, Inherited = false)]
public class CompilerLoweringPreserveAttribute : Attribute
{
    public CompilerLoweringPreserveAttribute() { }
}

API Usage

CompilerLoweringPreserveAttribute shall be placed on an attribute definition to indicate that this attribute should flow to compiler-generated symbols:

[CompilerLoweringPreserve]
[AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter)]
public class MyAttribute { }

For example, when the compiler generates fields for primary constructor parameters, attributes flow from source parameters to compiler-generated fields.

Original:

class C([My] int v)
{
    // ...
}

Generated IL (pseudo-C#)

class C(int v)
{
    [My] // Attribute flows to generated field
    [CompilerGenerated]
    int <v>P;

    public C([My] v)
    {
        <v>P = v;
        base..ctor();
    }

    // ...
}

(Note this does not specify whether the compiler generates a field - just that if it does, the attribute flows as long as it has compatible AttributeTargets. See discussion in https://github.com/dotnet/roslyn/issues/73920).

CompilerLoweringPreserveAttribute will be applied to DynamicallyAccessedMembersAttribute, to allow primary constructors, and other constructs that lower to compiler-generated types/members, to be annotated for trimming without requiring ILLink to do as much reverse-engineering of the compiler-generated constructs.

Alternative Designs

Don't flow attributes, and require downstream tools to reverse-engineer the compiler-generated code. This results in a more fragile dependency on the compiler-generated code.
Flow all attributes (don't make it conditional on CompilerLoweringPreserveAttribute) with compatible AttributeTargets. This would lead to metadata bloat and possible unexpected semantics for attributes that have different meanings for different AttributeTargets.

Risks

There is some risk that this will lead to more downstream tools taking a dependency on the compiler-generated code. We should make it clear that this is not a guarantee of any particular rewrite strategy from C# to IL. Any tooling which looks for CompilerLoweringPreserveAttribute-annotated attributes should not rely on the lowering strategy and may need to be adjusted in response to compiler changes.

@agocke @jaredpar @dotnet/appmodel

dotnet-policy-service[bot] commented 4 months ago

Tagging subscribers to this area: @dotnet/area-system-runtime-compilerservices See info in area-owners.md if you want to be subscribed.

jaredpar commented 4 months ago

Want to make sure we've marked this so that it's a net10.0 API not net9.0. Don't want to ship an API before the compiler work is far enough along that we know it's the shape we need.

stephentoub commented 4 months ago

Should this be tracked as a C# / Roslyn feature in dotnet/roslyn or dotnet/csharplang? Discussion in runtime would subsequently just be about naming.

sbomer commented 4 months ago

https://github.com/dotnet/roslyn/issues/73920 is the tracking issue for Roslyn, is that what you're looking for?

MichalStrehovsky commented 4 months ago

Is Inherited = true intentional, or just a copy paste from elsewhere? (I'm not sure we'd need it and it feels like it complicates things.)

sbomer commented 4 months ago

I put Inherited = true so that a hierarchy of attribute types could specify this only once on a base attribute, but I agree we don't need it and it would complicate the design if we wanted to allow opting out on derived attribute types - changed to Inherited = false.

dotnet / runtime