Skip to content

Commit

Permalink
Auto merge of #1850 - RalfJung:fmt, r=RalfJung
Browse files Browse the repository at this point in the history
fmt: set force_multiline_blocks=true

This is an experiment, I am not yet sure if I like it... but it does prevent rustfmt from putting stuff after the `=>` in a `match` (unless the entire arm fits there), which IMO is a big plus. What do others think?
(I also tried setting `match_arm_blocks` back to its default of `true`, but that adds too many braces for my taste.)

Btw, `@calebcartwright` is the interaction of `match_arm_blocks = false` and `force_multiline_blocks = true` as can be seen here expected? I think I like it, but it it is not at all what I expected from the docs which describe `force_multiline_blocks = true` as "Force multiline closure and match arm bodies to be wrapped in a block" -- but here that is not the effect it has, there are no new blocks being added.
  • Loading branch information
bors committed Jul 12, 2021
2 parents b061307 + cffa1d3 commit a4a9a36
Show file tree
Hide file tree
Showing 12 changed files with 149 additions and 120 deletions.
7 changes: 4 additions & 3 deletions benches/helpers/miri_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ fn find_sysroot() -> String {
let toolchain = option_env!("RUSTUP_TOOLCHAIN").or(option_env!("MULTIRUST_TOOLCHAIN"));
match (home, toolchain) {
(Some(home), Some(toolchain)) => format!("{}/toolchains/{}", home, toolchain),
_ => option_env!("RUST_SYSROOT")
.expect("need to specify RUST_SYSROOT env var or use rustup or multirust")
.to_owned(),
_ =>
option_env!("RUST_SYSROOT")
.expect("need to specify RUST_SYSROOT env var or use rustup or multirust")
.to_owned(),
}
}

Expand Down
14 changes: 8 additions & 6 deletions cargo-miri/bin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,9 +528,10 @@ fn phase_cargo_miri(mut args: env::Args) {
Some("run") => MiriCommand::Run,
Some("setup") => MiriCommand::Setup,
// Invalid command.
_ => show_error(format!(
"`cargo miri` supports the following subcommands: `run`, `test`, and `setup`."
)),
_ =>
show_error(format!(
"`cargo miri` supports the following subcommands: `run`, `test`, and `setup`."
)),
};
let verbose = has_arg_flag("-v");

Expand Down Expand Up @@ -1086,8 +1087,9 @@ fn main() {
));
}
}
_ => show_error(format!(
"`cargo-miri` called without first argument; please only invoke this binary through `cargo miri`"
)),
_ =>
show_error(format!(
"`cargo-miri` called without first argument; please only invoke this binary through `cargo miri`"
)),
}
}
1 change: 1 addition & 0 deletions rustfmt.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ version = "Two"
use_small_heuristics = "Max"
match_arm_blocks = false
match_arm_leading_pipes = "Preserve"
force_multiline_blocks = true
41 changes: 24 additions & 17 deletions src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,12 @@ fn compile_time_sysroot() -> Option<String> {
let toolchain = option_env!("RUSTUP_TOOLCHAIN").or(option_env!("MULTIRUST_TOOLCHAIN"));
Some(match (home, toolchain) {
(Some(home), Some(toolchain)) => format!("{}/toolchains/{}", home, toolchain),
_ => option_env!("RUST_SYSROOT")
.expect("To build Miri without rustup, set the `RUST_SYSROOT` env var at build time")
.to_owned(),
_ =>
option_env!("RUST_SYSROOT")
.expect(
"To build Miri without rustup, set the `RUST_SYSROOT` env var at build time",
)
.to_owned(),
})
}

Expand Down Expand Up @@ -336,9 +339,10 @@ fn main() {
"warn" => miri::IsolatedOp::Reject(miri::RejectOpWith::Warning),
"warn-nobacktrace" =>
miri::IsolatedOp::Reject(miri::RejectOpWith::WarningWithoutBacktrace),
_ => panic!(
"-Zmiri-isolation-error must be `abort`, `hide`, `warn`, or `warn-nobacktrace`"
),
_ =>
panic!(
"-Zmiri-isolation-error must be `abort`, `hide`, `warn`, or `warn-nobacktrace`"
),
};
}
"-Zmiri-ignore-leaks" => {
Expand Down Expand Up @@ -383,10 +387,11 @@ fn main() {
let id: u64 =
match arg.strip_prefix("-Zmiri-track-pointer-tag=").unwrap().parse() {
Ok(id) => id,
Err(err) => panic!(
"-Zmiri-track-pointer-tag requires a valid `u64` argument: {}",
err
),
Err(err) =>
panic!(
"-Zmiri-track-pointer-tag requires a valid `u64` argument: {}",
err
),
};
if let Some(id) = miri::PtrId::new(id) {
miri_config.tracked_pointer_tag = Some(id);
Expand Down Expand Up @@ -422,13 +427,15 @@ fn main() {
.parse::<f64>()
{
Ok(rate) if rate >= 0.0 && rate <= 1.0 => rate,
Ok(_) => panic!(
"-Zmiri-compare-exchange-weak-failure-rate must be between `0.0` and `1.0`"
),
Err(err) => panic!(
"-Zmiri-compare-exchange-weak-failure-rate requires a `f64` between `0.0` and `1.0`: {}",
err
),
Ok(_) =>
panic!(
"-Zmiri-compare-exchange-weak-failure-rate must be between `0.0` and `1.0`"
),
Err(err) =>
panic!(
"-Zmiri-compare-exchange-weak-failure-rate requires a `f64` between `0.0` and `1.0`: {}",
err
),
};
miri_config.cmpxchg_weak_failure_rate = rate;
}
Expand Down
11 changes: 6 additions & 5 deletions src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ impl fmt::Display for TerminationInfo {
Deadlock => write!(f, "the evaluated program deadlocked"),
MultipleSymbolDefinitions { link_name, .. } =>
write!(f, "multiple definitions of symbol `{}`", link_name),
SymbolShimClashing { link_name, .. } => write!(
f,
"found `{}` symbol definition that clashes with a built-in shim",
link_name
),
SymbolShimClashing { link_name, .. } =>
write!(
f,
"found `{}` symbol definition that clashes with a built-in shim",
link_name
),
}
}
}
Expand Down
9 changes: 5 additions & 4 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,10 +498,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
match err_kind {
NotFound => "ERROR_FILE_NOT_FOUND",
PermissionDenied => "ERROR_ACCESS_DENIED",
_ => throw_unsup_format!(
"io error {:?} cannot be translated into a raw os error",
err_kind
),
_ =>
throw_unsup_format!(
"io error {:?} cannot be translated into a raw os error",
err_kind
),
},
)?
} else {
Expand Down
9 changes: 5 additions & 4 deletions src/shims/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,11 @@ impl<'tcx> EnvVars<'tcx> {
"linux" | "macos" =>
alloc_env_var_as_c_str(name.as_ref(), value.as_ref(), ecx)?,
"windows" => alloc_env_var_as_wide_str(name.as_ref(), value.as_ref(), ecx)?,
unsupported => throw_unsup_format!(
"environment support for target OS `{}` not yet available",
unsupported
),
unsupported =>
throw_unsup_format!(
"environment support for target OS `{}` not yet available",
unsupported
),
};
ecx.machine.env_vars.map.insert(OsString::from(name), var_ptr);
}
Expand Down
96 changes: 49 additions & 47 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,54 +223,56 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx

// First: functions that diverge.
let (dest, ret) = match ret {
None => match &*link_name.as_str() {
"miri_start_panic" => {
// `check_shim` happens inside `handle_miri_start_panic`.
this.handle_miri_start_panic(abi, link_name, args, unwind)?;
return Ok(None);
}
// This matches calls to the foreign item `panic_impl`.
// The implementation is provided by the function with the `#[panic_handler]` attribute.
"panic_impl" => {
// We don't use `check_shim` here because we are just forwarding to the lang
// item. Argument count checking will be performed when the returned `Body` is
// called.
this.check_abi_and_shim_symbol_clash(abi, Abi::Rust, link_name)?;
let panic_impl_id = tcx.lang_items().panic_impl().unwrap();
let panic_impl_instance = ty::Instance::mono(tcx, panic_impl_id);
return Ok(Some(&*this.load_mir(panic_impl_instance.def, None)?));
}
#[rustfmt::skip]
| "exit"
| "ExitProcess"
=> {
let exp_abi = if link_name.as_str() == "exit" {
Abi::C { unwind: false }
} else {
Abi::System { unwind: false }
};
let &[ref code] = this.check_shim(abi, exp_abi, link_name, args)?;
// it's really u32 for ExitProcess, but we have to put it into the `Exit` variant anyway
let code = this.read_scalar(code)?.to_i32()?;
throw_machine_stop!(TerminationInfo::Exit(code.into()));
}
"abort" => {
let &[] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
throw_machine_stop!(TerminationInfo::Abort(
"the program aborted execution".to_owned()
))
}
_ => {
if let Some(body) = this.lookup_exported_symbol(link_name)? {
return Ok(Some(body));
None =>
match &*link_name.as_str() {
"miri_start_panic" => {
// `check_shim` happens inside `handle_miri_start_panic`.
this.handle_miri_start_panic(abi, link_name, args, unwind)?;
return Ok(None);
}
this.handle_unsupported(format!(
"can't call (diverging) foreign function: {}",
link_name
))?;
return Ok(None);
}
},
// This matches calls to the foreign item `panic_impl`.
// The implementation is provided by the function with the `#[panic_handler]` attribute.
"panic_impl" => {
// We don't use `check_shim` here because we are just forwarding to the lang
// item. Argument count checking will be performed when the returned `Body` is
// called.
this.check_abi_and_shim_symbol_clash(abi, Abi::Rust, link_name)?;
let panic_impl_id = tcx.lang_items().panic_impl().unwrap();
let panic_impl_instance = ty::Instance::mono(tcx, panic_impl_id);
return Ok(Some(&*this.load_mir(panic_impl_instance.def, None)?));
}
#[rustfmt::skip]
| "exit"
| "ExitProcess"
=> {
let exp_abi = if link_name.as_str() == "exit" {
Abi::C { unwind: false }
} else {
Abi::System { unwind: false }
};
let &[ref code] = this.check_shim(abi, exp_abi, link_name, args)?;
// it's really u32 for ExitProcess, but we have to put it into the `Exit` variant anyway
let code = this.read_scalar(code)?.to_i32()?;
throw_machine_stop!(TerminationInfo::Exit(code.into()));
}
"abort" => {
let &[] =
this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
throw_machine_stop!(TerminationInfo::Abort(
"the program aborted execution".to_owned()
))
}
_ => {
if let Some(body) = this.lookup_exported_symbol(link_name)? {
return Ok(Some(body));
}
this.handle_unsupported(format!(
"can't call (diverging) foreign function: {}",
link_name
))?;
return Ok(None);
}
},
Some(p) => p,
};

Expand Down
9 changes: 5 additions & 4 deletions src/shims/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,10 +295,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
this.float_to_int_unchecked(val.to_scalar()?.to_f32()?, dest.layout.ty)?,
ty::Float(FloatTy::F64) =>
this.float_to_int_unchecked(val.to_scalar()?.to_f64()?, dest.layout.ty)?,
_ => bug!(
"`float_to_int_unchecked` called with non-float input type {:?}",
val.layout.ty
),
_ =>
bug!(
"`float_to_int_unchecked` called with non-float input type {:?}",
val.layout.ty
),
};

this.write_scalar(res, dest)?;
Expand Down
45 changes: 27 additions & 18 deletions src/shims/posix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,10 +434,11 @@ trait EvalContextExtPrivate<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, '
Err(e) =>
return match e.raw_os_error() {
Some(error) => Ok(error),
None => throw_unsup_format!(
"the error {} couldn't be converted to a return value",
e
),
None =>
throw_unsup_format!(
"the error {} couldn't be converted to a return value",
e
),
},
}
}
Expand Down Expand Up @@ -1203,13 +1204,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
this.write_null(&this.deref_operand(result_op)?.into())?;
Ok(0)
}
Some(Err(e)) => match e.raw_os_error() {
// return positive error number on error
Some(error) => Ok(error),
None => {
throw_unsup_format!("the error {} couldn't be converted to a return value", e)
}
},
Some(Err(e)) =>
match e.raw_os_error() {
// return positive error number on error
Some(error) => Ok(error),
None => {
throw_unsup_format!(
"the error {} couldn't be converted to a return value",
e
)
}
},
}
}

Expand Down Expand Up @@ -1294,13 +1299,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
this.write_null(&this.deref_operand(result_op)?.into())?;
Ok(0)
}
Some(Err(e)) => match e.raw_os_error() {
// return positive error number on error
Some(error) => Ok(error),
None => {
throw_unsup_format!("the error {} couldn't be converted to a return value", e)
}
},
Some(Err(e)) =>
match e.raw_os_error() {
// return positive error number on error
Some(error) => Ok(error),
None => {
throw_unsup_format!(
"the error {} couldn't be converted to a return value",
e
)
}
},
}
}

Expand Down
9 changes: 5 additions & 4 deletions src/stacked_borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -699,10 +699,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
fn qualify(ty: ty::Ty<'_>, kind: RetagKind) -> Option<(RefKind, bool)> {
match ty.kind() {
// References are simple.
ty::Ref(_, _, Mutability::Mut) => Some((
RefKind::Unique { two_phase: kind == RetagKind::TwoPhase },
kind == RetagKind::FnEntry,
)),
ty::Ref(_, _, Mutability::Mut) =>
Some((
RefKind::Unique { two_phase: kind == RetagKind::TwoPhase },
kind == RetagKind::FnEntry,
)),
ty::Ref(_, _, Mutability::Not) =>
Some((RefKind::Shared, kind == RetagKind::FnEntry)),
// Raw pointers need to be enabled.
Expand Down
18 changes: 10 additions & 8 deletions src/vector_clock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,16 +186,18 @@ impl PartialOrd for VClock {
Ordering::Equal => Some(order),
// Right has at least 1 element > than the implicit 0,
// so the only valid values are Ordering::Less or None.
Ordering::Less => match order {
Ordering::Less | Ordering::Equal => Some(Ordering::Less),
Ordering::Greater => None,
},
Ordering::Less =>
match order {
Ordering::Less | Ordering::Equal => Some(Ordering::Less),
Ordering::Greater => None,
},
// Left has at least 1 element > than the implicit 0,
// so the only valid values are Ordering::Greater or None.
Ordering::Greater => match order {
Ordering::Greater | Ordering::Equal => Some(Ordering::Greater),
Ordering::Less => None,
},
Ordering::Greater =>
match order {
Ordering::Greater | Ordering::Equal => Some(Ordering::Greater),
Ordering::Less => None,
},
}
}

Expand Down

0 comments on commit a4a9a36

Please sign in to comment.