Skip to content

Commit

Permalink
auto merge of rust-lang#8288 : Kimundi/rust/opteitres4, r=brson
Browse files Browse the repository at this point in the history
This is an alternative version to rust-lang#8268, where instead of transitioning to `get()` completely, I transitioned to `unwrap()` completely.

My reasoning for also opening this PR is that having two different functions with identical behavior on a common datatype is bad for consistency and confusing for users, and should be solved as soon as possible. The fact that apparently half the code uses `get()`, and the other half `unwrap()` only makes it worse.

If the final naming decision ends up different, there needs to be a big renaming anyway, but until then it should at least be consistent.

---

- Made naming schemes consistent between Option, Result and Either
- Lifted the quality of the either and result module to that of option
- Changed Options Add implementation to work like the maybe Monad (return None if any of the inputs is None)  
  See rust-lang#6002, especially my last comment.
- Removed duplicate Option::get and renamed all related functions to use the term `unwrap` instead  
  See also rust-lang#7887.

Todo: 

Adding testcases for all function in the three modules. Even without the few functions I added, the coverage wasn't complete to begin with. But I'd rather do that as a follow up PR, I've touched to much code here already, need to go through them again later.
  • Loading branch information
bors committed Aug 5, 2013
2 parents 29099e4 + 0ac7a21 commit bbda3fa
Show file tree
Hide file tree
Showing 115 changed files with 792 additions and 817 deletions.
6 changes: 3 additions & 3 deletions src/compiletest/compiletest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ pub fn parse_config(args: ~[~str]) -> config {
ratchet_noise_percent:
getopts::opt_maybe_str(matches,
"ratchet-noise-percent").map(|s|
f64::from_str(*s).get()),
f64::from_str(*s).unwrap()),
runtool: getopts::opt_maybe_str(matches, "runtool"),
rustcflags: getopts::opt_maybe_str(matches, "rustcflags"),
jit: getopts::opt_present(matches, "jit"),
Expand Down Expand Up @@ -267,7 +267,7 @@ pub fn is_test(config: &config, testfile: &Path) -> bool {
_ => ~[~".rc", ~".rs"]
};
let invalid_prefixes = ~[~".", ~"#", ~"~"];
let name = testfile.filename().get();
let name = testfile.filename().unwrap();

let mut valid = false;

Expand Down Expand Up @@ -300,7 +300,7 @@ pub fn make_test_name(config: &config, testfile: &Path) -> test::TestName {
fn shorten(path: &Path) -> ~str {
let filename = path.filename();
let dir = path.pop().filename();
fmt!("%s/%s", dir.get_or_default(~""), filename.get_or_default(~""))
fmt!("%s/%s", dir.unwrap_or_default(~""), filename.unwrap_or_default(~""))
}

test::DynTestName(fmt!("[%s] %s",
Expand Down
12 changes: 6 additions & 6 deletions src/compiletest/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ fn run_pretty_test(config: &config, props: &TestProps, testfile: &Path) {
let rounds =
match props.pp_exact { Some(_) => 1, None => 2 };

let mut srcs = ~[io::read_whole_file_str(testfile).get()];
let mut srcs = ~[io::read_whole_file_str(testfile).unwrap()];

let mut round = 0;
while round < rounds {
Expand All @@ -166,7 +166,7 @@ fn run_pretty_test(config: &config, props: &TestProps, testfile: &Path) {
match props.pp_exact {
Some(ref file) => {
let filepath = testfile.dir_path().push_rel(file);
io::read_whole_file_str(&filepath).get()
io::read_whole_file_str(&filepath).unwrap()
}
None => { srcs[srcs.len() - 2u].clone() }
};
Expand Down Expand Up @@ -448,7 +448,7 @@ fn scan_until_char(haystack: &str, needle: char, idx: &mut uint) -> bool {
if opt.is_none() {
return false;
}
*idx = opt.get();
*idx = opt.unwrap();
return true;
}

Expand Down Expand Up @@ -709,7 +709,7 @@ fn aux_output_dir_name(config: &config, testfile: &Path) -> Path {
}

fn output_testname(testfile: &Path) -> Path {
Path(testfile.filestem().get())
Path(testfile.filestem().unwrap())
}

fn output_base_name(config: &config, testfile: &Path) -> Path {
Expand Down Expand Up @@ -878,7 +878,7 @@ fn append_suffix_to_stem(p: &Path, suffix: &str) -> Path {
if suffix.len() == 0 {
(*p).clone()
} else {
let stem = p.filestem().get();
let stem = p.filestem().unwrap();
p.with_filestem(stem + "-" + suffix)
}
}
Expand Down Expand Up @@ -938,7 +938,7 @@ fn disassemble_extract(config: &config, _props: &TestProps,


fn count_extracted_lines(p: &Path) -> uint {
let x = io::read_whole_file_str(&p.with_filetype("ll")).get();
let x = io::read_whole_file_str(&p.with_filetype("ll")).unwrap();
x.line_iter().len_()
}

Expand Down
20 changes: 10 additions & 10 deletions src/libextra/base64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,24 +322,24 @@ mod test {

#[test]
fn test_from_base64_basic() {
assert_eq!("".from_base64().get(), "".as_bytes().to_owned());
assert_eq!("Zg==".from_base64().get(), "f".as_bytes().to_owned());
assert_eq!("Zm8=".from_base64().get(), "fo".as_bytes().to_owned());
assert_eq!("Zm9v".from_base64().get(), "foo".as_bytes().to_owned());
assert_eq!("Zm9vYg==".from_base64().get(), "foob".as_bytes().to_owned());
assert_eq!("Zm9vYmE=".from_base64().get(), "fooba".as_bytes().to_owned());
assert_eq!("Zm9vYmFy".from_base64().get(), "foobar".as_bytes().to_owned());
assert_eq!("".from_base64().unwrap(), "".as_bytes().to_owned());
assert_eq!("Zg==".from_base64().unwrap(), "f".as_bytes().to_owned());
assert_eq!("Zm8=".from_base64().unwrap(), "fo".as_bytes().to_owned());
assert_eq!("Zm9v".from_base64().unwrap(), "foo".as_bytes().to_owned());
assert_eq!("Zm9vYg==".from_base64().unwrap(), "foob".as_bytes().to_owned());
assert_eq!("Zm9vYmE=".from_base64().unwrap(), "fooba".as_bytes().to_owned());
assert_eq!("Zm9vYmFy".from_base64().unwrap(), "foobar".as_bytes().to_owned());
}

#[test]
fn test_from_base64_newlines() {
assert_eq!("Zm9v\r\nYmFy".from_base64().get(),
assert_eq!("Zm9v\r\nYmFy".from_base64().unwrap(),
"foobar".as_bytes().to_owned());
}

#[test]
fn test_from_base64_urlsafe() {
assert_eq!("-_8".from_base64().get(), "+/8=".from_base64().get());
assert_eq!("-_8".from_base64().unwrap(), "+/8=".from_base64().unwrap());
}

#[test]
Expand All @@ -364,7 +364,7 @@ mod test {
push(random());
}
};
assert_eq!(v.to_base64(STANDARD).from_base64().get(), v);
assert_eq!(v.to_base64(STANDARD).from_base64().unwrap(), v);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/libextra/fileinput.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,8 @@ mod test {

do input_vec_state(filenames) |line, state| {
let nums: ~[&str] = line.split_iter(' ').collect();
let file_num = uint::from_str(nums[0]).get();
let line_num = uint::from_str(nums[1]).get();
let file_num = uint::from_str(nums[0]).unwrap();
let line_num = uint::from_str(nums[1]).unwrap();
assert_eq!(line_num, state.line_num_file);
assert_eq!(file_num * 3 + line_num, state.line_num);
true
Expand Down
8 changes: 4 additions & 4 deletions src/libextra/getopts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ fn find_opt(opts: &[Opt], nm: Name) -> Option<uint> {
* The type returned when the command line does not conform to the
* expected format. Pass this value to <fail_str> to get an error message.
*/
#[deriving(Clone, Eq)]
#[deriving(Clone, Eq, ToStr)]
pub enum Fail_ {
ArgumentMissing(~str),
UnrecognizedOption(~str),
Expand Down Expand Up @@ -288,7 +288,7 @@ pub fn getopts(args: &[~str], opts: &[Opt]) -> Result {
None => {
let arg_follows =
last_valid_opt_id.is_some() &&
match opts[last_valid_opt_id.get()]
match opts[last_valid_opt_id.unwrap()]
.hasarg {
Yes | Maybe => true,
Expand Down Expand Up @@ -322,15 +322,15 @@ pub fn getopts(args: &[~str], opts: &[Opt]) -> Result {
}
Maybe => {
if !i_arg.is_none() {
vals[optid].push(Val((i_arg.clone()).get()));
vals[optid].push(Val((i_arg.clone()).unwrap()));
} else if name_pos < names.len() ||
i + 1 == l || is_arg(args[i + 1]) {
vals[optid].push(Given);
} else { i += 1; vals[optid].push(Val(args[i].clone())); }
}
Yes => {
if !i_arg.is_none() {
vals[optid].push(Val(i_arg.clone().get()));
vals[optid].push(Val(i_arg.clone().unwrap()));
} else if i + 1 == l {
return Err(ArgumentMissing(name_str(nm)));
} else { i += 1; vals[optid].push(Val(args[i].clone())); }
Expand Down
12 changes: 6 additions & 6 deletions src/libextra/num/bigint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1510,11 +1510,11 @@ mod biguint_tests {

#[test]
fn test_is_even() {
assert!(FromStr::from_str::<BigUint>("1").get().is_odd());
assert!(FromStr::from_str::<BigUint>("2").get().is_even());
assert!(FromStr::from_str::<BigUint>("1000").get().is_even());
assert!(FromStr::from_str::<BigUint>("1000000000000000000000").get().is_even());
assert!(FromStr::from_str::<BigUint>("1000000000000000000001").get().is_odd());
assert!(FromStr::from_str::<BigUint>("1").unwrap().is_odd());
assert!(FromStr::from_str::<BigUint>("2").unwrap().is_even());
assert!(FromStr::from_str::<BigUint>("1000").unwrap().is_even());
assert!(FromStr::from_str::<BigUint>("1000000000000000000000").unwrap().is_even());
assert!(FromStr::from_str::<BigUint>("1000000000000000000001").unwrap().is_odd());
assert!((BigUint::from_uint(1) << 64).is_even());
assert!(((BigUint::from_uint(1) << 64) + BigUint::from_uint(1)).is_odd());
}
Expand Down Expand Up @@ -1595,7 +1595,7 @@ mod biguint_tests {
let &(ref n, ref rs) = num_pair;
for str_pair in rs.iter() {
let &(ref radix, ref str) = str_pair;
assert_eq!(n, &FromStrRadix::from_str_radix(*str, *radix).get());
assert_eq!(n, &FromStrRadix::from_str_radix(*str, *radix).unwrap());
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/libextra/ringbuf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,12 +437,12 @@ mod tests {
assert_eq!(deq.len(), 3);
deq.push_back(d.clone());
assert_eq!(deq.len(), 4);
assert_eq!((*deq.front().get()).clone(), b.clone());
assert_eq!((*deq.back().get()).clone(), d.clone());
assert_eq!(deq.pop_front().get(), b.clone());
assert_eq!(deq.pop_back().get(), d.clone());
assert_eq!(deq.pop_back().get(), c.clone());
assert_eq!(deq.pop_back().get(), a.clone());
assert_eq!((*deq.front().unwrap()).clone(), b.clone());
assert_eq!((*deq.back().unwrap()).clone(), d.clone());
assert_eq!(deq.pop_front().unwrap(), b.clone());
assert_eq!(deq.pop_back().unwrap(), d.clone());
assert_eq!(deq.pop_back().unwrap(), c.clone());
assert_eq!(deq.pop_back().unwrap(), a.clone());
assert_eq!(deq.len(), 0);
deq.push_back(c.clone());
assert_eq!(deq.len(), 1);
Expand Down
4 changes: 2 additions & 2 deletions src/libextra/semver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,8 +386,8 @@ fn test_spec_order() {
"1.0.0"];
let mut i = 1;
while i < vs.len() {
let a = parse(vs[i-1]).get();
let b = parse(vs[i]).get();
let a = parse(vs[i-1]).unwrap();
let b = parse(vs[i]).unwrap();
assert!(a < b);
i += 1;
}
Expand Down
6 changes: 3 additions & 3 deletions src/libextra/smallintmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,9 +366,9 @@ mod test_map {
map.update_with_key(3, 2, addMoreToCount);

// check the total counts
assert_eq!(map.find(&3).get(), &10);
assert_eq!(map.find(&5).get(), &3);
assert_eq!(map.find(&9).get(), &1);
assert_eq!(map.find(&3).unwrap(), &10);
assert_eq!(map.find(&5).unwrap(), &3);
assert_eq!(map.find(&9).unwrap(), &1);

// sadly, no sevens were counted
assert!(map.find(&7).is_none());
Expand Down
57 changes: 28 additions & 29 deletions src/libextra/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,8 @@ pub fn parse_opts(args: &[~str]) -> OptRes {
let ratchet_metrics = getopts::opt_maybe_str(&matches, "ratchet-metrics");
let ratchet_metrics = ratchet_metrics.map(|s| Path(*s));

let ratchet_noise_percent =
getopts::opt_maybe_str(&matches, "ratchet-noise-percent");
let ratchet_noise_percent = ratchet_noise_percent.map(|s| f64::from_str(*s).get());
let ratchet_noise_percent = getopts::opt_maybe_str(&matches, "ratchet-noise-percent");
let ratchet_noise_percent = ratchet_noise_percent.map(|s| f64::from_str(*s).unwrap());

let save_metrics = getopts::opt_maybe_str(&matches, "save-metrics");
let save_metrics = save_metrics.map(|s| Path(*s));
Expand Down Expand Up @@ -631,8 +630,8 @@ fn should_sort_failures_before_printing_them() {
st.write_failures();
};

let apos = s.find_str("a").get();
let bpos = s.find_str("b").get();
let apos = s.find_str("a").unwrap();
let bpos = s.find_str("b").unwrap();
assert!(apos < bpos);
}

Expand Down Expand Up @@ -868,7 +867,7 @@ impl MetricMap {
pub fn load(p: &Path) -> MetricMap {
assert!(os::path_exists(p));
let f = io::file_reader(p).unwrap();
let mut decoder = json::Decoder(json::from_reader(f).get());
let mut decoder = json::Decoder(json::from_reader(f).unwrap());
MetricMap(Decodable::decode(&mut decoder))
}

Expand Down Expand Up @@ -1207,7 +1206,7 @@ mod tests {
either::Left(o) => o,
_ => fail!("Malformed arg in first_free_arg_should_be_a_filter")
};
assert!("filter" == opts.filter.clone().get());
assert!("filter" == opts.filter.clone().unwrap());
}

#[test]
Expand Down Expand Up @@ -1346,28 +1345,28 @@ mod tests {

let diff1 = m2.compare_to_old(&m1, None);

assert_eq!(*(diff1.find(&~"in-both-noise").get()), LikelyNoise);
assert_eq!(*(diff1.find(&~"in-first-noise").get()), MetricRemoved);
assert_eq!(*(diff1.find(&~"in-second-noise").get()), MetricAdded);
assert_eq!(*(diff1.find(&~"in-both-want-downwards-but-regressed").get()),
assert_eq!(*(diff1.find(&~"in-both-noise").unwrap()), LikelyNoise);
assert_eq!(*(diff1.find(&~"in-first-noise").unwrap()), MetricRemoved);
assert_eq!(*(diff1.find(&~"in-second-noise").unwrap()), MetricAdded);
assert_eq!(*(diff1.find(&~"in-both-want-downwards-but-regressed").unwrap()),
Regression(100.0));
assert_eq!(*(diff1.find(&~"in-both-want-downwards-and-improved").get()),
assert_eq!(*(diff1.find(&~"in-both-want-downwards-and-improved").unwrap()),
Improvement(50.0));
assert_eq!(*(diff1.find(&~"in-both-want-upwards-but-regressed").get()),
assert_eq!(*(diff1.find(&~"in-both-want-upwards-but-regressed").unwrap()),
Regression(50.0));
assert_eq!(*(diff1.find(&~"in-both-want-upwards-and-improved").get()),
assert_eq!(*(diff1.find(&~"in-both-want-upwards-and-improved").unwrap()),
Improvement(100.0));
assert_eq!(diff1.len(), 7);

let diff2 = m2.compare_to_old(&m1, Some(200.0));

assert_eq!(*(diff2.find(&~"in-both-noise").get()), LikelyNoise);
assert_eq!(*(diff2.find(&~"in-first-noise").get()), MetricRemoved);
assert_eq!(*(diff2.find(&~"in-second-noise").get()), MetricAdded);
assert_eq!(*(diff2.find(&~"in-both-want-downwards-but-regressed").get()), LikelyNoise);
assert_eq!(*(diff2.find(&~"in-both-want-downwards-and-improved").get()), LikelyNoise);
assert_eq!(*(diff2.find(&~"in-both-want-upwards-but-regressed").get()), LikelyNoise);
assert_eq!(*(diff2.find(&~"in-both-want-upwards-and-improved").get()), LikelyNoise);
assert_eq!(*(diff2.find(&~"in-both-noise").unwrap()), LikelyNoise);
assert_eq!(*(diff2.find(&~"in-first-noise").unwrap()), MetricRemoved);
assert_eq!(*(diff2.find(&~"in-second-noise").unwrap()), MetricAdded);
assert_eq!(*(diff2.find(&~"in-both-want-downwards-but-regressed").unwrap()), LikelyNoise);
assert_eq!(*(diff2.find(&~"in-both-want-downwards-and-improved").unwrap()), LikelyNoise);
assert_eq!(*(diff2.find(&~"in-both-want-upwards-but-regressed").unwrap()), LikelyNoise);
assert_eq!(*(diff2.find(&~"in-both-want-upwards-and-improved").unwrap()), LikelyNoise);
assert_eq!(diff2.len(), 7);
}

Expand All @@ -1391,28 +1390,28 @@ mod tests {
let (diff1, ok1) = m2.ratchet(&pth, None);
assert_eq!(ok1, false);
assert_eq!(diff1.len(), 2);
assert_eq!(*(diff1.find(&~"runtime").get()), Regression(10.0));
assert_eq!(*(diff1.find(&~"throughput").get()), LikelyNoise);
assert_eq!(*(diff1.find(&~"runtime").unwrap()), Regression(10.0));
assert_eq!(*(diff1.find(&~"throughput").unwrap()), LikelyNoise);

// Check that it was not rewritten.
let m3 = MetricMap::load(&pth);
assert_eq!(m3.len(), 2);
assert_eq!(*(m3.find(&~"runtime").get()), Metric { value: 1000.0, noise: 2.0 });
assert_eq!(*(m3.find(&~"throughput").get()), Metric { value: 50.0, noise: 2.0 });
assert_eq!(*(m3.find(&~"runtime").unwrap()), Metric { value: 1000.0, noise: 2.0 });
assert_eq!(*(m3.find(&~"throughput").unwrap()), Metric { value: 50.0, noise: 2.0 });

// Ask for a ratchet with an explicit noise-percentage override,
// that should advance.
let (diff2, ok2) = m2.ratchet(&pth, Some(10.0));
assert_eq!(ok2, true);
assert_eq!(diff2.len(), 2);
assert_eq!(*(diff2.find(&~"runtime").get()), LikelyNoise);
assert_eq!(*(diff2.find(&~"throughput").get()), LikelyNoise);
assert_eq!(*(diff2.find(&~"runtime").unwrap()), LikelyNoise);
assert_eq!(*(diff2.find(&~"throughput").unwrap()), LikelyNoise);

// Check that it was rewritten.
let m4 = MetricMap::load(&pth);
assert_eq!(m4.len(), 2);
assert_eq!(*(m4.find(&~"runtime").get()), Metric { value: 1100.0, noise: 2.0 });
assert_eq!(*(m4.find(&~"throughput").get()), Metric { value: 50.0, noise: 2.0 });
assert_eq!(*(m4.find(&~"runtime").unwrap()), Metric { value: 1100.0, noise: 2.0 });
assert_eq!(*(m4.find(&~"throughput").unwrap()), Metric { value: 50.0, noise: 2.0 });

os::remove_dir_recursive(&dpth);
}
Expand Down
4 changes: 2 additions & 2 deletions src/libextra/url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ fn decode_inner(s: &str, full_url: bool) -> ~str {
match rdr.read_char() {
'%' => {
let bytes = rdr.read_bytes(2u);
let ch = uint::parse_bytes(bytes, 16u).get() as char;
let ch = uint::parse_bytes(bytes, 16u).unwrap() as char;

if full_url {
// Only decode some characters:
Expand Down Expand Up @@ -257,7 +257,7 @@ pub fn decode_form_urlencoded(s: &[u8]) -> HashMap<~str, ~[~str]> {
let ch = match ch {
'%' => {
let bytes = rdr.read_bytes(2u);
uint::parse_bytes(bytes, 16u).get() as char
uint::parse_bytes(bytes, 16u).unwrap() as char
}
'+' => ' ',
ch => ch
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ pub fn link_args(sess: Session,
}
let dir = cratepath.dirname();
if dir != ~"" { args.push(~"-L" + dir); }
let libarg = unlib(sess.targ_cfg, cratepath.filestem().get());
let libarg = unlib(sess.targ_cfg, cratepath.filestem().unwrap());
args.push(~"-l" + libarg);
}

Expand Down Expand Up @@ -950,7 +950,7 @@ pub fn link_args(sess: Session,
// be rpathed
if sess.targ_cfg.os == session::os_macos {
args.push(~"-Wl,-install_name,@rpath/"
+ output.filename().get());
+ output.filename().unwrap());
}
}
Expand Down
Loading

0 comments on commit bbda3fa

Please sign in to comment.