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

Local variable initialization is not always equivalent to immediate assignment after declaration #899

Closed
jskeet opened this issue Aug 17, 2023 · 16 comments
Milestone

Comments

@jskeet
Copy link
Contributor

jskeet commented Aug 17, 2023

From #837, in statements.md:

A local variable declaration that declares multiple variables is equivalent to multiple declarations of single variables with the same type and ref_kind. Furthermore, a variable initializer in a local variable declaration corresponds exactly to an assignment statement that is inserted immediately after the declaration.

@KalleOlaviNiemitalo wrote:

It no longer corresponds exactly, because of ref safety.

using System;
public class C {
    public void M() {
        Span<int> s1 = stackalloc int[1]; // OK
        Span<int> s2;
        s2 = stackalloc int[1]; // error CS8353
    }
}
@KalleOlaviNiemitalo
Copy link
Contributor

@jskeet, do you intend this issue to also cover that many of the ref safety rules only apply to "reference variables" and not to variables of ref-like type?

@KalleOlaviNiemitalo
Copy link
Contributor

There's also this in 13.6.2 (Local variable declarations):

However, it is a compile-time error to omit local_variable_initializer from a local_variable_declarator for a variable declared ref or ref readonly.

So you cannot transform a declaration with an initialiser, to a declaration without an initialiser, followed by an assignment.

@jskeet
Copy link
Contributor Author

jskeet commented Aug 17, 2023

I'd only expected it to cover the issue you brought up on #837. I'm not sure whether that answers your question or not, but I basically wanted to avoid that new issue from blocking the PR.

@jskeet jskeet added this to the C# 7.x milestone Aug 17, 2023
@KalleOlaviNiemitalo
Copy link
Contributor

@jskeet, it was a misunderstanding on my part. Elaborated in #837 (comment).

@jskeet
Copy link
Contributor Author

jskeet commented Aug 17, 2023

@KalleOlaviNiemitalo: Some aspects may have been a misunderstanding, but I still thing 13.6.2 needs clarification, as your example points out that the "exact correspondence" isn't the case. Or maybe I've misunderstood too :)

@Nigel-Ecma
Copy link
Contributor

The statement quoted at the start:

A local variable declaration that declares multiple variables is equivalent to multiple declarations of single variables with the same type and ref_kind. Furthermore, a variable initializer in a local variable declaration corresponds exactly to an assignment statement that is inserted immediately after the declaration.

Needs to be updated to cope with:

  1. ref & ref readonly locals: These must be initialised on declaration and so cannot be broken into declaration + assignment at all.
  2. ref struct variables such as Span<T>: These don’t need to be initialised on declaration, if they are their safe context comes from the initializer, if they are not then their safe context is caller-context. Sample output showing the results using VS2022 with LangVersion=7.3 is below.
    What this means is if there is no init, or the init is default, then only a value passed in as a parameter can be subsequently assigned; no local stackalloc is valid – which might be “surprising” and I don’t think the Standard explains this choice at the moment – but I might have missed that.

If the above sounds wrong to you please comment.

I haven’t wordsmithed a PR, yet…

Here is the output from a demo app (yes, it prints the code, the failing statements show its not code itself!):

Span<int> passedIn = stackalloc int[24];
RefSafetyDiff(passedIn)

static void RefSafetyDiff(Span<int> passedIn)
{

   /*** Declare and init with stackallo c***/
   Span<int> sA = stackalloc int[42]; // sA.Length => 42
   sA = passedIn;                     // sA.Length => 24
   sA = stackalloc int[33];           // sA.Length => 33
   sA = default;                      // sA.Length => 0

   /*** Declare only ***/
   Span<int> sB;                      // sB.Length => CS0165: Use of unassigned local variable 'sB'
   sB = passedIn;                     // sB.Length => 24
   sB = stackalloc int[33];           // => CS8353: A result of a stackalloc expression of type 'Span<int>'
                                      // cannot be used in this context because it may be exposed outside
                                      // of the containing method
   sB = default;                      // sB.Length => 0

   /*** Declare and init with default ***/
   Span<int> sC = default;            // sC.Length => 0
   sC = passedIn;                     // sC.Length => 24
   sC = stackalloc int[33];           // => CS8353: A result of a stackalloc expression of type 'Span<int>'
                                      // cannot be used in this context because it may be exposed outside
                                      // of the containing method
   sC = default;                      // sC.Length => 0
}

@KalleOlaviNiemitalo
Copy link
Contributor

There's also a minor complication with an array initializer, as int[] a = { 1 } is not equivalent to int[] a; a = { 1 }; but rather to int[] a; a = new int[] { 1 };.

@Nigel-Ecma
Copy link
Contributor

@KalleOlaviNiemitalo – I think array initializers are a “minor complication”, from §17.7:

int[] a = {0, 2, 4, 6, 8};

it is simply shorthand for an equivalent array creation expression:

int[] a = new int[] {0, 2, 4, 6, 8};

So once the shorthand is expanded the equivalence holds.

However there may well be other cases, both now and in the future, when the equivalence does not hold. Wordsmithing the exceptions is just messy. There is one obvious solution worth considering…

@Nigel-Ecma
Copy link
Contributor

Oops, fixed the title, its now:

Withdraw ”corresponds exactly” statement regarding initialization vs. declaration + assignment #909

@KalleOlaviNiemitalo
Copy link
Contributor

I was thinking that one could even add a note like "the initialiser can affect the safe-context of the local variable (16.4.12.3)". However, 13.6.2 already has this that can be a sufficient hint:

A local_variable_initializer for a variable declared ref or ref readonly shall be of the form “ref variable_reference”. It is a compile time error if the scope of the local variable is wider than the ref-safe-context of the variable_reference (§9.7.2).

But… now I don't understand what kind of code would trigger the compile-time error that is specified here.

@Nigel-Ecma
Copy link
Contributor

@KalleOlaviNiemitalo wrote:

But… now I don't understand what kind of code would trigger the compile-time error that is specified here.

If you are referring to “CS8353: A result of a stackalloc expression of type 'Span' cannot be used in this context because it may be exposed outside of the containing method” as you originally reported and shown in the sample output above then:

  • “A default expression, for any type, has safe-context of caller-context.” §16.4.12.1
  • “Otherwise the variable [declared without an initializer] is uninitialized at the point of declaration and has a safe-context of caller-context.” §16.4.12.3
  • So the variables sB & sC above have a safe-context of caller-context
  • “The result of a stackalloc expression has safe-context of function-member.” §16.4.12.7
  • So the assignments sB = stackalloc int[33];, ditto for sC, fail as they are trying to assign to a wider context. While the assignments sB = passedIn;, ditto for sC, are OK as the contexts match. The declaration Span<int> sA = stackalloc int[42]; gives sA function-member, and the assignment sA = passedIn; is OK as it is from a wider to a narrower context.

@KalleOlaviNiemitalo
Copy link
Contributor

@Nigel-Ecma, I understand a compile-time error due to ref-safe-context mismatch in ref assignment, but the quoted paragraph is for a local_variable_initializer.

@KalleOlaviNiemitalo
Copy link
Contributor

KalleOlaviNiemitalo commented Aug 18, 2023

In other words, for a local variable declaration

ref T local_variable = ref variable_reference;

What kind of variable_reference would trigger a compile-time error because "the scope of the local variable is wider than the ref-safe-context of the variable_reference"? Does the sentence mean a reference to a variable that has not been declared yet:

ref int a = ref b;
int b = 0;

IIRC there is some other text that disallows this case so it does not seem the purpose of the sentence.

@Nigel-Ecma
Copy link
Contributor

@KalleOlaviNiemitalo – I see what you're talking about now. This isn't the same thing as this issue and as there is now PR #909 this issue may be closed as done which will lose your comment. I suggest you put it in another issue.

In other words, for a local variable declaration

ref T local_variable = ref variable_reference;

What kind of variable_reference would trigger a compile-time error because "the scope of the local variable is wider than the ref-safe-context of the variable_reference"? Does the sentence mean a reference to a variable that has not been declared yet:

ref int a = ref b;
int b = 0;

IIRC there is some other text that disallows this case so it does not seem the purpose of the sentence.

jskeet pushed a commit that referenced this issue Aug 21, 2023
This addresses #899.

As explained there the statement “a variable initializer in a local variable declaration corresponds exactly to an assignment statement that is inserted immediately after the declaration” is not strictly true.

However describing the exceptions would at best make a simple statement rather more complicated.

This begs the question “What is the value of the statement?”

This PR answers that one way – by just deleting it.

Agree or disagree in your reviews (so yes, this is a “poll” which if passed automatically gets implemented).
@jskeet
Copy link
Contributor Author

jskeet commented Aug 21, 2023

@KalleOlaviNiemitalo: Is #909 enough to close this, or is there more needed, as far as you're aware?

@KalleOlaviNiemitalo
Copy link
Contributor

It looks sufficient to me.

@jskeet jskeet closed this as completed Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants