Skip to content

Commit

Permalink
Address CDB review feedback
Browse files Browse the repository at this point in the history
- Don't add path_buf_capacity feature.
- Convert `find_cdb` to early return style, reducing indentation
- Simplify `compute_stamp_hash` for CDB to just hash it's path, if any.
  • Loading branch information
MaulingMonkey committed May 20, 2019
1 parent 0a423a7 commit 56b18ce
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 32 deletions.
52 changes: 25 additions & 27 deletions src/tools/compiletest/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#![crate_name = "compiletest"]
#![feature(test)]
#![feature(path_buf_capacity)]
#![feature(vec_remove_item)]
#![deny(warnings, rust_2018_idioms)]

Expand Down Expand Up @@ -857,35 +856,34 @@ fn is_pc_windows_msvc_target(target: &String) -> bool {
}

fn find_cdb(target: &String) -> Option<OsString> {
if cfg!(windows) && is_pc_windows_msvc_target(target) {
let pf86 = env::var_os("ProgramFiles(x86)").or(env::var_os("ProgramFiles"))?;
let cdb_arch = if cfg!(target_arch="x86") {
"x86"
} else if cfg!(target_arch="x86_64") {
"x64"
} else if cfg!(target_arch="aarch64") {
"arm64"
} else if cfg!(target_arch="arm") {
"arm"
} else {
return None; // No compatible CDB.exe in the Windows 10 SDK
};
if !(cfg!(windows) && is_pc_windows_msvc_target(target)) {
return None;
}

let pf86 = env::var_os("ProgramFiles(x86)").or(env::var_os("ProgramFiles"))?;
let cdb_arch = if cfg!(target_arch="x86") {
"x86"
} else if cfg!(target_arch="x86_64") {
"x64"
} else if cfg!(target_arch="aarch64") {
"arm64"
} else if cfg!(target_arch="arm") {
"arm"
} else {
return None; // No compatible CDB.exe in the Windows 10 SDK
};

let mut path = PathBuf::with_capacity(64);
path.push(pf86);
path.push(r"Windows Kits\10\Debuggers"); // We could check 8.1 etc. too?
path.push(cdb_arch);
path.push(r"cdb.exe");
let mut path = PathBuf::new();
path.push(pf86);
path.push(r"Windows Kits\10\Debuggers"); // We could check 8.1 etc. too?
path.push(cdb_arch);
path.push(r"cdb.exe");

if path.exists() {
Some(path.into_os_string())
} else {
None
}
}
else {
None
if !path.exists() {
return None;
}

Some(path.into_os_string())
}

/// Returns Path to CDB
Expand Down
6 changes: 1 addition & 5 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,7 @@ pub fn compute_stamp_hash(config: &Config) -> String {
config.stage_id.hash(&mut hash);

if config.mode == DebugInfoCdb {
match config.cdb {
None => env::var_os("ProgramFiles(x86)").hash(&mut hash),
Some(ref s) if s.is_empty() => env::var_os("ProgramFiles(x86)").hash(&mut hash),
Some(ref s) => s.hash(&mut hash),
}
config.cdb.hash(&mut hash);
}

if config.mode == DebugInfoGdb || config.mode == DebugInfoGdbLldb {
Expand Down

0 comments on commit 56b18ce

Please sign in to comment.