Skip to content
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

[API Proposal]: CompilerLoweringPreserveAttribute #103430

Open
sbomer opened this issue Jun 13, 2024 · 6 comments
Open

[API Proposal]: CompilerLoweringPreserveAttribute #103430

sbomer opened this issue Jun 13, 2024 · 6 comments
Assignees
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.CompilerServices
Milestone

Comments

@sbomer
Copy link
Member

sbomer commented Jun 13, 2024

Background and motivation

Context: dotnet/roslyn#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 dotnet/roslyn#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

@sbomer sbomer added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jun 13, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jun 13, 2024
Copy link
Contributor

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

@jaredpar
Copy link
Member

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.

@sbomer sbomer added this to the Future milestone Jun 13, 2024
@stephentoub
Copy link
Member

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
Copy link
Member Author

sbomer commented Jun 13, 2024

dotnet/roslyn#73920 is the tracking issue for Roslyn, is that what you're looking for?

@MichalStrehovsky
Copy link
Member

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.)

@teo-tsirpanis teo-tsirpanis removed the untriaged New issue has not been triaged by the area owner label Jun 14, 2024
@sbomer
Copy link
Member Author

sbomer commented Jun 14, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.CompilerServices
Projects
None yet
Development

No branches or pull requests

5 participants