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

Expected behavior when spilling ref receiver for a call to a readonly member #34487

Open
RikkiGibson opened this issue Mar 26, 2019 · 1 comment
Assignees
Milestone

Comments

@RikkiGibson
Copy link
Contributor

Related to #32911 and maybe #22055

There is a specific case where implicitly spilling a receiver by value is allowed only if the receiver's type is a readonly struct.

using System;
using System.Threading.Tasks;
readonly struct S { 
    public int M(int i) => i;
}
public class C {
    ref S Get() => throw null;
    
    public async Task<int> M(Task<int> t) {
        Get().M(await t); // Spilling permitted
        return 42;
    }
}

The check can be found here: https://github.com/dotnet/roslyn/blob/d89223c/src/Compilers/CSharp/Portable/Lowering/SpillSequenceSpiller.cs#L788

The question is, basically: does moving the readonly from the struct S declaration to the public int M declaration result in a legal program? And if so, what's a reasonable way to adjust the check in SpillSequenceSpiller to make that work?

cc @jaredpar @VSadov

@jaredpar
Copy link
Member

jaredpar commented Mar 27, 2019

@gafter as well here.

I agree the current behavior is as described. I don't remember us making this explicit decision though hence I'm a bit interested in the reasoning here. To be clear this isn't "safe" as compared to traditional struct as it's still subject to mutations that are observable across await boundaries. Consider:

using System;
using System.Threading.Tasks;
readonly struct S {
    public readonly int Field;
    public S(int field) {
        Field = field;
    }
    public int M(int i) => i + Field;
}

public class Program {
    static S Value = new S(0);
    static ref S Get() => ref Value;
    static int F(int value) {
        Value = new S(value);
        return value;
    }
    
    public static async Task Main() {
        Value = new S(0);
        Console.WriteLine(Get().M(F(1)));
        Value = new S(0);
        Console.WriteLine(Get().M(F(await Task.FromResult(1))));
    }
}

This prints 2 then 1. Hence it's the readonly struct here is clearly not making the spilling any safer here than for a normal struct. At least for the cases I can think of. Hence I'm unsure why we have this behavior at all.

My instinct though is that whatever the reason the rule should be that spilling is allowed when the struct receiver has type of in. That though would likely mean we need to extend to extension methods as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants