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]: ParamCollectionAttribute #99285

Closed
AlekseyTs opened this issue Mar 5, 2024 · 9 comments · Fixed by #99385
Closed

[API Proposal]: ParamCollectionAttribute #99285

AlekseyTs opened this issue Mar 5, 2024 · 9 comments · Fixed by #99385
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.CompilerServices
Milestone

Comments

@AlekseyTs
Copy link
Contributor

Background and motivation

As part of supporting Params Collections feature, we need a new attribute that will be applied by C# compiler to non-array params parameters.

API Proposal

This is effectively a clone of ParamArrayAttribute, different namespace, different name, different meaning.
See https://github.com/dotnet/csharplang/blob/main/proposals/params-collections.md#metadata

namespace System.Runtime.CompilerServices
{
    [AttributeUsage(AttributeTargets.Parameter, Inherited = true, AllowMultiple = false)]
    public sealed class ParamCollectionAttribute : Attribute
    {
        public ParamCollectionAttribute() { }
    }
}

API Usage

Not meant for use in C# code.

Alternative Designs

No response

Risks

No response

@AlekseyTs AlekseyTs added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 5, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Mar 5, 2024
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Mar 5, 2024
@AlekseyTs AlekseyTs added blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Mar 5, 2024
@AlekseyTs
Copy link
Contributor Author

CC @stephentoub

@stephentoub
Copy link
Member

@AlekseyTs, I assume this is on the same plan as with other similar attributes, where the compiler with synthesize it internally if it can't find it in the compilation? I'm asking because of the "blocking" label; the feature will be usable even before this is added to the core libraries, right?

@stephentoub
Copy link
Member

Also, the "Inherited = true" part... I assume that was just copied from ParamArrayAttribute. Will that have any effect on the compiler? It's odd to me that it (both ParamArray and ParamCollection) would be inherited from a reflection perspective.

@stephentoub stephentoub added area-System.Runtime.CompilerServices and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Mar 5, 2024
@stephentoub stephentoub added this to the 9.0.0 milestone Mar 5, 2024
@ghost
Copy link

ghost commented Mar 5, 2024

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

Issue Details

Background and motivation

As part of supporting Params Collections feature, we need a new attribute that will be applied by C# compiler to non-array params parameters.

API Proposal

This is effectively a clone of ParamArrayAttribute, different namespace, different name, different meaning.
See https://github.com/dotnet/csharplang/blob/main/proposals/params-collections.md#metadata

namespace System.Runtime.CompilerServices
{
    [AttributeUsage(AttributeTargets.Parameter, Inherited = true, AllowMultiple = false)]
    public sealed class ParamCollectionAttribute : Attribute
    {
        public ParamCollectionAttribute() { }
    }
}

API Usage

Not meant for use in C# code.

Alternative Designs

No response

Risks

No response

Author: AlekseyTs
Assignees: -
Labels:

area-System.Runtime.CompilerServices, blocking, api-ready-for-review

Milestone: -

@AlekseyTs
Copy link
Contributor Author

I assume this is on the same plan as with other similar attributes, where the compiler with synthesize it internally if it can't find it in the compilation? I'm asking because of the "blocking" label; the feature will be usable even before this is added to the core libraries, right?

That is correct, but we need to make sure the API shape is approved in order to know what to synthesize.

@AlekseyTs
Copy link
Contributor Author

Also, the "Inherited = true" part... I assume that was just copied from ParamArrayAttribute.

Yes.

Will that have any effect on the compiler? It's odd to me that it (both ParamArray and ParamCollection) would be inherited from a reflection perspective.

I do not know why that property even exists. Neither C#, nor VB compiler ever pays any attention to it.

@stephentoub
Copy link
Member

we need to make sure the API shape is approved in order to know what to synthesize

Ok

@333fred
Copy link
Member

333fred commented Mar 5, 2024

I suspect that it is true because it is indeed inherited; we use the base definition to determine if a parameter is params. C# will always include ParamArrayAttribute on every override, but other languages or custom IL may not, and it's likely still desirable to have the attribute show up in reflection on overrides missing the attribute in IL.

@bartonjs
Copy link
Member

bartonjs commented Mar 5, 2024

Video

  • It was asked whether there should be an "s" at the end of "Param", and the answer was "no".
  • We also talked about the Inherited=true, and are sticking with "match what ParamArrayAttribute does", which is true.
namespace System.Runtime.CompilerServices
{
    [AttributeUsage(AttributeTargets.Parameter, Inherited = true, AllowMultiple = false)]
    public sealed class ParamCollectionAttribute : Attribute
    {
        public ParamCollectionAttribute() { }
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Mar 5, 2024
@stephentoub stephentoub self-assigned this Mar 6, 2024
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 6, 2024
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 7, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.CompilerServices
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@333fred @stephentoub @AlekseyTs @bartonjs and others