Proposal: flow attributes on parameters and type parameters to compiler-generated code

[sbomer]

When the compiler generates members or types as a private implementation detail of some language construct that supports attribute annotations, the generated members/types should include copies of the original attribute specifications, whenever the attribute type has compatible AttributeTargets.

This is a broader proposal intended to capture all cases where the compiler generates code that should inherit attributes from the original source, including the more specific cases discussed in https://github.com/dotnet/roslyn/issues/46646 (generic type parameters) and https://github.com/dotnet/roslyn/issues/73899 (primary constructor parameters).

This would help IL-based analysis tools (ILLink specifically, but possibly others) perform analysis driven by such attributes, without having to reverse-engineer the compiler-generated code. The hope is that the proposed behavior can remain an implementation detail to avoid constraining language evolution.

Cases where attributes should flow:

• Primary constructor parameters of non-record types: whenever a field is generated, it should have attributes matching annotations on the parameter, for attributes that allow AttributeTargets.Field.
• Type parameters: whenever a nested class, state machine, or closure gets has a generated type parameter corresponding to a type parameter of a parent type or method, it should get the same annotations. This should be done in all the same cases where type parameter constraints flow to the generated type parameters, as mentioned in https://github.com/dotnet/roslyn/issues/46646#issuecomment-1351953646.
• Method parameters hoisted to fields of state machines or closures: whenever a state machine or closure is generated and has a field corresponding to a method parameter, the field it should have attributes matching annotations on the parameter (again for attributes supporting AttributeTargets.Field).

Cases where attributes should not flow:

• Primary constructor parameters of record types: the compiler allows adding [property: ...] annotations to primary constructor parameters of record types, so parameter annotations should not otherwise flow to fields.
• Auto-implemented properties: the compiler allows adding [field: ...] annotations to an auto-implemented property, so property annotations should not otherwise flow to fields.

Open questions:

• Do we know of any attributes with AttributeTargets.Parameter | AttributeTargets.Field (or AttributeTargets.Property | AttributeTargets.Field) where the semantics are meaningfully different for parameters vs fields (or properties vs fields)?

[CyrusNajmabadi]

Primary constructor parameters of non-record types: whenever a field is generated, it should have attributes matching annotations on the parameter, for attributes that allow AttributeTargets.Field.

This somewhat worries me as it seems like taking any other impl approach might be a breaking change in teh future. What if teh compiler (or another compiler) doesn't implement these with fields?

Is this leaking internal impl details outwards? Could whatever system is looking at fields instead check the parameters instead itself?

[sbomer]

Our tooling needs to understand the fields (for the case of primary ctor parameters, or similar for the other cases). It could reverse-engineer the mapping back to parameters, but it will be relying on implementation details either way.

I'd like to see whether we can pick an implementation strategy that's more helpful for downstream tools, without making this emit strategy a guarantee forever. If the compiler changes the implementation of primary ctor parameters it would be fine to break this, and ILLink would have to respond, but in the meantime whenever it does generate fields it could preserve the attributes.

@jaredpar had a relevant comment when this came up for primary ctors:

My general concern though is that this locks the compiler into a particular emit strategy afaict.

I tend to think more along these lines. Like it or not there is a bit of a contract between the teams on how emit works today. Compiler can say it's an implementation detail but I also know if we changed how lambdas were emitted it would have an impact on the trimmer.

I don't want to get into a place where we're rigidly defining how the compiler emits metadata here. Essentially anything where it would prevent us from exploring new emit strategies. Think for the particular cases here though we can find some middle ground to work with.

Of course, changing the emit strategy to help downstream tools might result in more tools relying on it over time, so I understand the concern. @CyrusNajmabadi I'm curious whether you see room for a middle ground here, or if you believe it's better to strictly avoid giving any kind of assistance to tools trying to reason about IL semantics.

[agocke]

it's better to strictly avoid giving any kind of assistance to tools trying to reason about IL semantic

For context, it's actually a bit of an information-loss problem, not just IL reasoning. The issue here is that the runtime tooling is looking at IL and there are a few attributes, like DynamicallyAccessedMembersAttribute, that have special meaning to the runtime.

Users place these attributes in their code to signal that meaning to the runtime, but then the compiler rewriting discards the attributes during lowering. So there's a resulting information gap between what the user wrote, and what the tooling sees.

I don't see this as a feature strictly about the compiler, or about the runtime, but about an end-to-end scenario where the end user wants to influence the runtime and in certain cases there's no direct path for getting there today.

[jaredpar]

When the compiler generates members or types as a private implementation detail of some language construct that supports attribute annotations, the generated members/types should include copies of the original attribute specifications, whenever the attribute type has compatible AttributeTargets.

Are we certain that copying all attributes in the right approach? This behavior is only interesting to a very limited number of tooling scenarios. Copying all attributes could potentially lead to metadata bloat. Should we consider limiting it to a subset (providing there is a simple way to opt an attribute in)?

[MichalStrehovsky]

Are we certain that copying all attributes in the right approach? This behavior is only interesting to a very limited number of tooling scenarios. Copying all attributes could potentially lead to metadata bloat. Should we consider limiting it to a subset (providing there is a simple way to opt an attribute in)?

I would also think that some opt-in would be better, especially given "Do we know of any attributes ... where the semantics are meaningfully different for parameters vs fields (or parameters vs properties)?". Blindly copying all attributes could attach undesired semantics to the destination. Even if there is no such attribute today, there could be one tomorrow and then we'd need to build an opt-out mechanism.

[AlekseyTs]

Primary constructor parameters of non-record types: whenever a field is generated, it should have attributes matching annotations on the parameter, for attributes that allow AttributeTargets.Field.

Perhaps instead we should take another look at allowing field: attribute target on captured primary constructor parameters - https://github.com/dotnet/csharplang/blob/main/proposals/csharp-12.0/primary-constructors.md#field-targeting-attributes-for-captured-primary-constructor-parameters?

[agocke]

Perhaps instead we should take another look at allowing field: attribute target on captured primary constructor parameters

That’s one option, but it leaves out parameters of lambdas, async methods, and iterators.

I would also think that some opt-in would be better

Ok forgive my naming here, but what if we add a new attribute, CompilerLoweringPreserveAttribute. That attribute would appear on attribute definitions themselves. The compiler would then, best effort, preserve attributes that have been so attributed through lowering transformations, as permitted by attribute usage.

[jaredpar]

Taking in the discussion the core parts of the proposal are:

1. When compiler logically rewrites a symbol (parameter, type parameter, field, etc ...) into a new symbol it will copy attributes from the original symbol iff:
   1. The attribute is valid on the new symbol type: example when rewriting a parameter as a field the attribute must be able to target fields
   2. The attribute opts into this rewriting by using a new attribute. For the purpose of discussion calling it CompilerLoweringPreserveAttribute.
2. There is no change around the compiler specification on how rewrites occur. How closures, async, iterators, etc ... are emitted to IL remains an implementation detail the compiler can change at any time. There is just a subtle shift in what information is preserved during the rewrite.
   1. That means consumers like the ILLinker need to be adaptable to changes in how the compiler emits metadata in the future.

From the compiler side I think this is probably workable.

[CyrusNajmabadi]

That approach works for me as well. It helps, without being a hard contract.

[agocke]

I think this should work the trimming tools. We'll still have to do a little reverse engineering to handle local variables, since they can't have attributes in syntax, but this should lower the burden significantly -- and most importantly it should hopefully catch new lowering constructs that trimming didn't anticipate.

[jaredpar]

Next step is timing 😄

Think the chance of this getting completed in .NET 9 RTM timeframe is pretty low. But does seem like a good item to grab in the immediate post .NET 9 timeframe.

[agocke]

I don't see anything blocking on our side -- we've already built reverse-engineering code for most of the Roslyn features. Primary constructors aren't implemented, but we can direct users to use regular constructors until we get this implemented.

We'll also need to bring the proposed attribute through API review.

[jaredpar]

Please bring @333fred or me to the review