From 3d25238d6d702b4a60b185c08a8f50a4d16e2600 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Sun, 25 Jun 2017 17:37:03 +0200 Subject: [PATCH 1/3] fixed some clippy warnings in compiletest --- src/tools/compiletest/src/errors.rs | 6 +- src/tools/compiletest/src/header.rs | 27 +++---- src/tools/compiletest/src/json.rs | 2 +- src/tools/compiletest/src/main.rs | 24 +++--- src/tools/compiletest/src/procsrv.rs | 3 +- src/tools/compiletest/src/runtest.rs | 110 +++++++++++++-------------- 6 files changed, 79 insertions(+), 93 deletions(-) diff --git a/src/tools/compiletest/src/errors.rs b/src/tools/compiletest/src/errors.rs index 29ca54fda8d..0b9b9599be6 100644 --- a/src/tools/compiletest/src/errors.rs +++ b/src/tools/compiletest/src/errors.rs @@ -35,7 +35,7 @@ impl FromStr for ErrorKind { "ERROR" => Ok(ErrorKind::Error), "NOTE" => Ok(ErrorKind::Note), "SUGGESTION" => Ok(ErrorKind::Suggestion), - "WARN" => Ok(ErrorKind::Warning), + "WARN" | "WARNING" => Ok(ErrorKind::Warning), _ => Err(()), } @@ -95,7 +95,7 @@ pub fn load_errors(testfile: &Path, cfg: Option<&str>) -> Vec { let tag = match cfg { Some(rev) => format!("//[{}]~", rev), - None => format!("//~"), + None => "//~".to_string(), }; rdr.lines() @@ -153,7 +153,7 @@ fn parse_expected(last_nonfollow_error: Option, let msg = msg.trim().to_owned(); let (which, line_num) = if follow { - assert!(adjusts == 0, "use either //~| or //~^, not both."); + assert_eq!(adjusts, 0, "use either //~| or //~^, not both."); let line_num = last_nonfollow_error.expect("encountered //~| without \ preceding //~^ line."); (FollowPrevious(line_num), line_num) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index aa33580b337..ce33787a7d3 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -258,7 +258,7 @@ impl TestProps { check_stdout: false, no_prefer_dynamic: false, pretty_expanded: false, - pretty_mode: format!("normal"), + pretty_mode: "normal".to_string(), pretty_compare_only: false, forbid_output: vec![], incremental_dir: None, @@ -381,14 +381,11 @@ impl TestProps { } }); - for key in vec!["RUST_TEST_NOCAPTURE", "RUST_TEST_THREADS"] { - match env::var(key) { - Ok(val) => { - if self.exec_env.iter().find(|&&(ref x, _)| *x == key).is_none() { - self.exec_env.push((key.to_owned(), val)) - } + for key in &["RUST_TEST_NOCAPTURE", "RUST_TEST_THREADS"] { + if let Ok(val) = env::var(key) { + if self.exec_env.iter().find(|&&(ref x, _)| x == key).is_none() { + self.exec_env.push(((*key).to_owned(), val)) } - Err(..) => {} } } } @@ -409,7 +406,7 @@ fn iter_header(testfile: &Path, cfg: Option<&str>, it: &mut FnMut(&str)) { return; } else if ln.starts_with("//[") { // A comment like `//[foo]` is specific to revision `foo` - if let Some(close_brace) = ln.find("]") { + if let Some(close_brace) = ln.find(']') { let lncfg = &ln[3..close_brace]; let matches = match cfg { Some(s) => s == &lncfg[..], @@ -521,12 +518,10 @@ impl Config { fn parse_pp_exact(&self, line: &str, testfile: &Path) -> Option { if let Some(s) = self.parse_name_value_directive(line, "pp-exact") { Some(PathBuf::from(&s)) + } else if self.parse_name_directive(line, "pp-exact") { + testfile.file_name().map(PathBuf::from) } else { - if self.parse_name_directive(line, "pp-exact") { - testfile.file_name().map(PathBuf::from) - } else { - None - } + None } } @@ -555,8 +550,8 @@ pub fn lldb_version_to_int(version_string: &str) -> isize { let error_string = format!("Encountered LLDB version string with unexpected format: {}", version_string); let error_string = error_string; - let major: isize = version_string.parse().ok().expect(&error_string); - return major; + let major: isize = version_string.parse().expect(&error_string); + major } fn expand_variables(mut value: String, config: &Config) -> String { diff --git a/src/tools/compiletest/src/json.rs b/src/tools/compiletest/src/json.rs index 06cbd9a3df4..23782c3ccc9 100644 --- a/src/tools/compiletest/src/json.rs +++ b/src/tools/compiletest/src/json.rs @@ -65,7 +65,7 @@ pub fn parse_output(file_name: &str, output: &str, proc_res: &ProcRes) -> Vec Vec { // The compiler sometimes intermingles non-JSON stuff into the // output. This hack just skips over such lines. Yuck. - if line.chars().next() == Some('{') { + if line.starts_with('{') { match json::decode::(line) { Ok(diagnostic) => { let mut expected_errors = vec![]; diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index c88ffba357a..692e31ebad0 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -168,7 +168,7 @@ pub fn parse_config(args: Vec ) -> Config { src_base: opt_path(matches, "src-base"), build_base: opt_path(matches, "build-base"), stage_id: matches.opt_str("stage-id").unwrap(), - mode: matches.opt_str("mode").unwrap().parse().ok().expect("invalid mode"), + mode: matches.opt_str("mode").unwrap().parse().expect("invalid mode"), run_ignored: matches.opt_present("ignored"), filter: matches.free.first().cloned(), filter_exact: matches.opt_present("exact"), @@ -208,7 +208,7 @@ pub fn parse_config(args: Vec ) -> Config { pub fn log_config(config: &Config) { let c = config; - logv(c, format!("configuration:")); + logv(c, "configuration:".to_string()); logv(c, format!("compile_lib_path: {:?}", config.compile_lib_path)); logv(c, format!("run_lib_path: {:?}", config.run_lib_path)); logv(c, format!("rustc_path: {:?}", config.rustc_path.display())); @@ -238,10 +238,10 @@ pub fn log_config(config: &Config) { config.adb_device_status)); logv(c, format!("verbose: {}", config.verbose)); logv(c, format!("quiet: {}", config.quiet)); - logv(c, format!("\n")); + logv(c, "\n".to_string()); } -pub fn opt_str<'a>(maybestr: &'a Option) -> &'a str { +pub fn opt_str(maybestr: &Option) -> &str { match *maybestr { None => "(none)", Some(ref s) => s, @@ -465,11 +465,9 @@ pub fn make_test(config: &Config, testpaths: &TestPaths) -> test::TestDescAndFn }; // Debugging emscripten code doesn't make sense today - let mut ignore = early_props.ignore || !up_to_date(config, testpaths, &early_props); - if (config.mode == DebugInfoGdb || config.mode == DebugInfoLldb) && - config.target.contains("emscripten") { - ignore = true; - } + let ignore = early_props.ignore || !up_to_date(config, testpaths, &early_props) || + (config.mode == DebugInfoGdb || config.mode == DebugInfoLldb) && + config.target.contains("emscripten"); test::TestDescAndFn { desc: test::TestDesc { @@ -487,7 +485,7 @@ fn stamp(config: &Config, testpaths: &TestPaths) -> PathBuf { .to_str().unwrap(), config.stage_id); config.build_base.canonicalize() - .unwrap_or(config.build_base.clone()) + .unwrap_or_else(|_| config.build_base.clone()) .join(stamp_name) } @@ -512,7 +510,7 @@ fn up_to_date(config: &Config, testpaths: &TestPaths, props: &EarlyProps) -> boo fn mtime(path: &Path) -> FileTime { fs::metadata(path).map(|f| { FileTime::from_last_modification_time(&f) - }).unwrap_or(FileTime::zero()) + }).unwrap_or_else(|_| FileTime::zero()) } pub fn make_test_name(config: &Config, testpaths: &TestPaths) -> test::TestName { @@ -560,7 +558,7 @@ fn analyze_gdb(gdb: Option) -> (Option, Option, bool) { let gdb_native_rust = version.map_or(false, |v| v >= MIN_GDB_WITH_RUST); - return (Some(gdb.to_owned()), version, gdb_native_rust); + (Some(gdb.to_owned()), version, gdb_native_rust) } fn extract_gdb_version(full_version_line: &str) -> Option { @@ -600,7 +598,7 @@ fn extract_gdb_version(full_version_line: &str) -> Option { Some(idx) => if line.as_bytes()[idx] == b'.' { let patch = &line[idx + 1..]; - let patch_len = patch.find(|c: char| !c.is_digit(10)).unwrap_or(patch.len()); + let patch_len = patch.find(|c: char| !c.is_digit(10)).unwrap_or_else(|| patch.len()); let patch = &patch[..patch_len]; let patch = if patch_len > 3 || patch_len == 0 { None } else { Some(patch) }; diff --git a/src/tools/compiletest/src/procsrv.rs b/src/tools/compiletest/src/procsrv.rs index dbda8f4d802..35f6ed243fe 100644 --- a/src/tools/compiletest/src/procsrv.rs +++ b/src/tools/compiletest/src/procsrv.rs @@ -9,7 +9,6 @@ // except according to those terms. use std::env; -use std::ffi::OsString; use std::io::prelude::*; use std::io; use std::path::PathBuf; @@ -31,7 +30,7 @@ fn add_target_env(cmd: &mut Command, lib_path: &str, aux_path: Option<&str>) { // Need to be sure to put both the lib_path and the aux path in the dylib // search path for the child. let var = dylib_env_var(); - let mut path = env::split_paths(&env::var_os(var).unwrap_or(OsString::new())) + let mut path = env::split_paths(&env::var_os(var).unwrap_or_default()) .collect::>(); if let Some(p) = aux_path { path.insert(0, PathBuf::from(p)) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 0692e07253f..bb8591ef1fe 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -68,7 +68,7 @@ pub fn run(config: Config, testpaths: &TestPaths) { } else { for revision in &base_props.revisions { let mut revision_props = base_props.clone(); - revision_props.load_from(&testpaths.file, Some(&revision), &config); + revision_props.load_from(&testpaths.file, Some(revision), &config); let rev_cx = TestCx { config: &config, props: &revision_props, @@ -81,7 +81,7 @@ pub fn run(config: Config, testpaths: &TestPaths) { base_cx.complete_all(); - File::create(::stamp(&config, &testpaths)).unwrap(); + File::create(::stamp(&config, testpaths)).unwrap(); } struct TestCx<'test> { @@ -101,9 +101,8 @@ impl<'test> TestCx<'test> { /// invoked once before any revisions have been processed fn init_all(&self) { assert!(self.revision.is_none(), "init_all invoked for a revision"); - match self.config.mode { - Incremental => self.init_incremental_test(), - _ => { } + if let Incremental = self.config.mode { + self.init_incremental_test() } } @@ -111,7 +110,7 @@ impl<'test> TestCx<'test> { /// revisions, exactly once, with revision == None). fn run_revision(&self) { match self.config.mode { - CompileFail => self.run_cfail_test(), + CompileFail | ParseFail => self.run_cfail_test(), RunFail => self.run_rfail_test(), RunPass => self.run_rpass_test(), @@ -352,10 +351,10 @@ impl<'test> TestCx<'test> { aux_dir.to_str().unwrap().to_owned()]; args.extend(self.split_maybe_args(&self.config.target_rustcflags)); args.extend(self.props.compile_flags.iter().cloned()); - return ProcArgs { + ProcArgs { prog: self.config.rustc_path.to_str().unwrap().to_owned(), args: args, - }; + } } fn compare_source(&self, @@ -407,17 +406,17 @@ actual:\n\ aux_dir.to_str().unwrap().to_owned()]; if let Some(revision) = self.revision { args.extend(vec![ - format!("--cfg"), - format!("{}", revision), + "--cfg".to_string(), + revision.to_string(), ]); } args.extend(self.split_maybe_args(&self.config.target_rustcflags)); args.extend(self.props.compile_flags.iter().cloned()); // FIXME (#9639): This needs to handle non-utf8 paths - return ProcArgs { + ProcArgs { prog: self.config.rustc_path.to_str().unwrap().to_owned(), args: args, - }; + } } fn run_debuginfo_gdb_test(&self) { @@ -708,7 +707,7 @@ actual:\n\ } } - return None; + None } fn run_debuginfo_lldb_test(&self) { @@ -875,13 +874,13 @@ actual:\n\ for &(ref command_directive, ref check_directive) in &directives { self.config.parse_name_value_directive( &line, - &command_directive).map(|cmd| { + command_directive).map(|cmd| { commands.push(cmd) }); self.config.parse_name_value_directive( &line, - &check_directive).map(|cmd| { + check_directive).map(|cmd| { check_lines.push(cmd) }); } @@ -962,8 +961,7 @@ actual:\n\ (line, 0) }; - for fragment_index in first_fragment .. check_fragments.len() { - let current_fragment = check_fragments[fragment_index]; + for current_fragment in &check_fragments[first_fragment..] { match rest.find(current_fragment) { Some(pos) => { rest = &rest[pos + current_fragment.len() .. ]; @@ -976,7 +974,7 @@ actual:\n\ return false; } - return true; + true } } @@ -1059,7 +1057,7 @@ actual:\n\ let expect_note = expected_errors.iter().any(|ee| ee.kind == Some(ErrorKind::Note)); // Parse the JSON output from the compiler and extract out the messages. - let actual_errors = json::parse_output(&file_name, &proc_res.stderr, &proc_res); + let actual_errors = json::parse_output(&file_name, &proc_res.stderr, proc_res); let mut unexpected = Vec::new(); let mut found = vec![false; expected_errors.len()]; for actual_error in &actual_errors { @@ -1092,7 +1090,7 @@ actual:\n\ .map_or(String::from("message"), |k| k.to_string()), actual_error.msg)); - unexpected.push(actual_error.clone()); + unexpected.push(actual_error); } } } @@ -1110,20 +1108,20 @@ actual:\n\ .map_or("message".into(), |k| k.to_string()), expected_error.msg)); - not_found.push(expected_error.clone()); + not_found.push(expected_error); } } - if unexpected.len() > 0 || not_found.len() > 0 { + if !unexpected.is_empty() || !not_found.is_empty() { self.error( &format!("{} unexpected errors found, {} expected errors not found", unexpected.len(), not_found.len())); - print!("status: {}\ncommand: {}\n", + println!("status: {}\ncommand: {}", proc_res.status, proc_res.cmdline); - if unexpected.len() > 0 { + if !unexpected.is_empty() { println!("unexpected errors (from JSON output): {:#?}\n", unexpected); } - if not_found.len() > 0 { + if !not_found.is_empty() { println!("not found errors (from test file): {:#?}\n", not_found); } panic!(); @@ -1142,9 +1140,9 @@ actual:\n\ match actual_error.kind { Some(ErrorKind::Help) => expect_help, Some(ErrorKind::Note) => expect_note, - Some(ErrorKind::Error) => true, + Some(ErrorKind::Error) | Some(ErrorKind::Warning) => true, - Some(ErrorKind::Suggestion) => false, + Some(ErrorKind::Suggestion) | None => false } } @@ -1287,7 +1285,8 @@ actual:\n\ self.config); let mut crate_type = if aux_props.no_prefer_dynamic { Vec::new() - } else { + } else if (self.config.target.contains("musl") && !aux_props.force_host) || + self.config.target.contains("emscripten") { // We primarily compile all auxiliary libraries as dynamic libraries // to avoid code size bloat and large binaries as much as possible // for the test suite (otherwise including libstd statically in all @@ -1297,13 +1296,9 @@ actual:\n\ // dynamic libraries so we just go back to building a normal library. Note, // however, that for MUSL if the library is built with `force_host` then // it's ok to be a dylib as the host should always support dylibs. - if (self.config.target.contains("musl") && !aux_props.force_host) || - self.config.target.contains("emscripten") - { - vec!["--crate-type=lib".to_owned()] - } else { - vec!["--crate-type=dylib".to_owned()] - } + vec!["--crate-type=lib".to_owned()] + } else { + vec!["--crate-type=dylib".to_owned()] }; crate_type.extend(extra_link_args.clone()); let aux_output = { @@ -1344,7 +1339,7 @@ actual:\n\ lib_path: &str, aux_path: Option<&str>, input: Option) -> ProcRes { - return self.program_output(lib_path, prog, aux_path, args, procenv, input); + self.program_output(lib_path, prog, aux_path, args, procenv, input) } fn make_compile_args(&self, @@ -1367,7 +1362,7 @@ actual:\n\ // Optionally prevent default --target if specified in test compile-flags. let custom_target = self.props.compile_flags .iter() - .fold(false, |acc, ref x| acc || x.starts_with("--target")); + .fold(false, |acc, x| acc || x.starts_with("--target")); if !custom_target { args.extend(vec![ @@ -1377,14 +1372,14 @@ actual:\n\ if let Some(revision) = self.revision { args.extend(vec![ - format!("--cfg"), - format!("{}", revision), + "--cfg".to_string(), + revision.to_string(), ]); } if let Some(ref incremental_dir) = self.props.incremental_dir { args.extend(vec![ - format!("-Z"), + "-Z".to_string(), format!("incremental={}", incremental_dir.display()), ]); } @@ -1457,10 +1452,10 @@ actual:\n\ args.extend(self.split_maybe_args(&self.config.target_rustcflags)); } args.extend(self.props.compile_flags.iter().cloned()); - return ProcArgs { + ProcArgs { prog: self.config.rustc_path.to_str().unwrap().to_owned(), args: args, - }; + } } fn make_lib_name(&self, auxfile: &Path) -> PathBuf { @@ -1508,10 +1503,10 @@ actual:\n\ args.extend(self.split_maybe_args(&self.props.run_flags)); let prog = args.remove(0); - return ProcArgs { + ProcArgs { prog: prog, args: args, - }; + } } fn split_maybe_args(&self, argstr: &Option) -> Vec { @@ -1558,12 +1553,12 @@ actual:\n\ env, input).expect(&format!("failed to exec `{}`", prog)); self.dump_output(&out, &err); - return ProcRes { + ProcRes { status: status, stdout: out, stderr: err, cmdline: cmdline, - }; + } } fn make_cmdline(&self, libpath: &str, prog: &str, args: &[String]) -> String { @@ -1764,7 +1759,7 @@ actual:\n\ self.fatal_proc_rec("rustdoc failed!", &proc_res); } - if self.props.check_test_line_numbers_match == true { + if self.props.check_test_line_numbers_match { self.check_rustdoc_test_option(proc_res); } else { let root = self.find_rust_src_root().unwrap(); @@ -1791,7 +1786,7 @@ actual:\n\ .filter_map(|(line_nb, line)| { if (line.trim_left().starts_with("pub mod ") || line.trim_left().starts_with("mod ")) && - line.ends_with(";") { + line.ends_with(';') { if let Some(ref mut other_files) = other_files { other_files.push(line.rsplit("mod ") .next() @@ -1840,7 +1835,7 @@ actual:\n\ } let mut tested = 0; - for _ in res.stdout.split("\n") + for _ in res.stdout.split('\n') .filter(|s| s.starts_with("test ")) .inspect(|s| { let tmp: Vec<&str> = s.split(" - ").collect(); @@ -1853,7 +1848,7 @@ actual:\n\ iter.next(); let line = iter.next() .unwrap_or(")") - .split(")") + .split(')') .next() .unwrap_or("0") .parse() @@ -1873,7 +1868,7 @@ actual:\n\ self.fatal_proc_rec(&format!("No test has been found... {:?}", files), &res); } else { for (entry, v) in &files { - if v.len() != 0 { + if !v.is_empty() { self.fatal_proc_rec(&format!("Not found test at line{} \"{}\":{:?}", if v.len() > 1 { "s" } else { "" }, entry, v), &res); @@ -1916,11 +1911,10 @@ actual:\n\ .find(|ti| ti.name == expected_item.name); if let Some(actual_item) = actual_item_with_same_name { - if !expected_item.codegen_units.is_empty() { - // Also check for codegen units - if expected_item.codegen_units != actual_item.codegen_units { - wrong_cgus.push((expected_item.clone(), actual_item.clone())); - } + if !expected_item.codegen_units.is_empty() && + // Also check for codegen units + expected_item.codegen_units != actual_item.codegen_units { + wrong_cgus.push((expected_item.clone(), actual_item.clone())); } } else { missing.push(expected_item.string.clone()); @@ -2005,7 +1999,7 @@ actual:\n\ let cgus = if parts.len() > 1 { let cgus_str = parts[1]; - cgus_str.split(" ") + cgus_str.split(' ') .map(str::trim) .filter(|s| !s.is_empty()) .map(str::to_owned) @@ -2323,7 +2317,7 @@ actual:\n\ } } - fn compare_mir_test_output(&self, test_name: &str, expected_content: &Vec<&str>) { + fn compare_mir_test_output(&self, test_name: &str, expected_content: &[&str]) { let mut output_file = PathBuf::new(); output_file.push(self.get_mir_dump_dir()); output_file.push(test_name); From 1715559f82a7576c8d64430dbff05bc007f1a6e9 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Tue, 27 Jun 2017 07:16:54 +0200 Subject: [PATCH 2/3] address tidy error & comment --- src/tools/compiletest/src/header.rs | 4 +--- src/tools/compiletest/src/main.rs | 3 ++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index ce33787a7d3..c9e24492207 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -549,9 +549,7 @@ impl Config { pub fn lldb_version_to_int(version_string: &str) -> isize { let error_string = format!("Encountered LLDB version string with unexpected format: {}", version_string); - let error_string = error_string; - let major: isize = version_string.parse().expect(&error_string); - major + version_string.parse().expect(&error_string); } fn expand_variables(mut value: String, config: &Config) -> String { diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 692e31ebad0..defb405c09d 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -598,7 +598,8 @@ fn extract_gdb_version(full_version_line: &str) -> Option { Some(idx) => if line.as_bytes()[idx] == b'.' { let patch = &line[idx + 1..]; - let patch_len = patch.find(|c: char| !c.is_digit(10)).unwrap_or_else(|| patch.len()); + let patch_len = patch.find(|c: char| !c.is_digit(10)) + .unwrap_or_else(|| patch.len()); let patch = &patch[..patch_len]; let patch = if patch_len > 3 || patch_len == 0 { None } else { Some(patch) }; From 7be171db70685eb4f6465f2d9047f8bfe1651708 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Thu, 29 Jun 2017 23:38:13 +0200 Subject: [PATCH 3/3] fix a stray semicolon --- src/tools/compiletest/src/header.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index c9e24492207..5ac60d8f2c8 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -549,7 +549,7 @@ impl Config { pub fn lldb_version_to_int(version_string: &str) -> isize { let error_string = format!("Encountered LLDB version string with unexpected format: {}", version_string); - version_string.parse().expect(&error_string); + version_string.parse().expect(&error_string) } fn expand_variables(mut value: String, config: &Config) -> String {