should split-init consider mentions inside of nested functions?

[mppf]

On master (and in the 1.22 release), split-init behavior depends on whether or not a called function using the variable is declared between the declaration and initialization or not. This issue asks what should be done about this inconsistency.

This case split-inits x and raises a compilation error as I would expect:

proc main() {
  var x: sync int;
  inner();
  x = 23;
  proc inner() {
    writeln(x.readXX());
  }
}

As does this module-scope equivalent:

var x: sync int;
function();
x = 29;

proc function() {
  writeln(x.readXX());
}

However the following two cases do not split-init x at normalization time. (I think the split init code during normalize is delving into the body of inner). (The reason I came across this is that PR #15755 makes the split-init handling during resolution time slightly more general and it doesn't share this oddity).

Here is the nested function version, which does not split init x on master/1.22:

proc main() {
  var x: sync int;
  proc inner() {
    writeln(x.readXX());
  }
  inner();
  x = 23;
}

Here is the module-scope function version, which does not split init x on master/1.22:

var x: sync int;
proc function() {
  writeln(x.readXX());
}
function();
x = 23;

When the compiler is looking at mentions of x to decide if it can be split-inited, should it delve into functions between the declaration and the candidate assignment?

If the answer is "yes", then the resolution-time split-init code needs to have some adjustment to consider these "uses". If the answer is "no", then the normalize-time split-init code needs to stop delving into functions.

[bradcray]

[For those who didn't arrive on this issue from #15756, it's worth noting that nothing about the examples in this issue are related to the fact that the variable is a sync).

Getting these consistent definitely seems important. It's less obvious to me which way they should go.

What do you view the tradeoffs between the two approaches as being? (you say "as I would expect" for the first which makes me think that that's your preferred approach?).

The error seems less likely to cause surprises if a function uses (are starts using) the outer-scope variable, so I might have a slight preference in that direction. It'd be even more attractive if there were a straightforward language-level way to force default initialization for variable x as we were kicking around on #15756 (e.g., var s: t = init;).

I initially worried that delving into the functions might cause problems in more of a separate-compilation world, but thinking about it more, separately compiled functions couldn't refer to outer variables through lexical scoping, so I don't think that's an issue. I guess compile-time is the main downside? (but probably worth the lack of surprises).

[mppf]

Getting these consistent definitely seems important. It's less obvious to me which way they should go.

I don't think split-init should delve into functions based upon function calls; so I think the first case like

proc main() {
  var x: sync int;
  inner();
  x = 23;
  proc inner() {
    writeln(x.readXX());
  }
}

should definitely be an error (as it is now and in 1.22). Changing it to not split-init instead would seem to go back on the direction we arrived at in issue #15012. My rationale at the time was "make the behavior simpler but add checking" as described in https://github.com/chapel-lang/chapel/issues/15012#issuecomment-592939080 . I was also concerned that making the behavior depend on the bodies of these functions would make separate compilation significantly harder.

I don't necessarily think that the second case necessarily needs to be an error:

proc main() {
  var x: sync int;
  proc inner() {
    writeln(x.readXX());
  }
  inner();
  x = 23;
}

We could consider the function declaration of inner as a block of code that we scan for mentions. This already happens in 1.22 - but inconsistently - depending on if the split init works through out intent or not. However this is sensitive to the placement of the definition of proc inner which I think is something to avoid (and not generally something the language does).

So my preference is that both of these cases become compilation errors.

The error seems less likely to cause surprises if a function uses (are starts using) the outer-scope variable, so I might have a slight preference in that direction.

So are we agreeing?

[bradcray]

So my preference is that both of these cases become compilation errors.

So are we agreeing?

I think so, thanks.

[mppf]

Addressed by PR #15773.