Skip to content

fixed some clippy warnings in compiletest #42896

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 1, 2017
Merged

fixed some clippy warnings in compiletest #42896

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3d25238
fixed some clippy warnings in compiletest
llogiq Jun 25, 2017
1715559
address tidy error & comment
llogiq Jun 27, 2017
7be171d
fix a stray semicolon
llogiq Jun 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/tools/compiletest/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(()),
}
Expand Down Expand Up @@ -95,7 +95,7 @@ pub fn load_errors(testfile: &Path, cfg: Option<&str>) -> Vec<Error> {

let tag = match cfg {
Some(rev) => format!("//[{}]~", rev),
None => format!("//~"),
None => "//~".to_string(),
};

rdr.lines()
Expand Down Expand Up @@ -153,7 +153,7 @@ fn parse_expected(last_nonfollow_error: Option<usize>,
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)
Expand Down
27 changes: 11 additions & 16 deletions src/tools/compiletest/src/header.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(..) => {}
}
}
}
Expand All @@ -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[..],
Expand Down Expand Up @@ -521,12 +518,10 @@ impl Config {
fn parse_pp_exact(&self, line: &str, testfile: &Path) -> Option<PathBuf> {
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
}
}

Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this can be just one line, there's no need for the variable here. I guess it does indicate what we're getting from the version, but AIUI that's not actually important -- the isize is mostly opaque.

As a side note, which is out of scope for this PR, why is this an isize? Can the version number be negative? u32 or u64 would seem to make more sense...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I did not look at that code too closely. We had clippy ignore bindings with given type because we were getting some false positives around it. However, I agree, this should be one line. I'm not going to change APIs here...

}

fn expand_variables(mut value: String, config: &Config) -> String {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/json.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub fn parse_output(file_name: &str, output: &str, proc_res: &ProcRes) -> Vec<Er
fn parse_line(file_name: &str, line: &str, output: &str, proc_res: &ProcRes) -> Vec<Error> {
// 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::<Diagnostic>(line) {
Ok(diagnostic) => {
let mut expected_errors = vec![];
Expand Down
24 changes: 11 additions & 13 deletions src/tools/compiletest/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ pub fn parse_config(args: Vec<String> ) -> 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"),
Expand Down Expand Up @@ -208,7 +208,7 @@ pub fn parse_config(args: Vec<String> ) -> 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()));
Expand Down Expand Up @@ -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<String>) -> &'a str {
pub fn opt_str(maybestr: &Option<String>) -> &str {
match *maybestr {
None => "(none)",
Some(ref s) => s,
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}

Expand All @@ -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 {
Expand Down Expand Up @@ -560,7 +558,7 @@ fn analyze_gdb(gdb: Option<String>) -> (Option<String>, Option<u32>, 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<u32> {
Expand Down Expand Up @@ -600,7 +598,7 @@ fn extract_gdb_version(full_version_line: &str) -> Option<u32> {
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) };

Expand Down
3 changes: 1 addition & 2 deletions src/tools/compiletest/src/procsrv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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::<Vec<_>>();
if let Some(p) = aux_path {
path.insert(0, PathBuf::from(p))
Expand Down
Loading