New Issue: IO Module: When to use channel.read variant that returns a boolean?

20173, "benharsh", "IO Module: When to use channel.read variant that returns a boolean?", "2022-07-07T20:43:38Z"

While replacing the IO operator over on #19985 , I assumed that it would be trivial to replace such uses of the operator with calls to the version of channel.read that returns a boolean. They both take a value by reference, so it seemed natural. The problem is that the IO operator throws on EOF errors, whereas the channel.read method in question returns a boolean that indicates whether an EOF error was encountered.

proc MyRecord.readThis(r : channel) {
  r <~> fieldOne;
  r <~> fieldTwo;

  // becomes
  r.read(fieldOne);
  r.read(fieldTwo);
}

This bool-returning style is handy at a high level when trying to read some unknown number of items from a channel:

var x : int;
while myReader.read(x) do
  foo(x);

But when using this method in a more principled/structured context, it can be easy to drop that boolean without examining it:

proc R.readThis(r : channel) {
  r.read(fieldOne);
  r.read(fieldTwo);
}

This seems like a relatively intuitive way of implementing a readThis method, but unfortunately sidesteps error handling of EOF errors in a non-obvious way. A practical consequence of this I encountered during my work was that several tests failed with timeouts. These timeouts were caused by infinite loops that attempted to read an unknown (but finite) number of items from a channel. By using the bool-returning version of channel.read I neglected to check for EOF errors, and in doing so failed to establish the conditions that would terminate the loop.

Some questions:

How should users replace the IO operator?

In a recent meeting it was suggested that we should recommend replacing the IO operator with calls to the other channel.read method, the one that returns a value and accepts type arguments. This version will indeed throw EOF errors. It's not quite the same as the IO operator, which passed the argument in by reference, and will instead use assignment:

proc R.readThis(r : channel) {
  fieldOne = r.read(fieldOne.type); // or could use the types directly if non-generic, or parameterized
  fieldTwo = r.read(fieldTwo.type);
}

Should we have some general guidance for channel.read?

Should we go out of our way to recommend use-cases for the different channel.read variants? We could potentially recommend channel.read(type) by default, and go out of our way to emphasize channel.read(ref val) : bool as a "convenience" method.

Should we warn for unused results?

See: Should the Chapel compiler support an option like -Wunused? · Issue #20172 · chapel-lang/chapel · GitHub