Skip to content
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.

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

rustfmt fixes #125637

Merged
merged 3 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 4 deletions compiler/rustc_mir_transform/src/instsimplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,8 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> {
for (i, field) in variant.fields.iter_enumerated() {
let field_ty = field.ty(self.tcx, args);
if field_ty == *cast_ty {
let place = place.project_deeper(
&[ProjectionElem::Field(i, *cast_ty)],
self.tcx,
);
let place = place
.project_deeper(&[ProjectionElem::Field(i, *cast_ty)], self.tcx);
let operand = if operand.is_move() {
Operand::Move(place)
} else {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
location,
format!(
"You can't project to field {f:?} of `DynMetadata` because \
layout is weird and thinks it doesn't have fields."
layout is weird and thinks it doesn't have fields."
),
);
}
Expand Down
8 changes: 2 additions & 6 deletions library/core/src/ptr/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,18 +209,14 @@ impl<Dyn: ?Sized> DynMetadata<Dyn> {
// Consider a reference like `&(i32, dyn Send)`: the vtable will only store the size of the
// `Send` part!
// SAFETY: DynMetadata always contains a valid vtable pointer
return unsafe {
crate::intrinsics::vtable_size(self.vtable_ptr() as *const ())
};
return unsafe { crate::intrinsics::vtable_size(self.vtable_ptr() as *const ()) };
}

/// Returns the alignment of the type associated with this vtable.
#[inline]
pub fn align_of(self) -> usize {
// SAFETY: DynMetadata always contains a valid vtable pointer
return unsafe {
crate::intrinsics::vtable_align(self.vtable_ptr() as *const ())
};
return unsafe { crate::intrinsics::vtable_align(self.vtable_ptr() as *const ()) };
}

/// Returns the size and alignment together as a `Layout`
Expand Down
19 changes: 6 additions & 13 deletions rustfmt.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,18 @@ version = "Two"
use_small_heuristics = "Max"
merge_derives = false

# by default we ignore everything in the repository
# tidy only checks files which are not ignored, each entry follows gitignore style
# Files to ignore. Each entry uses gitignore syntax, but `!` prefixes aren't allowed.
ignore = [
"/build/",
"/*-build/",
"/build-*/",
"/vendor/",

# tests for now are not formatted, as they are sometimes pretty-printing constrained
# (and generally rustfmt can move around comments in UI-testing incompatible ways)
"/tests/*",
# Tests for now are not formatted, as they are sometimes pretty-printing constrained
# (and generally rustfmt can move around comments in UI-testing incompatible ways).
"/tests/",

# but we still want to format rmake.rs files in tests/run-make/ so we need to do this
# dance to avoid the parent directory from being excluded
"!/tests/run-make/",
"/tests/run-make/*/*.rs",
"!/tests/run-make/*/rmake.rs",

# do not format submodules
# Do not format submodules.
# FIXME: sync submodule list with tidy/bootstrap/etc
# tidy/src/walk.rs:filter_dirs
"library/backtrace",
Expand All @@ -43,7 +36,7 @@ ignore = [
"src/tools/rustc-perf",
"src/tools/rustfmt",

# these are ignored by a standard cargo fmt run
# These are ignored by a standard cargo fmt run.
"compiler/rustc_codegen_cranelift/scripts",
"compiler/rustc_codegen_cranelift/example/gen_block_iterate.rs", # uses edition 2024
]
42 changes: 26 additions & 16 deletions src/bootstrap/src/core/build_steps/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use std::sync::mpsc::SyncSender;

fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl FnMut(bool) -> bool {
let mut cmd = Command::new(rustfmt);
// avoid the submodule config paths from coming into play,
// we only allow a single global config for the workspace for now
// Avoid the submodule config paths from coming into play. We only allow a single global config
// for the workspace for now.
cmd.arg("--config-path").arg(&src.canonicalize().unwrap());
cmd.arg("--edition").arg("2021");
cmd.arg("--unstable-features");
Expand All @@ -24,7 +24,7 @@ fn rustfmt(src: &Path, rustfmt: &Path, paths: &[PathBuf], check: bool) -> impl F
cmd.args(paths);
let cmd_debug = format!("{cmd:?}");
let mut cmd = cmd.spawn().expect("running rustfmt");
// poor man's async: return a closure that'll wait for rustfmt's completion
// Poor man's async: return a closure that'll wait for rustfmt's completion.
move |block: bool| -> bool {
if !block {
match cmd.try_wait() {
Expand Down Expand Up @@ -72,7 +72,7 @@ fn verify_rustfmt_version(build: &Builder<'_>) -> bool {
!program_out_of_date(&stamp_file, &version)
}

/// Updates the last rustfmt version used
/// Updates the last rustfmt version used.
fn update_rustfmt_version(build: &Builder<'_>) {
let Some((version, stamp_file)) = get_rustfmt_version(build) else {
return;
Expand Down Expand Up @@ -115,8 +115,15 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) {
let rustfmt_config: RustfmtConfig = t!(toml::from_str(&rustfmt_config));
let mut fmt_override = ignore::overrides::OverrideBuilder::new(&build.src);
for ignore in rustfmt_config.ignore {
if let Some(ignore) = ignore.strip_prefix('!') {
fmt_override.add(ignore).expect(ignore);
if ignore.starts_with('!') {
// A `!`-prefixed entry could be added as a whitelisted entry in `fmt_override`, i.e.
// strip the `!` prefix. But as soon as whitelisted entries are added, an
// `OverrideBuilder` will only traverse those whitelisted entries, and won't traverse
// any files that aren't explicitly mentioned. No bueno! Maybe there's a way to combine
// explicit whitelisted entries and traversal of unmentioned files, but for now just
// forbid such entries.
eprintln!("`!`-prefixed entries are not supported in rustfmt.toml, sorry");
crate::exit!(1);
} else {
fmt_override.add(&format!("!{ignore}")).expect(&ignore);
}
Expand Down Expand Up @@ -168,9 +175,10 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) {
untracked_count += 1;
fmt_override.add(&format!("!/{untracked_path}")).expect(untracked_path);
}
// Only check modified files locally to speed up runtime.
// We still check all files in CI to avoid bugs in `get_modified_rs_files` letting regressions slip through;
// we also care about CI time less since this is still very fast compared to building the compiler.
// Only check modified files locally to speed up runtime. We still check all files in
// CI to avoid bugs in `get_modified_rs_files` letting regressions slip through; we
// also care about CI time less since this is still very fast compared to building the
// compiler.
if !CiEnv::is_ci() && paths.is_empty() {
match get_modified_rs_files(build) {
Ok(Some(files)) => {
Expand Down Expand Up @@ -275,21 +283,23 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) {
.overrides(fmt_override)
.build_parallel();

// there is a lot of blocking involved in spawning a child process and reading files to format.
// spawn more processes than available concurrency to keep the CPU busy
// There is a lot of blocking involved in spawning a child process and reading files to format.
// Spawn more processes than available concurrency to keep the CPU busy.
let max_processes = build.jobs() as usize * 2;

// spawn child processes on a separate thread so we can batch entries we have received from ignore
// Spawn child processes on a separate thread so we can batch entries we have received from
// ignore.
let thread = std::thread::spawn(move || {
let mut children = VecDeque::new();
while let Ok(path) = rx.recv() {
// try getting more paths from the channel to amortize the overhead of spawning processes
// Try getting more paths from the channel to amortize the overhead of spawning
// processes.
let paths: Vec<_> = rx.try_iter().take(63).chain(std::iter::once(path)).collect();

let child = rustfmt(&src, &rustfmt_path, paths.as_slice(), check);
children.push_back(child);

// poll completion before waiting
// Poll completion before waiting.
for i in (0..children.len()).rev() {
if children[i](false) {
children.swap_remove_back(i);
Expand All @@ -298,12 +308,12 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) {
}

if children.len() >= max_processes {
// await oldest child
// Await oldest child.
children.pop_front().unwrap()(true);
}
}

// await remaining children
// Await remaining children.
for mut child in children {
child(true);
}
Expand Down
Loading