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

Ref returns and Locals #8030

Merged
merged 15 commits into from
Feb 19, 2016
Merged

Ref returns and Locals #8030

merged 15 commits into from
Feb 19, 2016

Conversation

VSadov
Copy link
Member

@VSadov VSadov commented Jan 19, 2016

Implementation of Ref returns and Ref locals

The original proposal and motivation for the feature is: #118

Ref locals and Ref returns

Ref locals and ref returns are generalizations of ref parameters in a sense that they do not get attached to a data slot automatically and instead can represent existing storage locations.

The goal is to allow abstracting in-place data from its actual storage (fields, arrays, slices) with minimum overhead. The targeted audience is the set of high performance scenarios where in-place update operations could be used to reduce unnecessary copying.

Ref locals

For the most part, ref locals behave just like regular locals. The most important difference is that
unlike regular locals, ref locals do not get a distinct storage location and instead can be ref-assigned an existing location.
In fact, due to definite assignment rules, a ref local must be ref-assigned before it can be used as a regular local.

        int[] arr = new int[] { 1, 2, 3 };

        ref int element = ref arr[1];       // ref assignment of storage        
        element = element + 40;     // regular reads and writes 

        System.Console.WriteLine(arr[1]);  // prints 42

To guarantee that references do not outlive referents, a "safe to return" rule is applied at the point of ref assignment and further propagated.

Ref returns

We would allow a function to indicate that its return value is a ref:

public static ref S Choose(ref S s1, ref S s2)
{
    if () return ref s1;
    else return ref s2;
} 

Just like ref arguments need to be explicitly annotated as such at the call site, ref return values must also be explicitly indicated in the return statement. This is a syntactic rule to ensure people are aware that they are trading in refs. A call to a ref-returning method is classified as a variable, and can therefore be assigned to.

It should be legal for a method to pass its own locals to a ref returning method, and get one of them back. However, we want to avoid a method returning a ref to one of its own locals. Technically, we could capture them and hoist them to the heap to enable this, just as we do for lambdas, but that defeats the purpose of being nimble and close to the metal.

Instead there are simple rules about which variables are “safe to return”, essentially ensuring that a method does not return its own variables:

• refs to variables on the heap are safe to return
• ref parameters are safe to return
• out parameters are safe to return (but must be assigned, as is already the case today)
• instance struct fields are safe to return as long as the receiver is safe to return
• “this” is not safe to return inside struct members
• a ref returned from another method is safe to return if all refs passed to that method as formal parameters were safe to return. (specifically it is irrelevant if receiver is safe to return, regardless whether receiver is a struct, class or typed as a generic type parameter)

The last rule works because the rules apply recursively to the invoked method as well.

Here is an example of what’s allowed and what’s not:

public static ref int Foo(ref int r1, ref int r2)
{
    int v1 = 1, v2 = 2;
    Choose(ref r1, ref r2) = 3; // Ok – one of passed in refs is assigned to
    return ref Choose(ref r1, ref r2); // Ok – one of passed in refs is returned
    Choose (ref v1, ref v2) = 4; // Ok – one of my own locals is assigned to
    return ref Choose(ref v1, ref v2); // Error: Would return one of my locals
    return ref Choose (ref r1, ref v1); //Error: Might return one of my locals
}

public interface IRefIndexable<T>
{
    ref T this[int i] { get; } 
}

public static ref int Foo<T>(T indexable, int i) where T: IRefIndexable<int>
{
    DoSomething(blah, blah);
    . . .
    return ref indexable[i];   // valid even though T might be a struct
}

struct MyIndexable : IRefIndexable<int>
{
    int size;
    int[] array;

    public ref int this[int i]
    {
        get
        {
            return ref this.array[i]; // legal, the referent is on the heap
            return ref this.size;     // not legal, “this” is not ref returnable 
         }
    }
}

== Also in:

  • Ref-returning properties and indexers. (must not have setters)
  • Ref-returning lambdas
  • Ref-returning local functions

== Notes

  • In this implementation ref locals are "single assignment". They must be ref-initialized (or bound to a variable) once declared and cannot be rebound to a different variable.
    Although the feature can be generalized to allow multiple re-assignments, it would complicate the feature, perhaps unnecessarily, so it is not currently allowed.

@VSadov VSadov force-pushed the refReturns branch 5 times, most recently from 627fff2 to eebffc4 Compare January 22, 2016 20:54
@VSadov
Copy link
Member Author

VSadov commented Jan 22, 2016

@dotnet/roslyn-compiler @dotnet/roslyn-ide - could you take look?

@VSadov
Copy link
Member Author

VSadov commented Jan 25, 2016

ping

@dpoeschl
Copy link
Contributor

Notes to testers:

  • Specs: read the PR description. The most up to date spec so far.
  • To get this working in the IDE, just make SyntaxParser.IsFeatureEnabled return true
  • ref returns should work with local methods
  • this applies to anything method-like (lambdas, delegates, properties, indexers, ...)
  • top-level scripting "locals" aren't really handled (they're actually fields underneath)

@VSadov
Copy link
Member Author

VSadov commented Jan 27, 2016

@dotnet-bot test prtest/win/dbg/unit32 please

@VSadov
Copy link
Member Author

VSadov commented Jan 27, 2016

@dotnet-bot test prtest/lin/tst/unit32 please

@VSadov
Copy link
Member Author

VSadov commented Feb 6, 2016

@dotnet-bot test prtest/win/dbg/unit64 please

@dpoeschl
Copy link
Contributor

dpoeschl commented Feb 7, 2016

@dotnet-bot retest prtest/win/dbg/unit64 please
// Previous failure: http://dotnet-ci.cloudapp.net/job/roslyn_prtest_win_dbg_unit64/3451/
// Retest reason: It's the "MakeConstCS" failure...

@nguerrera
Copy link
Contributor

If I've understood via quick code search correctly, there will need to be a public representation of the assignment ref kind added to IAssignmentExpression so that IOperation analyzers can differentiate between the two cases. cc @JohnHamby

@VSadov
Copy link
Member Author

VSadov commented Feb 12, 2016

@nguerrera There are also ref returns. Not sure if you need to differentiate for those too.

@dpoeschl
Copy link
Contributor

Close/open cycling this PR to try to get past the MakeConst failure
// Previous failure: http://dotnet-ci.cloudapp.net/job/roslyn_prtest_win_dbg_unit64/3452/

@nguerrera
Copy link
Contributor

@VSadov. Yes. Correct me if I'm wrong, but I believe IInvocationExpression.TargetMethod.ReturnsByRef is sufficient for that.

@VSadov
Copy link
Member Author

VSadov commented Feb 12, 2016

@nguerrera - what I meant is the case inside a byref method or property getter where you do

{
     return ref arr[0];
}

@nguerrera
Copy link
Contributor

@VSadov Ah. Yes, I think there should be some indication of return vs return ref added to IReturnStatement as well.

@nguerrera
Copy link
Contributor

I think there is also an IOperation issue that there's no current way to make an IInvocationExptession into an IReferenceExpression for eligibility as LHS of assignment.

The feature needs to be properly implemented taking into account that variable is byref.
For now disabling this refactoring if variable happen to be byref to avoid crash as described in dotnet#8416
@VSadov
Copy link
Member Author

VSadov commented Feb 19, 2016

test prtest/mac/dbg/unit32 please

VSadov added a commit that referenced this pull request Feb 19, 2016
@VSadov VSadov merged commit 453bdae into dotnet:future Feb 19, 2016
@miloush
Copy link

miloush commented Apr 1, 2016

I have probably missed the discussion, but why is ref keyword needed on the calling side?
i.e.

ref int r = ref Find(numbers);

It looks a bit confusing to me, like asking for a reference to the reference... why not

ref int r = Find(numbers);

only?

@VSadov VSadov deleted the refReturns branch June 29, 2017 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants