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

Champion: ref local reassignment (15.7) #933

Open
2 of 5 tasks
gafter opened this issue Sep 23, 2017 · 21 comments
Open
2 of 5 tasks

Champion: ref local reassignment (15.7) #933

gafter opened this issue Sep 23, 2017 · 21 comments
Assignees
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion Proposal
Milestone

Comments

@gafter
Copy link
Member

gafter commented Sep 23, 2017

The span safety rules at https://github.com/dotnet/csharplang/blob/master/proposals/csharp-7.2/span-safety.md describe rules that would make it safe to reassign ref locals (and ref parameters), presumably with some syntax similar to variable = ref somelvalue. I propose we add support for this.

As part of this, we'd also allow iteration variables to be ref in for and foreach loops (as initially tracked by #1046).

LDM
https://github.com/dotnet/csharplang/blob/master/meetings/2017/LDM-2017-10-02.md


This was discussed in the 2017-09-02 LDM meeting, with the following tentative directions:

  • We will try to get ref reassignment into release 7.2 (update: we didn't manage to get this into 7.2, so it is now a 7.3 candidate)
  • The syntax for ref reassignment is e1 = ref e2. In other words, there is a single ref keyword and it appears after the = operator.
  • This is an expression form. It yields an lvalue.
  • Ref parameters may be ref-reassigned.

We do not expect to support "uninitialized" ref variables at this time. That is a possible future direction. We think it is a straightforward and simple extension, and we anticipate doing it sooner rather than later. If it turns out to be easy, it is possible it would make 7.2.


This was further discussed in the 2017-10-25 LDM meeting, where it was requested that the following be added to the scope of this championed issue for tentative work in C# 7.3:

@benaadams
Copy link
Member

Thank you :)

Aside

Not having separated declare and init would convolute some code e.g.

ref int value;
for (int i = 0; i < arr.Length; i++)
{
    ref value = arr[i];
}
// Though what would ref value be if arr.Length == 0?

However, can be worked around with the ternary operator (choice on condition) or initializing first item outside loop (in above example) - so isn't as important to address.

Though nullable ref may also be option to consider

ref? int TryGetLastIneffienctly()
{
    ref? int value = null;
    for (int i = 0; i < _arr.Length; i++)
    {
        if (i == _arr.Length - 1)
        {
            ref value = ref _arr[i];
        }
    }

    return ref value;
)

Which also resolves the current need to have a static placeholder ref

static int s_NotFound;

ref int TryGetLastIneffienctly(bool out success)
{
    for (int i = 0; i < _arr.Length; i++)
    {
        if (i == _arr.Length - 1)
        {
            success = true;
            return ref value = ref _arr[i];
        }
    }
    
    success = false;
    return ref s_NotFound
)

@gafter
Copy link
Member Author

gafter commented Sep 24, 2017

I don't think we can solve the problems arising from null by introducing new kinds of null.

@benaadams
Copy link
Member

Fair enough :)

@ygc369
Copy link

ygc369 commented Sep 24, 2017

good idea

@Thaina
Copy link

Thaina commented Sep 26, 2017

@gafter @benaadams I was propose #521

@gafter gafter changed the title ref local reassignment Champion: ref local reassignment Sep 28, 2017
@MadsTorgersen MadsTorgersen added this to the 7.2 candidate milestone Oct 2, 2017
@gafter
Copy link
Member Author

gafter commented Oct 2, 2017

This was discussed in the 2017-10-02 LDM meeting, with the following tentative directions:

  • We will try to get ref reassignment into release 7.2
  • The syntax for ref reassignment is e1 = ref e2. In other words, there is a single ref keyword and it appears after the = operator.
  • This is an expression form. It yields an lvalue.
  • Ref parameters may be ref-reassigned.

We do not expect to support "uninitialized" ref variables at this time. That is a possible future direction. We think it is a straightforward and simple extension, and we anticipate doing it sooner rather than later. If it turns out to be easy, it is possible it would make 7.2.

@gafter
Copy link
Member Author

gafter commented Oct 4, 2017

A tentative spec for ref local reassignment is here: #963

@MadsTorgersen MadsTorgersen modified the milestones: 7.2 candidate, 7.3 candidate Oct 25, 2017
@gafter
Copy link
Member Author

gafter commented Oct 30, 2017

This was further discussed in the 2017-10-25 LDM meeting, where it was requested that the following be added to the scope of this championed issue for tentative work in C# 7.3:

@ufcpp
Copy link

ufcpp commented Nov 7, 2017

In terms of consistency between ref-like and ref locals, should ref locals be able to be used in iterator as long as not stepping over a yield return/await?

// VS 15.5 Preview 2

static IEnumerable RefIterator()
{
    int x = 0;
    ref var r = ref x; // Error: CS8176
    yield return 0;
}

static IEnumerable SpanIterator1()
{
    Span<int> s = stackalloc int[1]; // OK!!
    yield return 0;
}

static IEnumerable SpanIterator2()
{
    Span<int> s = stackalloc int[1];
    var x = s[0]; // OK!!
    yield return 0;
    var y = s[0]; // Error: CS4013
}

static async Task RefAsync()
{
    int x = 0;
    ref var r = ref x; // Error: CS8177
}

static async Task SpanAsync()
{
    Span<int> s = stackalloc int[1]; // Error: CS4012
}

image

@vasiliuk
Copy link

vasiliuk commented Dec 6, 2017

please look at #1173 as it looks to be easy solution for local refs within async methods and iterators

@jcouv jcouv changed the title Champion: ref local reassignment Champion: ref local reassignment (15.7) Mar 16, 2018
@alrz
Copy link
Member

alrz commented Mar 21, 2018

Sadly, the current foreach ref implementation does not work with arrays. and I guess it won't make it into 7.3. Can we have a tracking issue for that? I supposed we could have it once this is implemented but seems like it missed as one of main motivating use cases for this feature.

@alrz
Copy link
Member

alrz commented Mar 21, 2018

Also, since this only works on duck-typed enumeration impls, I don't know if it's feasible to add support for ref iterators on ImmutableArray etc without incurring breaking changes.

@dmitriyse
Copy link

Does it ever discussed "full featured reference algebra" ? or "Managed Pointers algebra".
If local reference becomes reassignable than it will be usable to have reference to reference type.

int i = 5;
ref int r1 = ref i;
ref int r2 = ref i;
ref ref int rr = ref r1;
rr = ref r2;
rr = 10;

So probably we needs &i instead of "ref i". And classic C++ reference algebra ?

@atykhyy
Copy link

atykhyy commented Jan 11, 2020

It's a bit late at this point, but I'd like to know why ptr = ref foo was chosen for ref local reassignments instead of the IMO much more obvious and natural ref ptr = ref foo, consistent with the declaration syntax ref T ptr = ref foo. ptr behaves like T in all contexts where it's used without the ref keyword - except when it happens to be the LHS of an assignment expression where RHS is a reference. C and Go both make this explicit with *ptr = foo vs ptr = &foo, while in C++ (at least when I used to follow it) references are immutable and there is no scope to doubt what ptr = ... will do. In view of the above, perhaps you would consider introducing ref ptr = ref foo as an alternative and eventually deprecate the confusing ptr = ref foo while there is not that much code yet that uses this syntax?

@gafter
Copy link
Member Author

gafter commented Jan 11, 2020

Under your suggestion, the ref assignment equivalent of M(a = b) for a ref parameter would be M(ref ref a = ref b) and a similar issue arises for chained assignments. Does that feel right to you?

The ref on the left of a local declaration can be thought of as part of the “type” of the variable, not part of the assignment. =ref is the new binary operator that we introduced, and it is also used to initialize a ref local.

@atykhyy
Copy link

atykhyy commented Jan 12, 2020

M(ref ref a = ref b)

The second ref on the left is obviously redundant and could be elided.

The ref on the left of a local declaration can be thought of as part of the “type” of the variable, not part of the assignment.

Yes, that's right, but so what? The compiler is supposed to be there to serve the needs of users, not vice-versa. To reformulate my dissatisfaction in these terms, the current syntax of assignment to ref variables is equivalent to having an overloaded operator= for ref types. Overloading operator= is widely agreed to be confusing, and I've always thought that it was precisely for this reason that C# does not allow users to overload it (as well as several other operators where overloads can yield extremely non-obvious code, such as . and function application).

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Jan 12, 2020

Yes, that's right, but so what?

So that's why it was picked. Because that's the general intuition going on. There are other possible isomorphic formalization. This one was felt to be understandable and reasonable. So it was picked.

is equivalent to having an overloaded operator=

I don't really see how that's the case at all. The problem with overloaded = is that there's no way to tell syntactically that it's overloaded, and that it allows for arbitrary user code to run. That's not what's going on here. First, syntactically, it's clear indicated in the code what this is a ref-assignment. Second, even if it wasn't clearly indicated in the code, it's still the case that this functionality is baked directly into the language and doesn't allow arbitrary user code to run.

And, as you mentioned It's a bit late at this point. This approach was picked. The ship has sailed. It's not going to change.

an alternative and eventually deprecate the confusing

The alternative seems to be more verbose and not any more clear to me. I don't really find anything confusing about the current syntax. As neal mentions, =ref is already there and is already a syntactically unique way to express the concept.

Deprecation would break code that is working fine to address a non-problematic language feature. That only causes pain and cost to users.

@atykhyy
Copy link

atykhyy commented Jan 12, 2020

Of course there's no question of arbitrary code. It just feels a bit unnatural that = does completely different things depending on what's written to the right of the =. I'm used to that being determined by what's written to the left of the =. It's an easy mistake to make when scanning code, although to be fair it's also easy to miss the fact that the variable is a ref local in the first place, so perhaps it's not a big deal after all.

@gafter
Copy link
Member Author

gafter commented Jan 12, 2020

The second ref on the left is obviously redundant and could be elided.

No, the first ref distinguishes whether we’re calling a method that receives a value versus one that receives a ref. The second one is what you propose is required as part of the assignment syntax. So both would be required if we had selected the syntax you propose. A similar issue arises for chained ref assignments. We didn’t like that.

@atykhyy
Copy link

atykhyy commented Jan 13, 2020

Perhaps my phrasing was not good enough. I understand where the first ref comes from formal-grammar-wise. I meant rather that it adds nothing either semantically or to code clarity, and can be elided by a suitable modification of the grammar. AST expression nodes could carry a "is this refed" flag, to be used in method resolution among other things. In fact they might already do, if ref ref are rejected as syntactic errors rather than type errors.

@CyrusNajmabadi
Copy link
Member

As mentioned already: There are other possible isomorphic formalization. This one was felt to be understandable and reasonable. So it was picked.

And, as before, the ship has sailed. You're proposing a solution in search of problem.

@333fred 333fred added the Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification label Oct 16, 2020
@dotnet dotnet locked and limited conversation to collaborators Dec 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion Proposal
Projects
None yet
Development

No branches or pull requests