Skip to content

Add Nullable.GetValueRefOrDefaultRef API#64677

Merged
tannergooding merged 4 commits intodotnet:mainfrom
Sergio0694:nullable-get-value-ref-or-default-ref
Feb 17, 2022
Merged

Add Nullable.GetValueRefOrDefaultRef API#64677
tannergooding merged 4 commits intodotnet:mainfrom
Sergio0694:nullable-get-value-ref-or-default-ref

Conversation

@Sergio0694
Copy link
Copy Markdown
Contributor

Closes #1534

This PR adds the following API:

namespace System
{
    public static partial class Nullable
    {
        public static ref readonly T GetValueRefOrDefaultRef<T>(in T? nullable) where T : struct;
    }
}

@ghost ghost added community-contribution Indicates that the PR has been added by a community member new-api-needs-documentation and removed community-contribution Indicates that the PR has been added by a community member labels Feb 2, 2022
@ghost
Copy link
Copy Markdown

ghost commented Feb 2, 2022

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link
Copy Markdown

ghost commented Feb 2, 2022

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

@ghost
Copy link
Copy Markdown

ghost commented Feb 2, 2022

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

Issue Details

Closes #1534

This PR adds the following API:

namespace System
{
    public static partial class Nullable
    {
        public static ref readonly T GetValueRefOrDefaultRef<T>(in T? nullable) where T : struct;
    }
}
Author: Sergio0694
Assignees: -
Labels:

area-System.Runtime, new-api-needs-documentation

Milestone: -

@Sergio0694 Sergio0694 marked this pull request as ready for review February 2, 2022 14:36
@Sergio0694
Copy link
Copy Markdown
Contributor Author

Note: ideally the unit tests would compare the actual address, but they can't directly access internals from corelib and I'm not sure what the recommended approach is for this other than using a dynamic method, which seems a bit overkill and also potentially problematic for eg. NativeAOT? I'd have loved to use the APIs from #23716 if they had been implemented already 😄

Let me know if the included tests are enough or if any changes are recommended in that area 👍

@Sergio0694 Sergio0694 force-pushed the nullable-get-value-ref-or-default-ref branch 2 times, most recently from 55ec6f5 to b1f9aab Compare February 3, 2022 10:39
@stephentoub stephentoub closed this Feb 7, 2022
@stephentoub stephentoub reopened this Feb 7, 2022
@Sergio0694 Sergio0694 force-pushed the nullable-get-value-ref-or-default-ref branch from b1f9aab to 5f9c8e8 Compare February 17, 2022 11:49
@tannergooding tannergooding merged commit 5ab9fad into dotnet:main Feb 17, 2022
@Sergio0694 Sergio0694 deleted the nullable-get-value-ref-or-default-ref branch February 17, 2022 17:54
@Sergio0694
Copy link
Copy Markdown
Contributor Author

Thank you @stephentoub and @tannergooding for helping push this through! 😄
I'll be adding a .NET 7 target to the HighPerformance package soon to leverage this too 😋

@ghost ghost locked as resolved and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API proposal: Nullable.RefValue, RefValueOrDefault

4 participants