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

Variable from using cannot be used as ref or out #26313

Open
sharwell opened this issue Apr 21, 2018 · 16 comments
Open

Variable from using cannot be used as ref or out #26313

sharwell opened this issue Apr 21, 2018 · 16 comments

Comments

@sharwell
Copy link
Member

sharwell commented Apr 21, 2018

Version Used: 15.7 Preview 4

Steps to Reproduce:

using System;
public class C {
  public void M() {
    using (var d = new D())
    {
      d.Move();
      d.Move2(); // error CS1657: Cannot use 'd' as a ref or out value because it is a 'using variable'
    }
  }
}

public struct D : IDisposable
{
  public void Move() { }
  public void Dispose() { }
}

public static class DExtensions
{
  public static void Move2(ref this D d) { d.Move(); }
}

Expected Behavior:

The calls to Move() and Move2() are equivalent.

Actual Behavior:

The call to Move2() results in the error described.

Additional Information:

The call to Move() shows that invocations using a reference to a value used in a using statement are allowed in some contexts. In these contexts, CS1657 appears to be an arbitrary language limitation that could be removed. Allowing by-ref calls in this context would simplify the implementation of an allocation-free OwnedDisposable<T>, which I'm creating as part of an experimental way to address dotnet/roslyn-analyzers#1617. This limitation forces the code to use try/finally instead of the equivalent using statement.

@jcouv
Copy link
Member

jcouv commented Apr 21, 2018

@OmarTawfik Could you take a look?

@jcouv jcouv added this to the 15.8 milestone Apr 21, 2018
@OmarTawfik
Copy link
Contributor

@jcouv @sharwell even if we chose to remove that limitation, we will have to restrict that to a language version (7.4 or 8.0), which I don't know if it is in the near future.

@jcouv jcouv changed the title 'ref this' does not behave like member method inside using statement Variable from using cannot be used as ref or out Apr 23, 2018
@jcouv jcouv removed this from the 15.8 milestone Apr 23, 2018
@jcouv jcouv added this to the 16.0 milestone Apr 23, 2018
@jcouv jcouv added the Resolution-By Design The behavior reported in the issue matches the current design label Apr 23, 2018
@jcouv
Copy link
Member

jcouv commented Apr 23, 2018

Confirmed this is by-design.
See the language spec:

Local variables declared in a resource_acquisition are read-only, and must include an initializer. A compile-time error occurs if the embedded statement attempts to modify these local variables (via assignment or the ++ and -- operators) , take the address of them, or pass them as ref or out parameters.

The rationale is that we'll need to dispose the resource, so we can't let you point the variable to something else (we would lose the resource that needs disposing).

@jcouv jcouv closed this as completed Apr 23, 2018
@sharwell sharwell reopened this Apr 30, 2018
@sharwell
Copy link
Member Author

sharwell commented Apr 30, 2018

@jcouv Reopening the issue because the Expected behavior and Actual behavior statements remain correct. The specification language you found only impacts the Additional information section of what I wrote.

@sharwell sharwell removed the Resolution-By Design The behavior reported in the issue matches the current design label Apr 30, 2018
@Neme12
Copy link
Contributor

Neme12 commented Apr 30, 2018

@sharwell I'm sorry but the spec clearly mentions "or pass them as ref or out parameters" which would suggest that this is by design, so why is the expected behavior the one you're describing? Isn't this a language feature request as opposed to a compiler bug?

@Neme12
Copy link
Contributor

Neme12 commented Apr 30, 2018

arbitrary language limitation that could be removed

Yes and I understand this limitation might be frustrating but it is still a language limitation and not a compiler issue, so it would seem more appropriate for csharplang.

@sharwell
Copy link
Member Author

sharwell commented Apr 30, 2018

@Neme12 The resource variable is a value type. The behavior inconsistent with the current specification is the call d.Move(), which is currently passing the resource by reference (including allowing the resource to be overwritten).

If a deviation from the language of the specification is allowable in that instance, then it should also be allowable for the call to d.Move2() because the semantics of the calls are intended to be equivalent.

@Neme12
Copy link
Contributor

Neme12 commented Apr 30, 2018

Yes I know that an instance method can still modify the struct and you have a valid point to say it's logically inconsistent, but unfortunately it's consistent with what the specification says:

A compile-time error occurs if the embedded statement attempts to modify these local variables (via assignment or the ++ and -- operators) , take the address of them, or pass them as ref or out parameters.

There is no mention of instance methods being illegal but ref and out parameters are clearly forbidden.

@Neme12
Copy link
Contributor

Neme12 commented Apr 30, 2018

If a deviation from the language of the specification is allowable in that instance

Where do you see a deviation from the specification?

Yes this is passed by reference, but I don't think this counts as a "ref or out parameter" in the language.

@sharwell
Copy link
Member Author

@Neme12 You can see the impact that read-only has on by-ref call here:

https://sharplab.io/#v2:EYLgtghgzgLgpgJwDQxASwDZICYgNQA+AAgAwAERAjANwCwAUEQMwUBMZAwmQN4Nn9kEcCNgD2AOwwBPMgBEIMCIOFjJU+Yrr0BchUux6tfAcwoAWMgFkAFAEoexnfyEiJ0jRAB0luDAAWoth2Wk4CBorevgFBtiECAL4MifQMsAgArgDGMLqKDtomLEQWPv6BdjxkyclAA=

Notice that readonlyData.Method() operates on a temporary variable, while data.Method() uses ldflda to operate directly on the field.

If you modify the sample to review a using statement, you can see that the current implementation is treating the using variable as not readonly, and passing a reference to the using variable to the instance method.

https://sharplab.io/#v2:EYLgtghgzgLgpgJwDQxASwDZICYgNQA+AAgAwAERAjANwCwAUEQMwUBMZAwmQN4Nn8UWRACxkAsgAoAlDz4D5pMhIBuEBGWwQYEMgF4yAOzgB3MgBEtEaVLnz+venacbLAOjFwYACwD22aXSOzgC+tmSh9BEMsAgArgDGMOaWZCBkAJJmaFAADj5QEMAYcLJB/MwUolm5+XDSPOFhFSLinr7+MtyNkQxAA==

@Neme12
Copy link
Contributor

Neme12 commented Apr 30, 2018

Yes I'm aware of the semantics of readonly fields & structs.

@sharwell
Copy link
Member Author

Where do you see a deviation from the specification?

When calling instance methods of value types, the resource variable is treated as not read-only and is passed by reference. The spec says a compilation error should occur (or rather, that a reference to a copy should be passed). The current implementation behavior does not align with the specification for this case.

One thing working in favor of this issue is this may be a long-standing behavior of the compiler. In cases like this, the compiler has occasionally been left intentionally deviating from the specification. In addition, the current implementation fails to compile for the case where I've requested a change, so the change will not result in a silent change in behavior for any currently-valid code. The situtaion may allow the implementation to be modified for consistency while the language gets updated.

@Neme12
Copy link
Contributor

Neme12 commented Apr 30, 2018

@sharwell
I don't think the compiler is deviating from the spec at all. It is well documented that a reference to a field will produce a value (therefore a copy) as opposed to a variable if the field is readonly:

  • If T is a struct_type and I identifies an instance field of that struct_type:
    * If E is a value, or if the field is readonly and the reference occurs outside an instance constructor of the struct in which the field is declared, then the result is a value, namely the value of the field I in the struct instance given by E.
    * Otherwise, the result is a variable, namely the field I in the struct instance given by E.

See https://github.com/dotnet/csharplang/blob/master/spec/expressions.md#member-access

Note that this only applies to readonly fields, not readonly variables (such as a using or foreach statement variable). It looks to me like the behavior you're seeing in both cases is currently by design.

@Neme12
Copy link
Contributor

Neme12 commented Apr 30, 2018

It also seems to apply to readonly references but those are not in the spec so I won't comment on that.

@sharwell
Copy link
Member Author

sharwell commented Apr 30, 2018

I don't think the compiler is deviating from the spec at all. It is well documented that a reference to a field will produce a value (therefore a copy) as opposed to a variable if the field is readonly:

This same text states the behavior would be equivalent if E is classified as a value. The specification appears to be ambiguous regarding the classification of E for a simple name referencing a using or foreach variable:

https://github.com/dotnet/csharplang/blob/master/spec/expressions.md#simple-names

  • If K is zero and the simple_name appears within a block and if the block's (or an enclosing block's) local variable declaration space contains a local variable, parameter or constant with name I, then the simple_name refers to that local variable, parameter or constant and is classified as a variable or value.

@Neme12
Copy link
Contributor

Neme12 commented Apr 30, 2018

This same text states the behavior would be equivalent if E is classified as a value

Yes. In this example: field.Mutate(), E.I would be this.field. If we are accessing the field of something other than this that is only a value (maybe another readonly field), then the result would be a value too, that makes sense to me.

and is classified as a variable or value.

Hm, this wording is a little unfortunate. Maybe it is clarified elsewhere but I'm not going investigate.

@jinujoseph jinujoseph removed this from the 16.0 milestone Jun 9, 2019
@jinujoseph jinujoseph added this to the 16.3 milestone Jun 9, 2019
@jcouv jcouv modified the milestones: 16.3, Compiler.Next Jul 8, 2019
@jaredpar jaredpar modified the milestones: Compiler.Next, Backlog Sep 12, 2023
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

6 participants