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]: Add managed API surface area for ByRefLike Generic constraints #68002

Closed
AaronRobinsonMSFT opened this issue Apr 14, 2022 · 9 comments · Fixed by #98070
Closed
Labels
api-approved API was approved in API review, it can be implemented area-System.Reflection.Metadata
Milestone

Comments

@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Apr 14, 2022

Background and motivation

The design for ByRefLike Generic constraints requires some new API surface area. Specifically the need to represent the new metadata values and a new feature flag to indicate support for the compiler.

API Proposal

Metadata enum values:

namespace System.Reflection
{
    [Flags]
    public enum GenericParameterAttributes
    {
+        [Obsolete("No longer represents all special constraints")]
         SpecialConstraintMask = 0x001C,
+        AcceptByRefLike = 0x0020,
    }
}

Feature flag.

namespace System.Runtime.CompilerServices
{
    public static partial class RuntimeFeature
    {
+        /// <summary>
+        /// Represents a runtime feature where byref-like types can be used in Generic parameters.
+        /// </summary>
+        public const string GenericsAcceptByRefLike = nameof(GenericsAcceptByRefLike);
    }
}

API Usage

Usage is similar to the official documentation for any of the existing GenericParameterAttributes enumeration values.

Alternative Designs

The following would represent a binary breaking change.

namespace System.Reflection
{
    [Flags]
    public enum GenericParameterAttributes
    {
-        SpecialConstraintMask = 0x001C,
+        SpecialConstraintMask = 0x003C,
+        AcceptByRefLike = 0x0020,
    }
}

Risks

Updating the enumeration value is troublesome since it contains a value that is a mask—an established anti-pattern. The "safest" way to handle this is the initial proposal but that does introduce confusion about new scenarios. It might be prudent to create a new API for accessing the specific constraints from the variance values.

@AaronRobinsonMSFT AaronRobinsonMSFT added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Reflection.Metadata labels Apr 14, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 7.0.0 milestone Apr 14, 2022
@ghost
Copy link

ghost commented Apr 14, 2022

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

Issue Details

Background and motivation

The design for ByRefLike Generic constraints requires some new API surface area. Specifically the need to represent the new metadata values and a new feature flag to indicate support for the compiler.

API Proposal

Metadata enum values:

namespace System.Reflection
{
    [Flags]
    public enum GenericParameterAttributes
    {
+        [Obsolete("No longer represents all special constraints")]
         SpecialConstraintMask = 0x001C,
+        AcceptByRefLike = 0x0020,
    }
}

Feature flag:

namespace System.Runtime.CompilerServices
{
    public static partial class RuntimeFeature
    {
+        /// <summary>
+        /// Represents a runtime feature where byref-like types can be used in Generic parameters.
+        /// </summary>
+        public const string GenericsAcceptByRefLike = nameof(GenericsAcceptByRefLike);
    }
}

API Usage

Usage is similar to the official documentation for any of the existing GenericParameterAttributes enumeration values.

Alternative Designs

The following would represent a binary breaking change.

namespace System.Reflection
{
    [Flags]
    public enum GenericParameterAttributes
    {
-        SpecialConstraintMask = 0x001C,
+        SpecialConstraintMask = 0x003C,
+        AcceptByRefLike = 0x0020,
    }
}

Risks

Updating the enumeration value is troublesome since it contains a value that is a mask—an established anti-pattern. The "safest" way to handle this is the initial proposal but that does introduce confusion about new scenarios. It might be prudent to create a new API for accessing the specific constraints from the variance values.

Author: AaronRobinsonMSFT
Assignees: -
Labels:

api-suggestion, area-System.Reflection.Metadata

Milestone: 7.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 14, 2022
@AaronRobinsonMSFT
Copy link
Member Author

AaronRobinsonMSFT commented Apr 14, 2022

@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label Apr 14, 2022
@steveharter steveharter self-assigned this Jul 13, 2022
@steveharter
Copy link
Member

Todo for v7: review above and have a plan forward for V8.

@steveharter
Copy link
Member

@AaronRobinsonMSFT I prefer the original proposal is fine with obsoleting SpecialConstraintMask.

Is this ready for review? Is there any newer information that would affect this proposal?

@steveharter steveharter modified the milestones: 7.0.0, 8.0.0 Jul 18, 2022
@steveharter steveharter removed their assignment Jul 18, 2022
@AaronRobinsonMSFT
Copy link
Member Author

@AaronRobinsonMSFT I prefer the original proposal is fine with obsoleting SpecialConstraintMask.

I prefer obsoleting as well.

Is this ready for review? Is there any newer information that would affect this proposal?

It is ready for review, but since we are deferring till .NET 8 and we still have language design discussion I'm not in any hurry to move this forward as it may change. I could see fallout from the language design influencing this in subtle ways so think we can hold off.

@steveharter
Copy link
Member

@AaronRobinsonMSFT should this be moved to v9?

@AaronRobinsonMSFT AaronRobinsonMSFT modified the milestones: 8.0.0, 9.0.0 Aug 2, 2023
@AaronRobinsonMSFT
Copy link
Member Author

Yep. Done.

@AaronRobinsonMSFT AaronRobinsonMSFT added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Feb 1, 2024
@jaredpar
Copy link
Member

jaredpar commented Feb 1, 2024

@AlekseyTs

@AaronRobinsonMSFT AaronRobinsonMSFT added blocking Marks issues that we want to fast track in order to unblock other important work blocking-release and removed blocking Marks issues that we want to fast track in order to unblock other important work blocking-release labels Feb 8, 2024
@bartonjs
Copy link
Member

bartonjs commented Feb 13, 2024

Video

  • We decided not to obsolete SpecialConstraintMask, which we can revisit later.
  • "AcceptByRefLike" => "AllowByRefLike"
  • GenericsAllowByRefLike => ByRefLikeGenerics, to follow the pattern from ByRefFields
namespace System.Reflection
{
    [Flags]
    public enum GenericParameterAttributes
    {
         SpecialConstraintMask = 0x001C,
+        AllowByRefLike = 0x0020,
    }
}

namespace System.Runtime.CompilerServices
{
    public static partial class RuntimeFeature
    {
+        /// <summary>
+        /// Represents a runtime feature where byref-like types can be used in Generic parameters.
+        /// </summary>
+        public const string ByRefLikeGenerics = nameof(ByRefLikeGenerics);
    }
}

@bartonjs bartonjs removed the blocking Marks issues that we want to fast track in order to unblock other important work label Feb 13, 2024
@bartonjs bartonjs 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 Feb 13, 2024
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 13, 2024
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 2, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 1, 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.Reflection.Metadata
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants