New Issue: What is the value of a sync variable after readFE?

Notifications Issues
1

18477, "mppf", "What is the value of a sync variable after readFE?", "2021-09-28T20:04:41Z"

Consider the following program.

var x: sync int;

x.writeEF(1);
var y = x.readFE();
assert(y == 1); // OK this makes sense

var z = x.readXX();
writeln(z); // What value should z have? 0? 1?

My draft PR #18379 changes the behavior of this program. Before that PR, z has the value 1, because, readFE() sets the sync to "empty" but doesn't change its value. Currently, with the PR, z has the value 0, because readFE() "moves" out from the sync value (rather than copy-initializing) and then default-initializes the value. Intuitively, it makes sense to me that "setting the sync variable to empty will clear its value" since "empty" means the lack of a value. Additionally, I think that patterns of writeEF and readFE are the normal use of sync variables that we should optimize for.

For sync owned C?, the value type can't be copied (without modifying the RHS). So, for such types, I think we need the "move"+"default init" behavior rather than the "copy" behavior.

For what it's worth, the spec seems to be written to define the behavior in terms of separate operations on full/empty and the value, and it does not say that readFE changes the value. (It doesn't say explicitly it preserves the value, either; but it is arguably implied).

It would be fairly easy to make the PR preserve the old behavior for numeric types. However I think that the new behavior makes more sense for records/strings/etc, because it avoids copy-initializing the record in most cases. If we changed the behavior for numeric types only, that would make it inconsistent in a way.

So I can see 3 possibilities here:

  1. If the value type can be copied, readFE() copies the value out instead of moving it, so that the old value remains
  2. Do the above but only for numeric types or maybe POD types
  3. readFE() always moves the value out and default-initializes the "empty" sync variable's value.

Option 3. is what I have implemented currently and what I think is the most natural. In particular, it has consistent behavior for all value types.

In testing, this difference in behavior only comes up for test/arrays/deitz/jacobi*.chpl, and there, it would be trivial to modify the test to not assume that the empty sync variable has a meaningful value.

Thoughts?