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

Error trying to read map from file #20004

Closed
benharsh opened this issue Jun 14, 2022 · 5 comments
Closed

Error trying to read map from file #20004

benharsh opened this issue Jun 14, 2022 · 5 comments

Comments

@benharsh
Copy link
Member

Summary of Problem

I was updating the reading and writing implementation of various things, and observed this error while trying to confirm something unrelated. In the sample test below we print a map to a file and try to read it back in, but encounter a formatting error involving the ending curly-brace.

Steps to Reproduce

Source Code:

use IO;
use Map;

proc main() {
  var f = openmem();

  var m = new map(int, real);
  for i in 1..10 do m[i] = (i**2):real;

  var w = f.writer();
  w.write(m);

  {
    var r = f.reader();
    var contents : string;
    r.readstring(contents);
    writeln("Wrote:");
    writeln("==========");
    writeln(contents);
    writeln("==========");
  }

  var r = f.reader();
  var x : map(int,real);
  try {
    r.read(x);
    writeln(x);
  } catch (e : Error) {
    writeln("Error reading map:");
    writeln(e.message());
  }

}

This program fails with the following message when attempting to read the map back in:

bad format: missing expected literal (while reading ioLiteral "}" with path "unknown" offset 1)

Associated Future Test(s):
test/library/standard/Map/testMapIO.chpl #19983

@bradcray
Copy link
Member

I hate to be a pessimist, but I feel as though I would not be surprised if read() on most of the collection types in the standard library had similar issues (?).

@benharsh
Copy link
Member Author

benharsh commented Jun 14, 2022

I think there's a good chance of that. In going through some of the recent IO work I noticed a number of expressions like "<~> '['", where were implementing a 'readWriteThis'. Such expressions will fail to compile (passing const to ref), and so I suspect we were never resolving a path where the type was read.

I've added some compiler warnings to the cases I noticed over in #19983

@bradcray
Copy link
Member

Yeah, I definitely believe we've been very optimistic sometimes about thinking "I'll use <~> and things will just magically work" without actually checking ourselves, and only leaning on write(). And I think I've definitely been guilty of this at times, personally.

@benharsh
Copy link
Member Author

benharsh commented Aug 25, 2023

Checking back in on this, it appears that the core issue is that Map's readThis invokes a helper method that also implements writeThis, with a bit of param-checking to call either read or write in the right cases. The problem with this approach is that it results in the readThis case only trying to read as many key-value pairs as there are in the map, which in the example program above is zero. Therefore, the program attempts to read none of the key-value pairs in the channel and instead tries to read the closing curly brace, which of course is further down the line.

The new serialization effort handles this by clearing the map at the beginning of a read, and using the new API to add as many key-value pairs as there are in the fileReader's stream. Once serializers are enabled, I will be able to close this issue.

@benharsh
Copy link
Member Author

This is resolved by #23207, which enabled serializers by default.

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