Skip to content

Allow ref initializers for ref fields#64817

Merged
cston merged 4 commits intodotnet:mainfrom
cston:rf-initializers
Oct 22, 2022
Merged

Allow ref initializers for ref fields#64817
cston merged 4 commits intodotnet:mainfrom
cston:rf-initializers

Conversation

@cston
Copy link
Contributor

@cston cston commented Oct 19, 2022 •
edited
Loading

Allow ref fields to include initializers with ref expressions.

ref struct R
{
    ref int _f = ref s;
    static int s;
}

Fixes #64720.
Fixes #64725.

@ghost ghost added the Area-Compilers label Oct 19, 2022
@cston cston force-pushed the rf-initializers branch 2 times, most recently from fcfe669 to 7fe1bd7 Compare October 19, 2022 05:29
@cston cston marked this pull request as ready for review October 19, 2022 05:58
@cston cston requested a review from a team as a code owner October 19, 2022 05:58
@cston
Copy link
Contributor Author

cston commented Oct 20, 2022

@dotnet/roslyn-compiler, please review.

jcouv
jcouv reviewed Oct 20, 2022
View reviewed changes
@jcouv jcouv self-assigned this Oct 20, 2022
jcouv
jcouv reviewed Oct 20, 2022
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with review pass (iteration 1)

RikkiGibson
RikkiGibson reviewed Oct 21, 2022
View reviewed changes
src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs Outdated
ref byte _f1 = ref F(stackalloc byte[1]);
ref readonly byte _f2 = ref F(stackalloc byte[1]);
public R(object o) { }
static ref T F<T>(Span<T> s) => throw null;
Copy link
Member

@RikkiGibson RikkiGibson Oct 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider also testing a case where the span is scoped, so the initializer calling F() becomes legal.

consider also testing a case where an out var can escape through the return of a call, a la the following

ref struct RS
{
    ref byte rb = ref M(out var b);
    public RS() { }
    
    static ref byte M([UnscopedRef] out byte b)
    {
        b = 42;
        return ref b;
    }
}

jcouv
jcouv reviewed Oct 21, 2022
View reviewed changes
src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs
static ref T F<T>(scoped Span<T> s) => throw null;
}";
var comp = CreateCompilation(source, targetFramework: TargetFramework.Net70);
comp.VerifyEmitDiagnostics();
Copy link
Member

@jcouv jcouv Oct 21, 2022 •
edited by cston
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider verifying execution too #Resolved

jcouv
jcouv approved these changes Oct 21, 2022
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thanks (iteration 3)

@RikkiGibson RikkiGibson self-requested a review October 21, 2022 23:32
@RikkiGibson RikkiGibson self-assigned this Oct 21, 2022
@cston cston merged commit abc707c into dotnet:main Oct 22, 2022
@cston cston deleted the rf-initializers branch October 22, 2022 00:04
@ghost ghost added this to the Next milestone Oct 22, 2022
@allisonchou allisonchou mentioned this pull request Oct 24, 2022
Closed
@RikkiGibson RikkiGibson modified the milestones: Next, 17.5 P1 Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RikkiGibson RikkiGibson RikkiGibson approved these changes

@jcouv jcouv jcouv approved these changes

Assignees

@RikkiGibson RikkiGibson

@jcouv jcouv

Labels

Area-Compilers Feature - Ref Fields

Projects

None yet

Milestone

17.5 P1

Development

Successfully merging this pull request may close these issues.

Ref field initializer is treated as a value assignment Syntax error parsing field initializer with ref expression

3 participants

@cston @RikkiGibson @jcouv