Skip to content

C# 7.x ref assignment to input parameter #839

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

Closed
KalleOlaviNiemitalo opened this issue Jul 8, 2023 · 10 comments
Closed

C# 7.x ref assignment to input parameter #839

KalleOlaviNiemitalo opened this issue Jul 8, 2023 · 10 comments
Assignees
Labels
meeting: proposal There is an informal proposal in the issue, worth discussing in a meeting type: bug The Standard does not describe the language as intended or implemented
Milestone

Comments

@KalleOlaviNiemitalo
Copy link
Contributor

Describe the bug

In the C# 7.x draft, 12.21.3 could be a bit clearer about input parameters here:

If the left operand is a writeable ref (i.e., it designates anything other than a ref readonly local or in parameter), then the right operand shall be a writeable variable_reference. If the right operand variable is writeable, the left operand may be declared ref or ref readonly.

Example

This should be valid.

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

Expected behavior

Say that the left operand may be declared in as an input parameter.

Additional context

Poking this, I met the ref-safe-to-escape rules

class C {
    ref readonly int M(in int i, int j) {
        ////i = ref j; // error CS8374
        return ref i;
    }
}

The ref assignment here looks benign on its own, but it must be rejected because the method could then return that ref. I have not checked whether the draft specifies an error here.

@jskeet jskeet added this to the C# 7.x milestone Jul 11, 2023
@jskeet jskeet added the meeting: proposal There is an informal proposal in the issue, worth discussing in a meeting label Jul 11, 2023
@jskeet
Copy link
Contributor

jskeet commented Jul 11, 2023

At the start of 12.21.3 we already have:

The left operand shall be an expression that binds to a reference variable (§9.7), a reference parameter (other than this), an output parameter, or an input parameter.

The bit you've quoted is only meant to be restricting what's used for the right operand, e.g. to prevent:

static void M(in int i)
{
    ref int j = ref i;
}

For the second part, the error occurs even without return ref (even in a void method) I believe it's prohibited already:

12.21.3: "It is a compile time error if the ref-safe-context (§9.7.2) of the left operand is wider than the ref-safe-context of the right operand."

9.7.2.2: "If p is a ref, or in parameter, its ref-safe-context is the caller-context. If p is an in parameter, it can’t be returned as a writable ref but can be returned as ref readonly." (So the ref-safe-context of i is caller-context.)

9.7.2.2: (After things that don't apply to j) "Otherwise, the parameter is a value parameter, and its ref-safe-context is the function-member." So the ref-safe-context of j is function-member.

caller-context is wider than function-member, so it's prohibited.

Proposal for the meeting: check my logic, then close the issue without further spec changes.

@jskeet jskeet self-assigned this Jul 11, 2023
@jskeet jskeet added the type: bug The Standard does not describe the language as intended or implemented label Jul 11, 2023
@KalleOlaviNiemitalo
Copy link
Contributor Author

So you're saying that this

If the right operand variable is writeable, the left operand may be declared ref or ref readonly.

is not meant to disallow other ways of declaring the left operand.

@jskeet
Copy link
Contributor

jskeet commented Jul 11, 2023

So you're saying that this

If the right operand variable is writeable, the left operand may be declared ref or ref readonly.

is not meant to disallow other ways of declaring the left operand.

Correct. If it required the left operand to be declared ref or ref readonly then it would be "shall" rather than "may".

@Nigel-Ecma
Copy link
Contributor

I think we can improve this a little, I'll put out a PR…

@Nigel-Ecma
Copy link
Contributor

@KalleOlaviNiemitalo, @jskeet – see PR #917. I think that addresses it, I think it was a “typo”

@KalleOlaviNiemitalo
Copy link
Contributor Author

KalleOlaviNiemitalo commented Sep 2, 2023

The distinction between read-only and writable refs, or more generally variable_references, still seems inadequately specified but I guess it's better to fix that as part of #894.

@jskeet
Copy link
Contributor

jskeet commented Sep 4, 2023

@KalleOlaviNiemitalo: So just to check we're on the same page, are you happy for us to merge #917 and then take a bigger swing at read-only/writable refs in a separate PR to address #894?

@KalleOlaviNiemitalo
Copy link
Contributor Author

Yes that's fine.

jskeet pushed a commit that referenced this issue Sep 4, 2023
The original text appeared to restrict the left operand to variables declared `ref` or `ref readonly` when a writeable or read-only ref is allowed.
@BillWagner
Copy link
Member

Notes for 9/6 meeting:

I read through the comments, and #917. I agree that this can be closed without further action.

@jskeet
Copy link
Contributor

jskeet commented Sep 5, 2023

Closing.

@jskeet jskeet closed this as completed Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meeting: proposal There is an informal proposal in the issue, worth discussing in a meeting type: bug The Standard does not describe the language as intended or implemented
Projects
None yet
Development

No branches or pull requests

4 participants