From c76e260fa7b8318216dac0f78fe008ce1e0f8684 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 5 Mar 2023 07:21:41 -0600 Subject: [PATCH 1/6] Use named threads in tidy This makes it easier to profile. --- src/tools/tidy/src/main.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index d98758ace4fc8..ced1c02401523 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -13,7 +13,7 @@ use std::path::PathBuf; use std::process; use std::str::FromStr; use std::sync::atomic::{AtomicBool, Ordering}; -use std::thread::{scope, ScopedJoinHandle}; +use std::thread::{self, scope, ScopedJoinHandle}; fn main() { let root_path: PathBuf = env::args_os().nth(1).expect("need path to root of repo").into(); @@ -55,16 +55,28 @@ fn main() { VecDeque::with_capacity(concurrency.get()); macro_rules! check { - ($p:ident $(, $args:expr)* ) => { + ($p:ident) => { + check!(@ $p, name=format!("{}", stringify!($p))); + }; + ($p:ident, $path:expr $(, $args:expr)* ) => { + let shortened = $path.strip_prefix(&root_path).unwrap(); + let name = if shortened == std::path::Path::new("") { + format!("{} (.)", stringify!($p)) + } else { + format!("{} ({})", stringify!($p), shortened.display()) + }; + check!(@ $p, name=name, $path $(,$args)*); + }; + (@ $p:ident, name=$name:expr $(, $args:expr)* ) => { drain_handles(&mut handles); - let handle = s.spawn(|| { + let handle = thread::Builder::new().name($name).spawn_scoped(s, || { let mut flag = false; $p::check($($args, )* &mut flag); if (flag) { bad.store(true, Ordering::Relaxed); } - }); + }).unwrap(); handles.push_back(handle); } } From 9b606a3203da5141f4922ef405ef0d1b9bf0b8bf Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 5 Mar 2023 06:45:07 -0600 Subject: [PATCH 2/6] Speed up file walking in tidy - Skip files in `skip` wherever possible to avoid reading their contents - Don't look for `tidy-alphabetic-start` in tests. It's never currently used and slows the check down a lot. - Add new `filter_not_rust` helper function --- src/tools/tidy/src/debug_artifacts.rs | 12 +-- src/tools/tidy/src/features.rs | 17 ++-- src/tools/tidy/src/main.rs | 1 - src/tools/tidy/src/style.rs | 26 ++--- src/tools/tidy/src/target_specific_tests.rs | 100 ++++++++++---------- src/tools/tidy/src/walk.rs | 7 +- 6 files changed, 80 insertions(+), 83 deletions(-) diff --git a/src/tools/tidy/src/debug_artifacts.rs b/src/tools/tidy/src/debug_artifacts.rs index 2241375eaae11..84b133068055b 100644 --- a/src/tools/tidy/src/debug_artifacts.rs +++ b/src/tools/tidy/src/debug_artifacts.rs @@ -1,21 +1,15 @@ //! Tidy check to prevent creation of unnecessary debug artifacts while running tests. -use crate::walk::{filter_dirs, walk}; +use crate::walk::{filter_dirs, filter_not_rust, walk}; use std::path::Path; const GRAPHVIZ_POSTFLOW_MSG: &str = "`borrowck_graphviz_postflow` attribute in test"; pub fn check(test_dir: &Path, bad: &mut bool) { - walk(test_dir, filter_dirs, &mut |entry, contents| { - let filename = entry.path(); - let is_rust = filename.extension().map_or(false, |ext| ext == "rs"); - if !is_rust { - return; - } - + walk(test_dir, |path| filter_dirs(path) || filter_not_rust(path), &mut |entry, contents| { for (i, line) in contents.lines().enumerate() { if line.contains("borrowck_graphviz_postflow") { - tidy_error!(bad, "{}:{}: {}", filename.display(), i + 1, GRAPHVIZ_POSTFLOW_MSG); + tidy_error!(bad, "{}:{}: {}", entry.path().display(), i + 1, GRAPHVIZ_POSTFLOW_MSG); } } }); diff --git a/src/tools/tidy/src/features.rs b/src/tools/tidy/src/features.rs index 6d94417a10f1d..f18feda533c9c 100644 --- a/src/tools/tidy/src/features.rs +++ b/src/tools/tidy/src/features.rs @@ -9,8 +9,9 @@ //! * All unstable lang features have tests to ensure they are actually unstable. //! * Language features in a group are sorted by feature name. -use crate::walk::{filter_dirs, walk, walk_many}; +use crate::walk::{filter_dirs, filter_not_rust, walk, walk_many}; use std::collections::hash_map::{Entry, HashMap}; +use std::ffi::OsStr; use std::fmt; use std::fs; use std::num::NonZeroU32; @@ -101,17 +102,15 @@ pub fn check( &tests_path.join("rustdoc-ui"), &tests_path.join("rustdoc"), ], - filter_dirs, + |path| { + filter_dirs(path) + || filter_not_rust(path) + || path.file_name() == Some(OsStr::new("features.rs")) + || path.file_name() == Some(OsStr::new("diagnostic_list.rs")) + }, &mut |entry, contents| { let file = entry.path(); let filename = file.file_name().unwrap().to_string_lossy(); - if !filename.ends_with(".rs") - || filename == "features.rs" - || filename == "diagnostic_list.rs" - { - return; - } - let filen_underscore = filename.replace('-', "_").replace(".rs", ""); let filename_is_gate_test = test_filen_gate(&filen_underscore, &mut features); diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index ced1c02401523..f59406c404bab 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -120,7 +120,6 @@ fn main() { check!(edition, &library_path); check!(alphabetical, &src_path); - check!(alphabetical, &tests_path); check!(alphabetical, &compiler_path); check!(alphabetical, &library_path); diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index 75a4586cb7f1c..e3f04840bc71b 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -19,7 +19,7 @@ use crate::walk::{filter_dirs, walk}; use regex::{Regex, RegexSet}; -use std::path::Path; +use std::{ffi::OsStr, path::Path}; /// Error code markdown is restricted to 80 columns because they can be /// displayed on the console with --example. @@ -228,21 +228,28 @@ fn is_unexplained_ignore(extension: &str, line: &str) -> bool { pub fn check(path: &Path, bad: &mut bool) { fn skip(path: &Path) -> bool { - filter_dirs(path) || skip_markdown_path(path) + if filter_dirs(path) || skip_markdown_path(path) { + return true; + } + + let extensions = ["rs", "py", "js", "sh", "c", "cpp", "h", "md", "css", "ftl", "goml"]; + if extensions.iter().all(|e| path.extension() != Some(OsStr::new(e))) { + return true; + } + + // We only check CSS files in rustdoc. + path.extension().map_or(false, |e| e == "css") && !is_in(path, "src", "librustdoc") } + let problematic_consts_strings: Vec = (PROBLEMATIC_CONSTS.iter().map(u32::to_string)) .chain(PROBLEMATIC_CONSTS.iter().map(|v| format!("{:x}", v))) .chain(PROBLEMATIC_CONSTS.iter().map(|v| format!("{:X}", v))) .collect(); let problematic_regex = RegexSet::new(problematic_consts_strings.as_slice()).unwrap(); + walk(path, skip, &mut |entry, contents| { let file = entry.path(); let filename = file.file_name().unwrap().to_string_lossy(); - let extensions = - [".rs", ".py", ".js", ".sh", ".c", ".cpp", ".h", ".md", ".css", ".ftl", ".goml"]; - if extensions.iter().all(|e| !filename.ends_with(e)) || filename.starts_with(".#") { - return; - } let is_style_file = filename.ends_with(".css"); let under_rustfmt = filename.ends_with(".rs") && @@ -253,11 +260,6 @@ pub fn check(path: &Path, bad: &mut bool) { a.ends_with("src/doc/book") }); - if is_style_file && !is_in(file, "src", "librustdoc") { - // We only check CSS files in rustdoc. - return; - } - if contents.is_empty() { tidy_error!(bad, "{}: empty file", file.display()); } diff --git a/src/tools/tidy/src/target_specific_tests.rs b/src/tools/tidy/src/target_specific_tests.rs index f41fa4fcce1b5..e0fa6aceb853c 100644 --- a/src/tools/tidy/src/target_specific_tests.rs +++ b/src/tools/tidy/src/target_specific_tests.rs @@ -4,6 +4,8 @@ use std::collections::BTreeMap; use std::path::Path; +use crate::walk::filter_not_rust; + const COMMENT: &str = "//"; const LLVM_COMPONENTS_HEADER: &str = "needs-llvm-components:"; const COMPILE_FLAGS_HEADER: &str = "compile-flags:"; @@ -35,61 +37,57 @@ struct RevisionInfo<'a> { } pub fn check(path: &Path, bad: &mut bool) { - crate::walk::walk( - path, - |path| path.extension().map(|p| p == "rs") == Some(false), - &mut |entry, content| { - let file = entry.path().display(); - let mut header_map = BTreeMap::new(); - iter_header(content, &mut |cfg, directive| { - if let Some(value) = directive.strip_prefix(LLVM_COMPONENTS_HEADER) { - let info = header_map.entry(cfg).or_insert(RevisionInfo::default()); - let comp_vec = info.llvm_components.get_or_insert(Vec::new()); - for component in value.split(' ') { - let component = component.trim(); - if !component.is_empty() { - comp_vec.push(component); - } - } - } else if directive.starts_with(COMPILE_FLAGS_HEADER) { - let compile_flags = &directive[COMPILE_FLAGS_HEADER.len()..]; - if let Some((_, v)) = compile_flags.split_once("--target") { - if let Some((arch, _)) = - v.trim_start_matches(|c| c == ' ' || c == '=').split_once("-") - { - let info = header_map.entry(cfg).or_insert(RevisionInfo::default()); - info.target_arch.replace(arch); - } else { - eprintln!("{file}: seems to have a malformed --target value"); - *bad = true; - } + crate::walk::walk(path, filter_not_rust, &mut |entry, content| { + let file = entry.path().display(); + let mut header_map = BTreeMap::new(); + iter_header(content, &mut |cfg, directive| { + if let Some(value) = directive.strip_prefix(LLVM_COMPONENTS_HEADER) { + let info = header_map.entry(cfg).or_insert(RevisionInfo::default()); + let comp_vec = info.llvm_components.get_or_insert(Vec::new()); + for component in value.split(' ') { + let component = component.trim(); + if !component.is_empty() { + comp_vec.push(component); } } - }); - for (rev, RevisionInfo { target_arch, llvm_components }) in &header_map { - let rev = rev.unwrap_or("[unspecified]"); - match (target_arch, llvm_components) { - (None, None) => {} - (Some(_), None) => { - eprintln!( - "{}: revision {} should specify `{}` as it has `--target` set", - file, rev, LLVM_COMPONENTS_HEADER - ); + } else if directive.starts_with(COMPILE_FLAGS_HEADER) { + let compile_flags = &directive[COMPILE_FLAGS_HEADER.len()..]; + if let Some((_, v)) = compile_flags.split_once("--target") { + if let Some((arch, _)) = + v.trim_start_matches(|c| c == ' ' || c == '=').split_once("-") + { + let info = header_map.entry(cfg).or_insert(RevisionInfo::default()); + info.target_arch.replace(arch); + } else { + eprintln!("{file}: seems to have a malformed --target value"); *bad = true; } - (None, Some(_)) => { - eprintln!( - "{}: revision {} should not specify `{}` as it doesn't need `--target`", - file, rev, LLVM_COMPONENTS_HEADER - ); - *bad = true; - } - (Some(_), Some(_)) => { - // FIXME: check specified components against the target architectures we - // gathered. - } } } - }, - ); + }); + for (rev, RevisionInfo { target_arch, llvm_components }) in &header_map { + let rev = rev.unwrap_or("[unspecified]"); + match (target_arch, llvm_components) { + (None, None) => {} + (Some(_), None) => { + eprintln!( + "{}: revision {} should specify `{}` as it has `--target` set", + file, rev, LLVM_COMPONENTS_HEADER + ); + *bad = true; + } + (None, Some(_)) => { + eprintln!( + "{}: revision {} should not specify `{}` as it doesn't need `--target`", + file, rev, LLVM_COMPONENTS_HEADER + ); + *bad = true; + } + (Some(_), Some(_)) => { + // FIXME: check specified components against the target architectures we + // gathered. + } + } + } + }); } diff --git a/src/tools/tidy/src/walk.rs b/src/tools/tidy/src/walk.rs index 94152e75168f9..90cd62531d0c2 100644 --- a/src/tools/tidy/src/walk.rs +++ b/src/tools/tidy/src/walk.rs @@ -1,6 +1,6 @@ use ignore::DirEntry; -use std::{fs::File, io::Read, path::Path}; +use std::{ffi::OsStr, fs::File, io::Read, path::Path}; /// The default directory filter. pub fn filter_dirs(path: &Path) -> bool { @@ -33,6 +33,11 @@ pub fn filter_dirs(path: &Path) -> bool { skip.iter().any(|p| path.ends_with(p)) } +/// Filter for only files that end in `.rs`. +pub fn filter_not_rust(path: &Path) -> bool { + !path.is_dir() && path.extension() != Some(OsStr::new("rs")) +} + pub fn walk_many( paths: &[&Path], skip: impl Clone + Send + Sync + 'static + Fn(&Path) -> bool, From d26a15563d34c3c77788e48d880d1c39d3770fb3 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 5 Mar 2023 07:37:44 -0600 Subject: [PATCH 3/6] Make `ui_tests` non-quadratic Previously, it would walk each directory twice: once in the main `Walk` iterator, and once to count the number of entries in the directory. Now it only walks each directory once. --- src/tools/tidy/src/ui_tests.rs | 64 ++++++++++++++++------------------ 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 15c36923e885f..ec8edf84ef38a 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -3,50 +3,46 @@ //! - there are no stray `.stderr` files use ignore::Walk; -use ignore::WalkBuilder; +use std::collections::HashMap; use std::fs; -use std::path::Path; +use std::path::{Path, PathBuf}; const ENTRY_LIMIT: usize = 1000; // FIXME: The following limits should be reduced eventually. const ROOT_ENTRY_LIMIT: usize = 940; const ISSUES_ENTRY_LIMIT: usize = 1978; -fn check_entries(path: &Path, bad: &mut bool) { - for dir in Walk::new(&path.join("ui")) { +fn check_entries(tests_path: &Path, bad: &mut bool) { + let mut directories: HashMap = HashMap::new(); + + for dir in Walk::new(&tests_path.join("ui")) { if let Ok(entry) = dir { - if entry.file_type().map(|ft| ft.is_dir()).unwrap_or(false) { - let dir_path = entry.path(); - // Use special values for these dirs. - let is_root = path.join("ui") == dir_path; - let is_issues_dir = path.join("ui/issues") == dir_path; - let limit = if is_root { - ROOT_ENTRY_LIMIT - } else if is_issues_dir { - ISSUES_ENTRY_LIMIT - } else { - ENTRY_LIMIT - }; + let parent = entry.path().parent().unwrap().to_path_buf(); + *directories.entry(parent).or_default() += 1; + } + } - let count = WalkBuilder::new(&dir_path) - .max_depth(Some(1)) - .build() - .into_iter() - .collect::>() - .len() - - 1; // remove the dir itself + for (dir_path, count) in directories { + // Use special values for these dirs. + let is_root = tests_path.join("ui") == dir_path; + let is_issues_dir = tests_path.join("ui/issues") == dir_path; + let limit = if is_root { + ROOT_ENTRY_LIMIT + } else if is_issues_dir { + ISSUES_ENTRY_LIMIT + } else { + ENTRY_LIMIT + }; - if count > limit { - tidy_error!( - bad, - "following path contains more than {} entries, \ - you should move the test to some relevant subdirectory (current: {}): {}", - limit, - count, - dir_path.display() - ); - } - } + if count > limit { + tidy_error!( + bad, + "following path contains more than {} entries, \ + you should move the test to some relevant subdirectory (current: {}): {}", + limit, + count, + dir_path.display() + ); } } } From 19b272a94b6f311affc86d721c34519299144681 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 5 Mar 2023 07:49:28 -0600 Subject: [PATCH 4/6] Use a single WalkBuilder for multiple paths --- src/tools/tidy/src/bins.rs | 2 +- src/tools/tidy/src/ui_tests.rs | 58 +++++++++++++++++----------------- src/tools/tidy/src/walk.rs | 25 ++++++++------- 3 files changed, 43 insertions(+), 42 deletions(-) diff --git a/src/tools/tidy/src/bins.rs b/src/tools/tidy/src/bins.rs index 2d6abe59343f2..070ce93f97c17 100644 --- a/src/tools/tidy/src/bins.rs +++ b/src/tools/tidy/src/bins.rs @@ -103,7 +103,7 @@ mod os_impl { // FIXME: we don't need to look at all binaries, only files that have been modified in this branch // (e.g. using `git ls-files`). - walk_no_read(path, |path| filter_dirs(path) || path.ends_with("src/etc"), &mut |entry| { + walk_no_read(&[path], |path| filter_dirs(path) || path.ends_with("src/etc"), &mut |entry| { let file = entry.path(); let extension = file.extension(); let scripts = ["py", "sh", "ps1"]; diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index ec8edf84ef38a..66f5c932be2a0 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -49,37 +49,37 @@ fn check_entries(tests_path: &Path, bad: &mut bool) { pub fn check(path: &Path, bad: &mut bool) { check_entries(&path, bad); - for path in &[&path.join("ui"), &path.join("ui-fulldeps")] { - crate::walk::walk_no_read(path, |_| false, &mut |entry| { - let file_path = entry.path(); - if let Some(ext) = file_path.extension() { - if ext == "stderr" || ext == "stdout" { - // Test output filenames have one of the formats: - // ``` - // $testname.stderr - // $testname.$mode.stderr - // $testname.$revision.stderr - // $testname.$revision.$mode.stderr - // ``` - // - // For now, just make sure that there is a corresponding - // `$testname.rs` file. - // - // NB: We do not use file_stem() as some file names have multiple `.`s and we - // must strip all of them. - let testname = - file_path.file_name().unwrap().to_str().unwrap().split_once('.').unwrap().0; - if !file_path.with_file_name(testname).with_extension("rs").exists() { - tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path); - } + let (ui, ui_fulldeps) = (path.join("ui"), path.join("ui-fulldeps")); + let paths = [ui.as_path(), ui_fulldeps.as_path()]; + crate::walk::walk_no_read(&paths, |_| false, &mut |entry| { + let file_path = entry.path(); + if let Some(ext) = file_path.extension() { + if ext == "stderr" || ext == "stdout" { + // Test output filenames have one of the formats: + // ``` + // $testname.stderr + // $testname.$mode.stderr + // $testname.$revision.stderr + // $testname.$revision.$mode.stderr + // ``` + // + // For now, just make sure that there is a corresponding + // `$testname.rs` file. + // + // NB: We do not use file_stem() as some file names have multiple `.`s and we + // must strip all of them. + let testname = + file_path.file_name().unwrap().to_str().unwrap().split_once('.').unwrap().0; + if !file_path.with_file_name(testname).with_extension("rs").exists() { + tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path); + } - if let Ok(metadata) = fs::metadata(file_path) { - if metadata.len() == 0 { - tidy_error!(bad, "Empty file with UI testing output: {:?}", file_path); - } + if let Ok(metadata) = fs::metadata(file_path) { + if metadata.len() == 0 { + tidy_error!(bad, "Empty file with UI testing output: {:?}", file_path); } } } - }); - } + } + }); } diff --git a/src/tools/tidy/src/walk.rs b/src/tools/tidy/src/walk.rs index 90cd62531d0c2..2ade22c209f5a 100644 --- a/src/tools/tidy/src/walk.rs +++ b/src/tools/tidy/src/walk.rs @@ -35,26 +35,24 @@ pub fn filter_dirs(path: &Path) -> bool { /// Filter for only files that end in `.rs`. pub fn filter_not_rust(path: &Path) -> bool { - !path.is_dir() && path.extension() != Some(OsStr::new("rs")) + path.extension() != Some(OsStr::new("rs")) && !path.is_dir() } -pub fn walk_many( - paths: &[&Path], +pub fn walk( + path: &Path, skip: impl Clone + Send + Sync + 'static + Fn(&Path) -> bool, f: &mut dyn FnMut(&DirEntry, &str), ) { - for path in paths { - walk(path, skip.clone(), f); - } + walk_many(&[path], skip, f); } -pub fn walk( - path: &Path, - skip: impl Send + Sync + 'static + Fn(&Path) -> bool, +pub fn walk_many( + paths: &[&Path], + skip: impl Clone + Send + Sync + 'static + Fn(&Path) -> bool, f: &mut dyn FnMut(&DirEntry, &str), ) { let mut contents = Vec::new(); - walk_no_read(path, skip, &mut |entry| { + walk_no_read(paths, skip, &mut |entry| { contents.clear(); let mut file = t!(File::open(entry.path()), entry.path()); t!(file.read_to_end(&mut contents), entry.path()); @@ -67,11 +65,14 @@ pub fn walk( } pub(crate) fn walk_no_read( - path: &Path, + paths: &[&Path], skip: impl Send + Sync + 'static + Fn(&Path) -> bool, f: &mut dyn FnMut(&DirEntry), ) { - let mut walker = ignore::WalkBuilder::new(path); + let mut walker = ignore::WalkBuilder::new(paths[0]); + for path in &paths[1..] { + walker.add(path); + } let walker = walker.filter_entry(move |e| !skip(e.path())); for entry in walker.build() { if let Ok(entry) = entry { From 3a58b2b3b065fcb367357482f636ce918d1080bd Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 5 Mar 2023 07:59:36 -0600 Subject: [PATCH 5/6] Let tidy use more threads This has a significant speedup for me locally, from about 1.3 seconds to .9 seconds. --- src/bootstrap/test.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index f5d680df1133b..baddc9da48db8 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -1118,7 +1118,11 @@ impl Step for Tidy { cmd.arg(&builder.src); cmd.arg(&builder.initial_cargo); cmd.arg(&builder.out); - cmd.arg(builder.jobs().to_string()); + // Tidy is heavily IO constrained. Still respect `-j`, but use a higher limit if `jobs` hasn't been configured. + let jobs = builder.config.jobs.unwrap_or_else(|| { + 8 * std::thread::available_parallelism().map_or(1, std::num::NonZeroUsize::get) as u32 + }); + cmd.arg(jobs.to_string()); if builder.is_verbose() { cmd.arg("--verbose"); } From 675c4aa2c1c142415d4e95bf550ec0b1de2493d0 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 18 Mar 2023 00:29:53 -0500 Subject: [PATCH 6/6] address review comments --- .gitignore | 1 + .../src/build/expr/as_rvalue.rs | 47 ++++++++++++++----- .../rustc_mir_build/src/build/matches/mod.rs | 4 +- compiler/rustc_mir_build/src/build/mod.rs | 19 ++++---- src/tools/tidy/src/style.rs | 5 ++ 5 files changed, 52 insertions(+), 24 deletions(-) diff --git a/.gitignore b/.gitignore index ce797a7a8371d..04d2597ecc696 100644 --- a/.gitignore +++ b/.gitignore @@ -42,6 +42,7 @@ no_llvm_build /llvm/ /mingw-build/ build/ +!/compiler/rustc_mir_build/src/build/ /build-rust-analyzer/ /dist/ /unicode-downloads diff --git a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs index 9f48986b1ad80..140d1154718f7 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -73,19 +73,34 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } ExprKind::Binary { op, lhs, rhs } => { let lhs = unpack!( - block = - this.as_operand(block, scope, &this.thir[lhs], LocalInfo::Boring, NeedsTemporary::Maybe) + block = this.as_operand( + block, + scope, + &this.thir[lhs], + LocalInfo::Boring, + NeedsTemporary::Maybe + ) ); let rhs = unpack!( - block = - this.as_operand(block, scope, &this.thir[rhs], LocalInfo::Boring, NeedsTemporary::No) + block = this.as_operand( + block, + scope, + &this.thir[rhs], + LocalInfo::Boring, + NeedsTemporary::No + ) ); this.build_binary_op(block, op, expr_span, expr.ty, lhs, rhs) } ExprKind::Unary { op, arg } => { let arg = unpack!( - block = - this.as_operand(block, scope, &this.thir[arg], LocalInfo::Boring, NeedsTemporary::No) + block = this.as_operand( + block, + scope, + &this.thir[arg], + LocalInfo::Boring, + NeedsTemporary::No + ) ); // Check for -MIN on signed integers if this.check_overflow && op == UnOp::Neg && expr.ty.is_signed() { @@ -272,8 +287,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } ExprKind::Pointer { cast, source } => { let source = unpack!( - block = - this.as_operand(block, scope, &this.thir[source], LocalInfo::Boring, NeedsTemporary::No) + block = this.as_operand( + block, + scope, + &this.thir[source], + LocalInfo::Boring, + NeedsTemporary::No + ) ); block.and(Rvalue::Cast(CastKind::Pointer(cast), source, expr.ty)) } @@ -502,8 +522,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Category::of(&expr.kind), Some(Category::Rvalue(RvalueFunc::AsRvalue) | Category::Constant) )); - let operand = - unpack!(block = this.as_operand(block, scope, expr, LocalInfo::Boring, NeedsTemporary::No)); + let operand = unpack!( + block = + this.as_operand(block, scope, expr, LocalInfo::Boring, NeedsTemporary::No) + ); block.and(Rvalue::Use(operand)) } } @@ -662,8 +684,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Repeating a const does nothing } else { // For a non-const, we may need to generate an appropriate `Drop` - let value_operand = - unpack!(block = this.as_operand(block, scope, value, LocalInfo::Boring, NeedsTemporary::No)); + let value_operand = unpack!( + block = this.as_operand(block, scope, value, LocalInfo::Boring, NeedsTemporary::No) + ); if let Operand::Move(to_drop) = value_operand { let success = this.cfg.start_new_block(); this.cfg.terminate( diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 22785dfd2ce0e..2d52102db2cc6 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -2252,7 +2252,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { user_ty: None, source_info, internal: false, - local_info: ClearCrossCrate::Set(Box::new(LocalInfo::User(BindingForm::RefForGuard))), + local_info: ClearCrossCrate::Set(Box::new(LocalInfo::User( + BindingForm::RefForGuard, + ))), }); self.var_debug_info.push(VarDebugInfo { name, diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 6814fd4cb35e0..80d8b27336c54 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -876,21 +876,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } => { self.local_decls[local].mutability = mutability; self.local_decls[local].source_info.scope = self.source_scope; - **self.local_decls[local].local_info.as_mut().assert_crate_local() = if let Some(kind) = param.self_kind { - LocalInfo::User( - BindingForm::ImplicitSelf(kind), - ) - } else { - let binding_mode = ty::BindingMode::BindByValue(mutability); - LocalInfo::User(BindingForm::Var( - VarBindingForm { + **self.local_decls[local].local_info.as_mut().assert_crate_local() = + if let Some(kind) = param.self_kind { + LocalInfo::User(BindingForm::ImplicitSelf(kind)) + } else { + let binding_mode = ty::BindingMode::BindByValue(mutability); + LocalInfo::User(BindingForm::Var(VarBindingForm { binding_mode, opt_ty_info: param.ty_span, opt_match_place: Some((None, span)), pat_span: span, - }, - )) - }; + })) + }; self.var_indices.insert(var, LocalsForNode::One(local)); } _ => { diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index e3f04840bc71b..a965c98f48491 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -228,6 +228,11 @@ fn is_unexplained_ignore(extension: &str, line: &str) -> bool { pub fn check(path: &Path, bad: &mut bool) { fn skip(path: &Path) -> bool { + if path.file_name().map_or(false, |name| name.to_string_lossy().starts_with(".#")) { + // vim or emacs temporary file + return true; + } + if filter_dirs(path) || skip_markdown_path(path) { return true; }