Skip to content

Commit

Permalink
Don't format tests/run-make/*/rmake.rs.
Browse files Browse the repository at this point in the history
It's reasonable to want to, but in the current implementation this
causes multiple problems.

- All the `rmake.rs` files are formatted every time even when they
  haven't changed. This is because they get whitelisted unconditionally
  in the `OverrideBuilder`, before the changed files get added.

- The way `OverrideBuilder` works, if any files gets whitelisted then no
  unmentioned files will get traversed. This is surprising, and means
  that the `rmake.rs` entries broke the use of explicit paths to `x
  fmt`, and also broke `GITHUB_ACTIONS=true git check --fmt`.

The commit removes the `rmake.rs` entries, fixes the formatting of a
couple of files that were misformatted (not previously caught due to the
`GITHUB_ACTIONS` breakage), and bans `!`-prefixed entries in
`rustfmt.toml` because they cause all these problems.
  • Loading branch information
nnethercote committed May 28, 2024
1 parent 4702a1c commit f1b0ca0
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 21 deletions.
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
10 changes: 2 additions & 8 deletions rustfmt.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ version = "Two"
use_small_heuristics = "Max"
merge_derives = false

# Files to ignore. Each entry uses gitignore syntax.
# Files to ignore. Each entry uses gitignore syntax, but `!` prefixes aren't allowed.
ignore = [
"/build/",
"/*-build/",
Expand All @@ -12,13 +12,7 @@ ignore = [

# 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",
"/tests/",

# Do not format submodules.
# FIXME: sync submodule list with tidy/bootstrap/etc
Expand Down
11 changes: 9 additions & 2 deletions src/bootstrap/src/core/build_steps/format.rs
Original file line number Diff line number Diff line change
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

0 comments on commit f1b0ca0

Please sign in to comment.