Giter Site home page Giter Site logo

Comments (13)

jaredpar avatar jaredpar commented on July 2, 2024 2

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.

from roslyn.

agocke avatar agocke commented on July 2, 2024 1

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.

from roslyn.

CyrusNajmabadi avatar CyrusNajmabadi commented on July 2, 2024

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?

from roslyn.

sbomer avatar sbomer commented on July 2, 2024

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.

from roslyn.

jaredpar avatar jaredpar commented on July 2, 2024

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

from roslyn.

MichalStrehovsky avatar MichalStrehovsky commented on July 2, 2024

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.

from roslyn.

AlekseyTs avatar AlekseyTs commented on July 2, 2024

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?

from roslyn.

agocke avatar agocke commented on July 2, 2024

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.

from roslyn.

CyrusNajmabadi avatar CyrusNajmabadi commented on July 2, 2024

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

from roslyn.

agocke avatar agocke commented on July 2, 2024

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.

from roslyn.

jaredpar avatar jaredpar commented on July 2, 2024

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.

from roslyn.

agocke avatar agocke commented on July 2, 2024

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.

from roslyn.

jaredpar avatar jaredpar commented on July 2, 2024

Please bring @333fred or me to the review

from roslyn.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.