From 2cebef095e61608a3d35710cb5fd3d7de18b68ac Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Fri, 28 Oct 2011 21:19:59 -0700 Subject: [PATCH] stdlib: Make io failures recoverable by returning a result --- src/comp/driver/rustc.rs | 16 ++++++--- src/comp/syntax/codemap.rs | 10 ++++-- src/comp/syntax/parse/parser.rs | 13 +++++-- src/compiletest/header.rs | 2 +- src/compiletest/runtest.rs | 8 +++-- src/fuzzer/fuzzer.rs | 10 +++--- src/lib/io.rs | 43 +++++++++++++--------- src/lib/result.rs | 2 ++ src/test/bench/task-perf-word-count.rs | 3 +- src/test/compile-fail/missingmod.rc | 3 ++ src/test/stdtest/io.rs | 50 ++++++++++++++++++++++++-- 11 files changed, 123 insertions(+), 37 deletions(-) create mode 100644 src/test/compile-fail/missingmod.rc diff --git a/src/comp/driver/rustc.rs b/src/comp/driver/rustc.rs index 41b1faec8a4..650e46b3898 100644 --- a/src/comp/driver/rustc.rs +++ b/src/comp/driver/rustc.rs @@ -11,7 +11,7 @@ import syntax::print::{pp, pprust}; import util::{ppaux, common, filesearch}; import back::link; import lib::llvm; -import std::{fs, option, str, vec, int, io, run, getopts}; +import std::{fs, option, str, vec, int, io, run, getopts, result}; import std::map::mk_hashmap; import std::option::{some, none}; import std::getopts::{optopt, optmulti, optflag, optflagopt, opt_present}; @@ -77,10 +77,16 @@ fn parse_input(sess: session::session, cfg: ast::crate_cfg, input: str) -> fn parse_input_src(sess: session::session, cfg: ast::crate_cfg, infile: str) -> {crate: @ast::crate, src: str} { - let srcbytes = - if infile != "-" { - io::file_reader(infile) - } else { io::stdin() }.read_whole_stream(); + let srcbytes = if infile != "-" { + alt io::file_reader(infile) { + result::ok(reader) { reader } + result::err(e) { + sess.fatal(e) + } + } + } else { + io::stdin() + }.read_whole_stream(); let src = str::unsafe_from_bytes(srcbytes); let crate = parser::parse_crate_from_source_str(infile, src, cfg, diff --git a/src/comp/syntax/codemap.rs b/src/comp/syntax/codemap.rs index 5156570c8ba..2db5e1e2c4c 100644 --- a/src/comp/syntax/codemap.rs +++ b/src/comp/syntax/codemap.rs @@ -1,4 +1,4 @@ -import std::{vec, uint, str, term, io, option}; +import std::{vec, uint, str, term, io, option, result}; import std::option::{some, none}; type filename = str; @@ -154,7 +154,13 @@ fn maybe_highlight_lines(sp: option::t, cm: codemap, // FIXME: reading in the entire file is the worst possible way to // get access to the necessary lines. - let file = io::read_whole_file_str(lines.name); + let file = alt io::read_whole_file_str(lines.name) { + result::ok(file) { file } + result::err(e) { + emit_error(none, e, cm); + fail; + } + }; let fm = get_filemap(cm, lines.name); // arbitrarily only print up to six lines of the error diff --git a/src/comp/syntax/parse/parser.rs b/src/comp/syntax/parse/parser.rs index dc1b7fb3047..f605a2fb699 100644 --- a/src/comp/syntax/parse/parser.rs +++ b/src/comp/syntax/parse/parser.rs @@ -1,5 +1,5 @@ -import std::{io, vec, str, option, either}; +import std::{io, vec, str, option, either, result}; import std::option::{some, none}; import std::either::{left, right}; import std::map::{hashmap, new_str_hash}; @@ -53,7 +53,16 @@ type parser = fn new_parser_from_file(sess: parse_sess, cfg: ast::crate_cfg, path: str, chpos: uint, byte_pos: uint, ftype: file_type) -> parser { - let src = io::read_whole_file_str(path); + let src = alt io::read_whole_file_str(path) { + result::ok(src) { + // FIXME: This copy is unfortunate + src + } + result::err(e) { + codemap::emit_error(none, e, sess.cm); + fail; + } + }; let filemap = codemap::new_filemap(path, chpos, byte_pos); sess.cm.files += [filemap]; let itr = @interner::mk(str::hash, str::eq); diff --git a/src/compiletest/header.rs b/src/compiletest/header.rs index 60ff2dc0967..93fbdd3dead 100644 --- a/src/compiletest/header.rs +++ b/src/compiletest/header.rs @@ -63,7 +63,7 @@ fn is_test_ignored(config: config, testfile: str) -> bool { } fn iter_header(testfile: str, it: block(str)) { - let rdr = io::file_reader(testfile); + let rdr = std::result::get(io::file_reader(testfile)); while !rdr.eof() { let ln = rdr.read_line(); diff --git a/src/compiletest/runtest.rs b/src/compiletest/runtest.rs index 2697f8bbc98..198ef5e1101 100644 --- a/src/compiletest/runtest.rs +++ b/src/compiletest/runtest.rs @@ -5,6 +5,7 @@ import std::fs; import std::os; import std::vec; import std::test; +import std::result; import common::mode_run_pass; import common::mode_run_fail; @@ -92,7 +93,7 @@ fn run_pretty_test(cx: cx, props: test_props, testfile: str) { let rounds = alt props.pp_exact { option::some(_) { 1 } option::none. { 2 } }; - let srcs = [io::read_whole_file_str(testfile)]; + let srcs = [result::get(io::read_whole_file_str(testfile))]; let round = 0; while round < rounds { @@ -112,7 +113,7 @@ fn run_pretty_test(cx: cx, props: test_props, testfile: str) { alt props.pp_exact { option::some(file) { let filepath = fs::connect(fs::dirname(testfile), file); - io::read_whole_file_str(filepath) + result::get(io::read_whole_file_str(filepath)) } option::none. { srcs[vec::len(srcs) - 2u] } }; @@ -339,7 +340,8 @@ fn dump_output(config: config, testfile: str, out: str, err: str) { #[cfg(target_os = "linux")] fn dump_output_file(config: config, testfile: str, out: str, extension: str) { let outfile = make_out_name(config, testfile, extension); - let writer = io::file_writer(outfile, [io::create, io::truncate]); + let writer = result::get( + io::file_writer(outfile, [io::create, io::truncate])); writer.write_str(out); } diff --git a/src/fuzzer/fuzzer.rs b/src/fuzzer/fuzzer.rs index ab07d3ede8e..4a96877dfc1 100644 --- a/src/fuzzer/fuzzer.rs +++ b/src/fuzzer/fuzzer.rs @@ -1,7 +1,7 @@ use std; use rustc; -import std::{fs, io, getopts, math, vec, str, int, uint, option}; +import std::{fs, io, getopts, math, vec, str, int, uint, option, result}; import std::getopts::{optopt, opt_present, opt_str}; import std::io::stdout; @@ -13,7 +13,9 @@ tag test_mode { tm_converge; tm_run; } type context = { mode: test_mode }; // + rng fn write_file(filename: str, content: str) { - io::file_writer(filename, [io::create, io::truncate]).write_str(content); + result::get( + io::file_writer(filename, [io::create, io::truncate])) + .write_str(content); // Work around https://github.com/graydon/rust/issues/726 std::run::run_program("chmod", ["644", filename]); } @@ -517,7 +519,7 @@ fn check_convergence(files: [str]) { log_err #fmt["pp convergence tests: %u files", vec::len(files)]; for file in files { if !file_might_not_converge(file) { - let s = io::read_whole_file_str(file); + let s = result::get(io::read_whole_file_str(file)); if !content_might_not_converge(s) { log_err #fmt["pp converge: %s", file]; // Change from 7u to 2u once https://github.com/graydon/rust/issues/850 is fixed @@ -533,7 +535,7 @@ fn check_variants(files: [str], cx: context) { cont; } - let s = io::read_whole_file_str(file); + let s = result::get(io::read_whole_file_str(file)); if contains(s, "#") { cont; // Macros are confusing } diff --git a/src/lib/io.rs b/src/lib/io.rs index ab93699f8e5..dad3001262f 100644 --- a/src/lib/io.rs +++ b/src/lib/io.rs @@ -173,14 +173,16 @@ fn stdin() -> reader { ret new_reader(FILE_buf_reader(rustrt::rust_get_stdin(), option::none)); } -fn file_reader(path: str) -> reader { +fn file_reader(path: str) -> result::t { let f = str::as_buf(path, {|pathbuf| str::as_buf("r", {|modebuf| os::libc::fopen(pathbuf, modebuf) }) }); - if f as uint == 0u { log_err "error opening " + path; fail; } - ret new_reader(FILE_buf_reader(f, option::some(@FILE_res(f)))); + ret if f as uint == 0u { result::err("error opening " + path) } + else { + result::ok(new_reader(FILE_buf_reader(f, option::some(@FILE_res(f))))) + } } @@ -278,7 +280,8 @@ obj fd_buf_writer(fd: int, res: option::t<@fd_res>) { } } -fn file_buf_writer(path: str, flags: [fileflag]) -> buf_writer { +fn file_buf_writer(path: str, + flags: [fileflag]) -> result::t { let fflags: int = os::libc_constants::O_WRONLY() | os::libc_constants::O_BINARY(); for f: fileflag in flags { @@ -296,12 +299,12 @@ fn file_buf_writer(path: str, flags: [fileflag]) -> buf_writer { os::libc_constants::S_IRUSR() | os::libc_constants::S_IWUSR()) }); - if fd < 0 { - log_err "error opening file for writing"; + ret if fd < 0 { log_err sys::last_os_error(); - fail; + result::err("error opening " + path) + } else { + result::ok(fd_buf_writer(fd, option::some(@fd_res(fd)))) } - ret fd_buf_writer(fd, option::some(@fd_res(fd))); } type writer = @@ -359,13 +362,15 @@ obj new_writer(out: buf_writer) { } } -fn file_writer(path: str, flags: [fileflag]) -> writer { - ret new_writer(file_buf_writer(path, flags)); +fn file_writer(path: str, flags: [fileflag]) -> result::t { + result::chain(file_buf_writer(path, flags), { |w| + result::ok(new_writer(w)) + }) } // FIXME: fileflags -fn buffered_file_buf_writer(path: str) -> buf_writer { +fn buffered_file_buf_writer(path: str) -> result::t { let f = str::as_buf(path, {|pathbuf| @@ -374,8 +379,8 @@ fn buffered_file_buf_writer(path: str) -> buf_writer { os::libc::fopen(pathbuf, modebuf) }) }); - if f as uint == 0u { log_err "error opening " + path; fail; } - ret FILE_writer(f, option::some(@FILE_res(f))); + ret if f as uint == 0u { result::err("error opening " + path) } + else { result::ok(FILE_writer(f, option::some(@FILE_res(f)))) } } @@ -452,14 +457,18 @@ fn seek_in_buf(offset: int, pos: uint, len: uint, whence: seek_style) -> ret bpos as uint; } -fn read_whole_file_str(file: str) -> str { - str::unsafe_from_bytes(read_whole_file(file)) +fn read_whole_file_str(file: str) -> result::t { + result::chain(read_whole_file(file), { |bytes| + result::ok(str::unsafe_from_bytes(bytes)) + }) } -fn read_whole_file(file: str) -> [u8] { +fn read_whole_file(file: str) -> result::t<[u8], str> { // FIXME: There's a lot of copying here - file_reader(file).read_whole_stream() + result::chain(file_reader(file), { |rdr| + result::ok(rdr.read_whole_stream()) + }) } diff --git a/src/lib/result.rs b/src/lib/result.rs index 8b07138d8d0..9586a4becfd 100644 --- a/src/lib/result.rs +++ b/src/lib/result.rs @@ -41,6 +41,8 @@ fn get(res: t) -> T { alt res { ok(t) { t } err(_) { + // FIXME: Serialize the error value + // and include it in the fail message fail "get called on error result"; } } diff --git a/src/test/bench/task-perf-word-count.rs b/src/test/bench/task-perf-word-count.rs index 321dfb57caa..2c50327454a 100644 --- a/src/test/bench/task-perf-word-count.rs +++ b/src/test/bench/task-perf-word-count.rs @@ -20,6 +20,7 @@ import std::io; import std::time; import std::u64; +import std::result; import std::task; import std::task::joinable_task; @@ -30,7 +31,7 @@ import std::comm::recv; import std::comm::send; fn map(filename: str, emit: map_reduce::putter) { - let f = io::file_reader(filename); + let f = result::get(io::file_reader(filename)); while true { diff --git a/src/test/compile-fail/missingmod.rc b/src/test/compile-fail/missingmod.rc new file mode 100644 index 00000000000..6e13a970509 --- /dev/null +++ b/src/test/compile-fail/missingmod.rc @@ -0,0 +1,3 @@ +// error-pattern:error opening + +mod doesnotexist; \ No newline at end of file diff --git a/src/test/stdtest/io.rs b/src/test/stdtest/io.rs index 08330c619fc..d84b80e2ed3 100644 --- a/src/test/stdtest/io.rs +++ b/src/test/stdtest/io.rs @@ -2,6 +2,7 @@ use std; import std::io; import std::str; +import std::result; #[cfg(target_os = "linux")] #[cfg(target_os = "win32")] @@ -13,10 +14,10 @@ fn test_simple() { log frood; { let out: io::writer = - io::file_writer(tmpfile, [io::create, io::truncate]); + result::get(io::file_writer(tmpfile, [io::create, io::truncate])); out.write_str(frood); } - let inp: io::reader = io::file_reader(tmpfile); + let inp: io::reader = result::get(io::file_reader(tmpfile)); let frood2: str = inp.read_c_str(); log frood2; assert (str::eq(frood, frood2)); @@ -28,3 +29,48 @@ fn test_simple() { #[ignore] fn test_simple() { } +#[test] +fn file_reader_not_exist() { + alt io::file_reader("not a file") { + result::err(e) { + assert e == "error opening not a file"; + } + result::ok(_) { fail; } + } +} + +#[cfg(target_os = "linux")] +#[cfg(target_os = "win32")] +#[test] +fn file_buf_writer_bad_name() { + alt io::file_buf_writer("/?", []) { + result::err(e) { + assert e == "error opening /?"; + } + result::ok(_) { fail; } + } +} + +// FIXME (726) +#[cfg(target_os = "macos")] +#[test] +#[ignore] +fn file_buf_writer_bad_name() { } + +#[cfg(target_os = "linux")] +#[cfg(target_os = "win32")] +#[test] +fn buffered_file_buf_writer_bad_name() { + alt io::buffered_file_buf_writer("/?") { + result::err(e) { + assert e == "error opening /?"; + } + result::ok(_) { fail; } + } +} + +// FIXME (726) +#[cfg(target_os = "macos")] +#[test] +#[ignore] +fn buffered_file_buf_writer_bad_name() { }