roslyn icon indicating copy to clipboard operation
roslyn copied to clipboard

Ref and in parameters used in ternary vs if statement

Open NinoFloris opened this issue 3 years ago • 4 comments

Version Used: C# compiler included in 7.0 rc1

Steps to Reproduce:

https://sharplab.io/#v2:EYLgtghglgdgPgJwKYQCYHsYBsCeACAZwBcEBXAYyLwCEoBzAZRIqIFgAoAbw717w4C+HDsgBmhZpTwAxdOj4du7PngACAZjUBGAGwy5eAMLIIRJAAoAlDz5KVK1QHY8MJAHd96KwG4bvIcoKgbwa2nqy8sYoZuawNPRMZFLACZJE1sF4dvYhzq4eET5+eAEBKhyhqgBMRlnFqgAseADyANLmYvGMaQD8eCndSel1mSoAbhAIeKIGALz9qUN4UAQu6FQwpFhYeH0RAHRRphYDiSz7AGoQWKRIlnggnocmMZbeObzFZUEOTQCiCAQ6AQHSQ4logxYfVOaXu2RyEWmcl8oz4UHE5hhSxWaw2WywGQ+9hm8nmByOMSx5yuNzuKKJSCwBCQ/FRORJeDJcme0Qsby+giAA===

Adding scoped to Foo.Create(scoped in BigStruct bigStruct) does allow it to compile in both versions of the compiler. It feels like the ternary case should produce the same errors but its a missed case in the analysis today. (As an aside, the error messages are fairly obtuse, hopefully they'll still get some love to become a bit more actionable)

Expected Behavior:

Either errors on both or on neither method. This may be new behavior to make ref fields sound? In that case it should probably error at both the ternary and if methods.

Actual Behavior:

An error appears on the "Error" method but not on the "OK" method. Previous versions of the compiler (take sharplab x64 as an example) accept both methods.

NinoFloris avatar Sep 22 '22 19:09 NinoFloris

It's a little obtuse, but the issue is really that a local's escape scope is determined by its initializer, and if no initializer is used, then it defaults to calling method. This can be demonstrated by an edited version of your example.

The reason this scenario changed is that in C# 11, we assume that references can escape into ref struct return values. In C# 10, we assumed that this escape does not happen.

RikkiGibson avatar Sep 22 '22 22:09 RikkiGibson

Thanks for the reply, it's interesting to understand how the initializer plays its role here.

Two questions: EDIT: First question seems obvious in hindsight, you can pass a ref struct to an in reference parameter, which could then carry a ref field that flows back out of the return value. Still, it might be nice to loosen the restrictions if in references a normal - non ref struct - type?

  1. What I don't understand so well is why in parameters are subject to this new escape scope analysis. So far as I know you can't ever store an in reference on a ref field nor on a readonly ref field. So how would such a reference flow back out through a ref struct return value?
  2. Are there some notes on why a local's escape scope without an initializer defaults to the calling method? I'd like to understand better why this default is desirable over the alternatives. Is this primarily because unscoped is not (yet) a keyword and just an attribute?

At this point, having this knowledge I'm still a bit lost as to why this does not compile: https://sharplab.io/#v2:EYLgtghglgdgPgJwKYQCYHsYBsCeACAZwBcEBXAYyLwCEoBzAZRIqIFgAoAbw717w4C+HDsgBmhZpTwAxdOj4du7PngACAZjUBGAGwy5eAMLIIRJAAoAlDz5KVK1QHY8MJAHd96KwG4bvIcoKgbwa2nqy8sYoZuawNPRMZFLACZJE1sF4dvYhzq4eET5+eAEBKhyhqgBMRlnFqgAseADyANLmYvGMaQD8eCndSel1mSoAbhAIeKIGALz9qUN4UAQu6FQwpFhYeH0RAHRRphYDiSz7AGoQWKRIlnggnocmMZbeObzFZUEOTQCiCAQ6AQHSQ4logxYfVOaXu2RyBHI6AADkhUJ5pnJfKM+FBxOYYUsVmsNlssBkPvYZvJ5gcjjFCecrjc7tjKUgsAQkPwcTlqXhaXJntELG8voIgA=

Looking at this example and setting aside my previous question of why in would even be subject to these rules. How would foo escape its scope here, given it is marked scoped?

NinoFloris avatar Sep 23 '22 00:09 NinoFloris

So far as I know you can't ever store an in reference on a ref field nor on a readonly ref field.

ref struct Foo
{
    public ref readonly int Ref;
    public Foo(in int r) => Ref = ref r;
}

ufcpp avatar Sep 23 '22 01:09 ufcpp

Heh right ref readonly, how I didn't try that.

FYI, I updated my answer as there are other ways to get a ref to flow out of a method with an in parameter.

NinoFloris avatar Sep 23 '22 01:09 NinoFloris

I wasn't around when the original decision was made, so I can only speculate. There might be LDM notes in the csharplang repo related to it. My guess is that it was viewed as simpler/more useful if the local RS rs = default; had the same escape scope as RS rs;.

RikkiGibson avatar Sep 23 '22 20:09 RikkiGibson

My guess is that it was viewed as simpler/more useful if the local RS rs = default; had the same escape scope as RS rs;.

I don't believe there are notes to this effect but that is the conclusion that was reached. Essentially more code in the world compiled with this set of defaults hence it was the better default.

jaredpar avatar Oct 10 '22 23:10 jaredpar

@jaredpar I would still love to hear an answer on

At this point, having this knowledge I'm still a bit lost as to why this does not compile: https://sharplab.io/#v2:EYLgtghglgdgPgJwKYQCYHsYBsCeACAZwBcEBXAYyLwCEoBzAZRIqIFgAoAbw717w4C+HDsgBmhZpTwAxdOj4du7PngACAZjUBGAGwy5eAMLIIRJAAoAlDz5KVK1QHY8MJAHd96KwG4bvIcoKgbwa2nqy8sYoZuawNPRMZFLACZJE1sF4dvYhzq4eET5+eAEBKhyhqgBMRlnFqgAseADyANLmYvGMaQD8eCndSel1mSoAbhAIeKIGALz9qUN4UAQu6FQwpFhYeH0RAHRRphYDiSz7AGoQWKRIlnggnocmMZbeObzFZUEOTQCiCAQ6AQHSQ4logxYfVOaXu2RyBHI6AADkhUJ5pnJfKM+FBxOYYUsVmsNlssBkPvYZvJ5gcjjFCecrjc7tjKUgsAQkPwcTlqXhaXJntELG8voIgA=

Looking at this example and setting aside my previous question of why in would even be subject to these rules. How would foo escape its scope here, given it is marked scoped?

NinoFloris avatar Oct 10 '22 23:10 NinoFloris

At the call site of Foo.Create(in BigStruct), a value argument is being passed rather than an in-reference. Therefore a local variable scoped to the if-statement is automatically declared and passed by in. We think a reference to this local could be captured in the result of the call, so we give an error for attempting to assign it to a scoped variable from a wider scope.

One potential fix at the call site might look like this: SharpLab.

RikkiGibson avatar Oct 10 '22 23:10 RikkiGibson

OK! I did indeed see the hidden variable in sharplab, and nullable is a bit special here which is why I picked it ;) What surprises me is that in the ternary this is somehow ok, and not seen as potentially capturable (nevermind that you can't get to that variable in the if statement case either).

I get the difference in scope, the ternary being one scope entirely while the if statement has separate scopes per branch, it just seems like something that should 'just' work

NinoFloris avatar Oct 10 '22 23:10 NinoFloris

i get the difference in scope, the ternary being one scope entirely while the if statement has separate scopes per branch, it just seems like something that should 'just' work

Unfortunately the way our temporaries are defined as that they exist in the scope of the statement in which they are created. This is a long standing approach and it just falls out in this case.

jaredpar avatar Oct 11 '22 00:10 jaredpar

Heh I noticed you had a similar "why doesn't this work" moment in the email ;)

It's all wonderfully tricky. Good luck to you all having to explain these new intricacies, hopefully not to an exceedingly broad audience.

NinoFloris avatar Oct 11 '22 00:10 NinoFloris

Heh I noticed you had a similar "why doesn't this work" moment in the email ;)

Yeah it is subtle but it does follow from how all the temp rules work.

At a glance it's likely tempting to say the temp lifetime should be the same as the smallest input then the lifetimes work out. That is easy for cases like this but I worry it would get really complex for other cases. If we see enough reports of this we can look into that.

jaredpar avatar Oct 11 '22 03:10 jaredpar