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

Feature Request: more convenient mark-commit interface for fileReader.read #22676

Open
jeremiah-corrado opened this issue Jul 6, 2023 · 1 comment

Comments

@jeremiah-corrado
Copy link
Contributor

jeremiah-corrado commented Jul 6, 2023

Limitation with current design:

Currently, the IO module provides a mechanism for "undoing" a read operation when an error is encountered. This makes it easy to do speculative reading, where one would like to move the fileReader position back to its starting point after a failure to read some particular type or format from a file.

This is achieved using the mark, commit, and revert methods which provide a semi-low-level interface for controlling the fileReader position. See the following example where the tryReadC procedure will revert the channel position if a C cannot be read:

use IO;

class C {
  var x: int;
  var y: bool;
  var z: 3*real;
}

proc tryReadC(fr: fileReader): C throws {
  var c: C;
  fr.mark(); // mark a starting offset
  try {
    fr.read(c);
  } catch e {
    fr.revert(); // an error was encountered; revert to the start
    throw e;
  }
  fr.commit(); // no errors were encountered; pop from the mark stack
  return c;
}

try {
  var c = tryRead(openReader("input.txt"));
} catch e {
  // try to read something else instead? ...
}

The marking logic is useful here because without it, failing to read a C would leave the fileReader position at some offset part-way through C's fields in the input. For example, if the first field was read successfully but the second wasn't, the offset would be left somewhere after the first integer in the input. This prevents the user from trying to read something else where the C started.

Although useful, the mark-commit-revert logic tends to be somewhat verbose and is likely to have repeated boilerplate used across a variety of situations. One could even imagine writing their own generic wrapper around read to avoid the repeated code:

proc revertingRead(fr: fileReader, ref args: ...) throws {
  fr.mark();
  try {
    fr.read((...args));
  } catch e {
    fr.revert();
    throw e;
  }
  fr.commit();
}

Proposals:

To make "auto-reverting reads" simpler, there are a few potential improvements that could be made to the fileReader interface:

  1. Allow fileReader's to be configured in an "auto-mark" mode, such that all read calls will mark and revert on errors internally. This way, the above tryReadC procedure wouldn't be needed, and the try-catch block could be replaced with:
try {
  var c = openReader("input.txt", autoMark=true).read(C);
} catch e {
   // try to read something else instead? ...
}
  1. Add an optional argument to the read methods to enable marking (this would require solving a know limitation with combining var-args and optional arguments). Again, this would simplify the try-catch block in the above example to:
try {
  var c = openReader("input.txt").read(C, mark=true);
} catch e {
   // try to read something else instead? ...
}
  1. Use context managers to control marking behavior of a fileReader (see: I/O module: express channel mark/revert/commit via context manager #19611 for more).
var fr = openReader("input.txt");
manage fr as offset {
  fr.read(C);
}

For the first two proposals, the formal names would need to be decided. Additionally, these are all non-breaking changes that could be added post 2.0.

@lydia-duncan
Copy link
Member

Another option would be to add a read overload that indicates it marks and reverts/commits as part of the name, e.g.

try {
  var c = openReader("input.txt").readMarked(C);
} catch e {
   // try to read something else instead? ...
}

Adding this would still be a non-breaking change (but there are already a lot of read methods on fileReaders today, so it might feel more cluttered)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants