Specify temporary for this as an input parameter

[BillWagner]

Fixes #894

Issue #894 has several comments that describe different possible concerns. To help reviewers (and to keep my head around each different scenario). Each commit will fix one of the concerns.

The first commit fixes the concern in the description:

If a method takes an in parameter of a struct type that is not a readonly struct, then the compiler creates a temporary to invoke a (non-readonly) member of that type. Note that for C# 8.0, this will need to be revisited again to note that a copy needn't be created to invoke a readonly member of a non-readonly struct.

[BillWagner]

Regarding #894 (comment) and #894 (comment)

I don't believe a spec change is needed to prohibit a defensive copy for a readonly struct as an input_parameter when a member is called. The fact that a defensive copy isn't required should be sufficient to allow compiler writers freedom of implementation.

[BillWagner]

Regarding #894 (comment) (what about System.Enum.ToString() and related methods?

I think this can be something we leave to implementations as an optimization. In the case of roslyn, they may avoid the defensive copy because the compiler "knows" that System.Enum.ToString() doesn't modify the instance.

Without writing contrived code, you can't observe the lack of a defensive copy.

Question for the committee: Should we add a note that a specific implementation may be able to avoid the defensive copy when the behavior of a specific method is well known?

[BillWagner]

Regarding #894 (comment) Is the readonly struct special case also required when the parameter is not classified as a variable?

I don't think any changes are needed. If E is not classified as a variable, a copy must be made, because this is classified as a ref parameter. (Starting in C# 11, this is classified as a scoped ref parameter, which makes the ref safety rules more consistent.) The result is that a temporary variable must be created for E.

[BillWagner]

Regarding #894 (comment) (other readonly ref variables

2nd commit updates the restriction to include readonly fields, ref readonly local variables and ref readonly returns.

[BillWagner]

Regarding #894 (comment)

As another example, I think draft-v7 does not currently forbid this, even though it should:

class C {
   void M(ref int i) {}
    void N(in int i) {
        M(ref i);
    }
}

I think the standard does prohibit this via §15.6.2.3 ("It is a compile-time error to modify the value of an input parameter.") A reasonable interpretation of passing a parameter as a ref parameter is that it is modified in the body of that method. For committee discussion. If a change is needed, we should prohibit passing an in parameter as a ref or out argument to another method, except as the receiver.

[BillWagner]

The final two comments on #894 detail where we could take the strategy of defining a *variable_reference* and *writable_variable_reference* to distinguish.

I think the proposed changes fix the issue, and are smaller in scope.

[BillWagner]

Tagging @cston to take a look at this PR and my notes regarding #894. I'd like you to make sure these changes to prohibit any optimizations for in parameters.

[BillWagner]

Regarding #894 (comment)

Example:

struct S {
    public void M() {}
}
class C {
    void N(bool b, ref S r, in S i) {
        (b ? ref r : ref i).M();
    }
}

I believe this is covered, but it requires a look at 12.18:

If ref is present:
...

• The result is a variable_reference, which is writable if both variable_references are writable.

This example highlights why I'm trying to avoid limitations on implementers. This doesn't require a defensive copy of the ref parameter. However, the semantics of an in parameter require the defensive copy when the result is the in parameter.

If a defensive copy were required regardless of the result, a compliant compiler would be forced to create that copy, even when it isn't necessary. The motivation for all the ref safety features is to improve performance by providing techniques to avoid both allocations and copies.

We could make a change to 12.18, but I think the description of in parameters already enforces the defensive copy there. We should also avoid mandating a copy when the result is a writable variable reference.

[jskeet]

A couple of comments, but generally okay.

[Nigel-Ecma]

Regarding #894 (comment) (what about System.Enum.ToString() and related methods?

Question for the committee: Should we add a note that a specific implementation may be able to avoid the defensive copy when the behavior of a specific method is well known?

Can we not say that for all defensive copies? I.e. that a compiler can omit any of them if it knows the defense is not required? Indeed can we not specify that normatively as in “a defensive copy shall be made unless the compiler can determine it is not required”?