-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Proposal: flow attributes on parameters and type parameters to compiler-generated code #73920
Comments
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? |
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:
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. |
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 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. |
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. |
Perhaps instead we should take another look at allowing |
That’s one option, but it leaves out parameters of lambdas, async methods, and iterators.
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. |
Taking in the discussion the core parts of the proposal are:
From the compiler side I think this is probably workable. |
That approach works for me as well. It helps, without being a hard contract. |
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. |
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. |
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. |
Please bring @333fred or me to the review |
…cking fields. Related to dotnet#73920.
Note, type parameters that nested types inherit from enclosing types already come with their attributes. This is existing behavior. However, in VB there is no syntax to apply an attribute to a type parameter. |
…eters of synthesized types.. Related to dotnet#73920.
…eters of synthesized types.. Related to dotnet#73920.
- From a parameter to a state machine field - From a parameter to a closure field - From a VB property/event to a backing field Related to dotnet#73920.
- From a parameter to a state machine field - From a parameter to a closure field - From a VB property/event to a backing field Related to #73920.
…cking fields. (dotnet#76475) Related to dotnet#73920.
…eters of synthesized types. (dotnet#76492) Related to dotnet#73920.
- From a parameter to a state machine field - From a parameter to a closure field - From a VB property/event to a backing field Related to dotnet#73920.
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.
edit: The attribute should flow to generated code on an opt-in basis. Tentatively, the presence of
CompilerLoweringPreserveAttribute
on the attribute type definition indicates that it should flow: dotnet/runtime#103430This 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 #46646 (generic type parameters) and #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:
Cases where attributes should not flow:
[property: ...]
annotations to primary constructor parameters of record types, so parameter annotations should not otherwise flow to fields.[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
(orAttributeTargets.Property | AttributeTargets.Field
) where the semantics are meaningfully different for parameters vs fields (or properties vs fields)?edit: we resolved this concern by proposing that the automatic flow of attributes is opt-in per attribute type.
The text was updated successfully, but these errors were encountered: