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]: RequiredMemberAttribute and NoRequiredMembersAttribute #64248

Closed
333fred opened this issue Jan 25, 2022 · 5 comments · Fixed by #64287
Closed

[API Proposal]: RequiredMemberAttribute and NoRequiredMembersAttribute #64248

333fred opened this issue Jan 25, 2022 · 5 comments · Fixed by #64287
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.CompilerServices
Milestone

Comments

@333fred
Copy link
Member

333fred commented Jan 25, 2022

Background and motivation

As part of supporting C# 11's Required Members feature, we need two new attributes: RequiredMemberAttribute and NoRequiredMembersAttribute. They work as follows:

  • RequiredMemberAttribute will be applied to every type (class or struct) that defines required members, and to all the members (fields or properties) that are required by that type.
  • NoRequiredMembersAttribute will be applied to a constructor of a type that has required members, and then users of that constructor will not be required to set any members of the type when calling it. Think copy constructors for records: this is how they will communicate that calling the constructor does not require setting any members, because all members will have been set by the copy constructor itself.

API Proposal

namespace System.Runtime.CompilerServices
{
    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Field | AttributeTargets.Property)]
    public sealed class RequiredMemberAttribute : Attribute { }
}
namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(AttributeTargets.Constructor)]
    public sealed class NoRequiredMembersAttribute  : Attribute { }
}

API Usage

RequiredMemberAttribute will not be manually usable. It will be an error to apply it to C# code by hand.

A copy constructor for a type that has required members might look like this:

public class C
{
    public required int Property { get; init; }
    public C() {}

    [NoRequiredMembers]
    public C(C c) => this.Property = c.Property;
}


// Usage
var c1 = new C { Property = 1 };
var c2 = new C(c1); // No error, constructor waives requirements

Alternative Designs

I am open to alternative naming schemes. We had also considered a version of RequiredMemberAttribute where all the members were listed by name as a string argument to the constructor, but we redesigned after some community feedback and consultation with Wrighton to make sure that more attributes wouldn't break perf.

Risks

No response

@333fred 333fred added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 25, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 25, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Jan 25, 2022

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 C# 11's Required Members feature, we need two new attributes: RequiredMemberAttribute and NoRequiredMembersAttribute. They work as follows:

  • RequiredMemberAttribute will be applied to every type (class or struct) that defines required members, and to all the members (fields or properties) that are required by that type.
  • NoRequiredMembersAttribute will be applied to a constructor of a type that has required members, and then users of that constructor will not be required to set any members of the type when calling it. Think copy constructors for records: this is how they will communicate that calling the constructor does not require setting any members, because all members will have been set by the copy constructor itself.

API Proposal

namespace System.Runtime.CompilerServices
{
    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Field | AttributeTargets.Property)]
    public sealed class RequiredMemberAttribute : Attribute { }
}
namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(AttributeTargets.Constructor)]
    public sealed class NoRequiredMembersAttribute  : Attribute { }
}

API Usage

RequiredMemberAttribute will not be manually usable. It will be an error to apply it to C# code by hand.

A copy constructor for a type that has required members might look like this:

public class C
{
    public required int Property { get; init; }
    public C() {}

    [NoRequiredMembers]
    public C(C c) => this.Property = c.Property;
}


// Usage
var c1 = new C { Property = 1 };
var c2 = new C(c1); // No error, constructor waives requirements

Alternative Designs

I am open to alternative naming schemes. We had also considered a version of RequiredMemberAttribute where all the members were listed by name as a string argument to the constructor, but we redesigned after some community feedback and consultation with Wrighton to make sure that more attributes wouldn't break perf.

Risks

No response

Author: 333fred
Assignees: -
Labels:

api-suggestion, area-System.Runtime.CompilerServices, untriaged

Milestone: -

@ghost ghost added this to Needs triage in Triage POD for Meta, Reflection, etc Jan 25, 2022
@stephentoub
Copy link
Member

@333fred, you didn't specify AllowMultiple or Inherited, which means these would be AllowMultiple=false and Inherited=true. Is that the intention? I assume "yes", just double checking.

@stephentoub stephentoub added api-ready-for-review API is ready for review, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work 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 labels Jan 25, 2022
@stephentoub
Copy link
Member

@333fred, I believe we discussed this, but for NoRequiredMembersAttribute will be applied to a constructor of a type that has required members, and then users of that constructor will not be required to set any members of the type when calling it, this is an all-or-nothing approach. Are there no scenarios where the ctor itself supplies some but not all of the required members, thus making some not required but others still required?

@terrajobst
Copy link
Member

terrajobst commented Jan 25, 2022

Video

  • We'd prefer the name SatisfiesRequiredMembersAttribute
    • This gives us the extensibility by adding constructors/properties to allow specifying which members will be initialized/not initialized later, if the language wants to go there.
  • If we want serializers to throw an exception unless all members are specified, we should consider exposing a reflection API
namespace System.Runtime.CompilerServices
{
    [AttributeUsage(AttributeTargets.Class |
                    AttributeTargets.Struct |
                    AttributeTargets.Field |
                    AttributeTargets.Property,
                    AllowMultiple=false,
                    Inherited=false)]
    public sealed class RequiredMemberAttribute : Attribute
    {
        public RequiredMemberAttribute();
    }
}
namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(AttributeTargets.Constructor,
                    AllowMultiple=false,
                    Inherited=false)]
    public sealed class SetsRequiredMembersAttribute : Attribute
    {
        public SetsRequiredMembersAttribute();
    }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jan 25, 2022
333fred added a commit to 333fred/runtime that referenced this issue Jan 25, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 25, 2022
@stephentoub stephentoub removed the blocking Marks issues that we want to fast track in order to unblock other important work label Jan 25, 2022
@buyaa-n buyaa-n added this to the 7.0.0 milestone Jan 26, 2022
Triage POD for Meta, Reflection, etc automation moved this from Needs triage to Done Jan 27, 2022
stephentoub pushed a commit that referenced this issue Jan 27, 2022
* Add attributes for required members

Closes #64248.

* Address PR feedback

* Update src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems

* Address PR feedback
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 27, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Feb 27, 2022
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
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants