mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-22 23:04:33 +00:00
Auto merge of #97434 - matthiaskrgr:rollup-7j3y16l, r=matthiaskrgr
Rollup of 3 pull requests Successful merges: - #96033 (Add section on common message styles for Result::expect) - #97354 (Updates to browser-ui-test) - #97424 (clippy::complexity fixes) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This commit is contained in:
commit
490324f7b2
@ -1463,10 +1463,9 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
|
||||
if let GenericBound::Trait(ref poly, modify) = *bound {
|
||||
match (ctxt, modify) {
|
||||
(BoundKind::SuperTraits, TraitBoundModifier::Maybe) => {
|
||||
let mut err = self.err_handler().struct_span_err(
|
||||
poly.span,
|
||||
&format!("`?Trait` is not permitted in supertraits"),
|
||||
);
|
||||
let mut err = self
|
||||
.err_handler()
|
||||
.struct_span_err(poly.span, "`?Trait` is not permitted in supertraits");
|
||||
let path_str = pprust::path_to_string(&poly.trait_ref.path);
|
||||
err.note(&format!("traits are `?{}` by default", path_str));
|
||||
err.emit();
|
||||
@ -1474,7 +1473,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
|
||||
(BoundKind::TraitObject, TraitBoundModifier::Maybe) => {
|
||||
let mut err = self.err_handler().struct_span_err(
|
||||
poly.span,
|
||||
&format!("`?Trait` is not permitted in trait object types"),
|
||||
"`?Trait` is not permitted in trait object types",
|
||||
);
|
||||
err.emit();
|
||||
}
|
||||
|
@ -450,10 +450,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
|
||||
_ => None,
|
||||
};
|
||||
|
||||
if defined_hir.is_some() {
|
||||
if let Some(def_hir) = defined_hir {
|
||||
let upvars_map = self.infcx.tcx.upvars_mentioned(def_id).unwrap();
|
||||
let upvar_def_span = self.infcx.tcx.hir().span(defined_hir.unwrap());
|
||||
let upvar_span = upvars_map.get(&defined_hir.unwrap()).unwrap().span;
|
||||
let upvar_def_span = self.infcx.tcx.hir().span(def_hir);
|
||||
let upvar_span = upvars_map.get(&def_hir).unwrap().span;
|
||||
diag.span_label(upvar_def_span, "variable defined here");
|
||||
diag.span_label(upvar_span, "variable captured here");
|
||||
}
|
||||
|
@ -462,7 +462,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
|
||||
}
|
||||
};
|
||||
for elem in place_ref.projection[base..].iter() {
|
||||
cg_base = match elem.clone() {
|
||||
cg_base = match *elem {
|
||||
mir::ProjectionElem::Deref => {
|
||||
// a box with a non-zst allocator should not be directly dereferenced
|
||||
if cg_base.layout.ty.is_box() && !cg_base.layout.field(cx, 1).is_zst() {
|
||||
|
@ -520,13 +520,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
|
||||
frame
|
||||
.instance
|
||||
.try_subst_mir_and_normalize_erasing_regions(*self.tcx, self.param_env, value)
|
||||
.or_else(|e| {
|
||||
.map_err(|e| {
|
||||
self.tcx.sess.delay_span_bug(
|
||||
self.cur_span(),
|
||||
format!("failed to normalize {}", e.get_type_for_failure()).as_str(),
|
||||
);
|
||||
|
||||
Err(InterpError::InvalidProgram(InvalidProgramInfo::TooGeneric))
|
||||
InterpError::InvalidProgram(InvalidProgramInfo::TooGeneric)
|
||||
})
|
||||
}
|
||||
|
||||
@ -1009,11 +1009,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> std::fmt::Debug
|
||||
}
|
||||
}
|
||||
|
||||
write!(
|
||||
fmt,
|
||||
": {:?}",
|
||||
self.ecx.dump_allocs(allocs.into_iter().filter_map(|x| x).collect())
|
||||
)
|
||||
write!(fmt, ": {:?}", self.ecx.dump_allocs(allocs.into_iter().flatten().collect()))
|
||||
}
|
||||
Place::Ptr(mplace) => match mplace.ptr.provenance.and_then(Provenance::get_alloc_id) {
|
||||
Some(alloc_id) => write!(
|
||||
|
@ -69,13 +69,7 @@ pub fn init_env_logger(env: &str) -> Result<(), Error> {
|
||||
|
||||
let verbose_entry_exit = match env::var_os(String::from(env) + "_ENTRY_EXIT") {
|
||||
None => false,
|
||||
Some(v) => {
|
||||
if &v == "0" {
|
||||
false
|
||||
} else {
|
||||
true
|
||||
}
|
||||
}
|
||||
Some(v) => &v != "0",
|
||||
};
|
||||
|
||||
let layer = tracing_tree::HierarchicalLayer::default()
|
||||
|
@ -167,8 +167,7 @@ impl<'tcx> MirPatch<'tcx> {
|
||||
if loc.statement_index > body[loc.block].statements.len() {
|
||||
let term = body[loc.block].terminator();
|
||||
for i in term.successors() {
|
||||
stmts_and_targets
|
||||
.push((Statement { source_info, kind: stmt.clone() }, i.clone()));
|
||||
stmts_and_targets.push((Statement { source_info, kind: stmt.clone() }, i));
|
||||
}
|
||||
delta += 1;
|
||||
continue;
|
||||
|
@ -435,11 +435,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
|
||||
}
|
||||
thir::InlineAsmOperand::SymFn { value, span } => {
|
||||
mir::InlineAsmOperand::SymFn {
|
||||
value: Box::new(Constant {
|
||||
span,
|
||||
user_ty: None,
|
||||
literal: value.into(),
|
||||
}),
|
||||
value: Box::new(Constant { span, user_ty: None, literal: value }),
|
||||
}
|
||||
}
|
||||
thir::InlineAsmOperand::SymStatic { def_id } => {
|
||||
|
@ -264,7 +264,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
|
||||
);
|
||||
} else if let [success, fail] = *make_target_blocks(self) {
|
||||
assert_eq!(value.ty(), ty);
|
||||
let expect = self.literal_operand(test.span, value.into());
|
||||
let expect = self.literal_operand(test.span, value);
|
||||
let val = Operand::Copy(place);
|
||||
self.compare(block, success, fail, source_info, BinOp::Eq, expect, val);
|
||||
} else {
|
||||
@ -277,8 +277,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
|
||||
let target_blocks = make_target_blocks(self);
|
||||
|
||||
// Test `val` by computing `lo <= val && val <= hi`, using primitive comparisons.
|
||||
let lo = self.literal_operand(test.span, lo.into());
|
||||
let hi = self.literal_operand(test.span, hi.into());
|
||||
let lo = self.literal_operand(test.span, lo);
|
||||
let hi = self.literal_operand(test.span, hi);
|
||||
let val = Operand::Copy(place);
|
||||
|
||||
let [success, fail] = *target_blocks else {
|
||||
@ -370,7 +370,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
|
||||
place: Place<'tcx>,
|
||||
mut ty: Ty<'tcx>,
|
||||
) {
|
||||
let mut expect = self.literal_operand(source_info.span, value.into());
|
||||
let mut expect = self.literal_operand(source_info.span, value);
|
||||
let mut val = Operand::Copy(place);
|
||||
|
||||
// If we're using `b"..."` as a pattern, we need to insert an
|
||||
|
@ -845,7 +845,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|
||||
let first_provided_ty = if let Some((ty, _)) = final_arg_types[input_idx] {
|
||||
format!(",found `{}`", ty)
|
||||
} else {
|
||||
"".into()
|
||||
String::new()
|
||||
};
|
||||
labels.push((
|
||||
first_span,
|
||||
@ -857,7 +857,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|
||||
if let Some((ty, _)) = final_arg_types[other_input_idx] {
|
||||
format!(",found `{}`", ty)
|
||||
} else {
|
||||
"".into()
|
||||
String::new()
|
||||
};
|
||||
labels.push((
|
||||
second_span,
|
||||
@ -875,7 +875,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|
||||
let provided_ty = if let Some((ty, _)) = final_arg_types[dst_arg] {
|
||||
format!(",found `{}`", ty)
|
||||
} else {
|
||||
"".into()
|
||||
String::new()
|
||||
};
|
||||
labels.push((
|
||||
provided_args[dst_arg].span,
|
||||
@ -1744,8 +1744,7 @@ fn label_fn_like<'tcx>(
|
||||
.get_if_local(def_id)
|
||||
.and_then(|node| node.body_id())
|
||||
.into_iter()
|
||||
.map(|id| tcx.hir().body(id).params)
|
||||
.flatten();
|
||||
.flat_map(|id| tcx.hir().body(id).params);
|
||||
|
||||
for param in params {
|
||||
spans.push_span_label(param.span, String::new());
|
||||
|
@ -492,7 +492,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|
||||
Obligation {
|
||||
cause: cause.clone(),
|
||||
param_env: self.param_env,
|
||||
predicate: predicate.clone(),
|
||||
predicate: *predicate,
|
||||
recursion_depth: 0,
|
||||
},
|
||||
));
|
||||
@ -676,7 +676,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|
||||
let span = self_ty.span.ctxt().outer_expn_data().call_site;
|
||||
let mut spans: MultiSpan = span.into();
|
||||
spans.push_span_label(span, derive_msg.to_string());
|
||||
let entry = spanned_predicates.entry(spans.into());
|
||||
let entry = spanned_predicates.entry(spans);
|
||||
entry.or_insert_with(|| (path, tr_self_ty, Vec::new())).2.push(p);
|
||||
}
|
||||
|
||||
@ -704,7 +704,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|
||||
ident.span.into()
|
||||
};
|
||||
spans.push_span_label(ident.span, "in this trait".to_string());
|
||||
let entry = spanned_predicates.entry(spans.into());
|
||||
let entry = spanned_predicates.entry(spans);
|
||||
entry.or_insert_with(|| (path, tr_self_ty, Vec::new())).2.push(p);
|
||||
}
|
||||
|
||||
@ -748,7 +748,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|
||||
}
|
||||
spans.push_span_label(self_ty.span, String::new());
|
||||
|
||||
let entry = spanned_predicates.entry(spans.into());
|
||||
let entry = spanned_predicates.entry(spans);
|
||||
entry.or_insert_with(|| (path, tr_self_ty, Vec::new())).2.push(p);
|
||||
}
|
||||
_ => {}
|
||||
@ -1460,7 +1460,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|
||||
{
|
||||
derives.push((
|
||||
self_name.clone(),
|
||||
self_span.clone(),
|
||||
self_span,
|
||||
parent_diagnostic_name.to_string(),
|
||||
));
|
||||
}
|
||||
|
@ -705,7 +705,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|
||||
let predicates = errors
|
||||
.iter()
|
||||
.filter_map(|error| {
|
||||
error.obligation.predicate.clone().to_opt_poly_trait_pred()
|
||||
error.obligation.predicate.to_opt_poly_trait_pred()
|
||||
});
|
||||
for pred in predicates {
|
||||
self.infcx.suggest_restricting_param_bound(
|
||||
|
@ -708,6 +708,26 @@ impl<T> Option<T> {
|
||||
/// let x: Option<&str> = None;
|
||||
/// x.expect("fruits are healthy"); // panics with `fruits are healthy`
|
||||
/// ```
|
||||
///
|
||||
/// # Recommended Message Style
|
||||
///
|
||||
/// We recommend that `expect` messages are used to describe the reason you
|
||||
/// _expect_ the `Option` should be `Some`.
|
||||
///
|
||||
/// ```should_panic
|
||||
/// # let slice: &[u8] = &[];
|
||||
/// let item = slice.get(0)
|
||||
/// .expect("slice should not be empty");
|
||||
/// ```
|
||||
///
|
||||
/// **Hint**: If you're having trouble remembering how to phrase expect
|
||||
/// error messages remember to focus on the word "should" as in "env
|
||||
/// variable should be set by blah" or "the given binary should be available
|
||||
/// and executable by the current user".
|
||||
///
|
||||
/// For more detail on expect message styles and the reasoning behind our
|
||||
/// recommendation please refer to the section on ["Common Message
|
||||
/// Styles"](../../std/error/index.html#common-message-styles) in the [`std::error`](../../std/error/index.html) module docs.
|
||||
#[inline]
|
||||
#[track_caller]
|
||||
#[stable(feature = "rust1", since = "1.0.0")]
|
||||
|
@ -1023,6 +1023,26 @@ impl<T, E> Result<T, E> {
|
||||
/// let x: Result<u32, &str> = Err("emergency failure");
|
||||
/// x.expect("Testing expect"); // panics with `Testing expect: emergency failure`
|
||||
/// ```
|
||||
///
|
||||
/// # Recommended Message Style
|
||||
///
|
||||
/// We recommend that `expect` messages are used to describe the reason you
|
||||
/// _expect_ the `Result` should be `Ok`.
|
||||
///
|
||||
/// ```should_panic
|
||||
/// let path = std::env::var("IMPORTANT_PATH")
|
||||
/// .expect("env variable `IMPORTANT_PATH` should be set by `wrapper_script.sh`");
|
||||
/// ```
|
||||
///
|
||||
/// **Hint**: If you're having trouble remembering how to phrase expect
|
||||
/// error messages remember to focus on the word "should" as in "env
|
||||
/// variable should be set by blah" or "the given binary should be available
|
||||
/// and executable by the current user".
|
||||
///
|
||||
/// For more detail on expect message styles and the reasoning behind our recommendation please
|
||||
/// refer to the section on ["Common Message
|
||||
/// Styles"](../../std/error/index.html#common-message-styles) in the
|
||||
/// [`std::error`](../../std/error/index.html) module docs.
|
||||
#[inline]
|
||||
#[track_caller]
|
||||
#[stable(feature = "result_expect", since = "1.4.0")]
|
||||
|
@ -1,4 +1,140 @@
|
||||
//! Traits for working with Errors.
|
||||
//! Interfaces for working with Errors.
|
||||
//!
|
||||
//! # Error Handling In Rust
|
||||
//!
|
||||
//! The Rust language provides two complementary systems for constructing /
|
||||
//! representing, reporting, propagating, reacting to, and discarding errors.
|
||||
//! These responsibilities are collectively known as "error handling." The
|
||||
//! components of the first system, the panic runtime and interfaces, are most
|
||||
//! commonly used to represent bugs that have been detected in your program. The
|
||||
//! components of the second system, `Result`, the error traits, and user
|
||||
//! defined types, are used to represent anticipated runtime failure modes of
|
||||
//! your program.
|
||||
//!
|
||||
//! ## The Panic Interfaces
|
||||
//!
|
||||
//! The following are the primary interfaces of the panic system and the
|
||||
//! responsibilities they cover:
|
||||
//!
|
||||
//! * [`panic!`] and [`panic_any`] (Constructing, Propagated automatically)
|
||||
//! * [`PanicInfo`] (Reporting)
|
||||
//! * [`set_hook`], [`take_hook`], and [`#[panic_handler]`][panic-handler] (Reporting)
|
||||
//! * [`catch_unwind`] and [`resume_unwind`] (Discarding, Propagating)
|
||||
//!
|
||||
//! The following are the primary interfaces of the error system and the
|
||||
//! responsibilities they cover:
|
||||
//!
|
||||
//! * [`Result`] (Propagating, Reacting)
|
||||
//! * The [`Error`] trait (Reporting)
|
||||
//! * User defined types (Constructing / Representing)
|
||||
//! * [`match`] and [`downcast`] (Reacting)
|
||||
//! * The question mark operator ([`?`]) (Propagating)
|
||||
//! * The partially stable [`Try`] traits (Propagating, Constructing)
|
||||
//! * [`Termination`] (Reporting)
|
||||
//!
|
||||
//! ## Converting Errors into Panics
|
||||
//!
|
||||
//! The panic and error systems are not entirely distinct. Often times errors
|
||||
//! that are anticipated runtime failures in an API might instead represent bugs
|
||||
//! to a caller. For these situations the standard library provides APIs for
|
||||
//! constructing panics with an `Error` as it's source.
|
||||
//!
|
||||
//! * [`Result::unwrap`]
|
||||
//! * [`Result::expect`]
|
||||
//!
|
||||
//! These functions are equivalent, they either return the inner value if the
|
||||
//! `Result` is `Ok` or panic if the `Result` is `Err` printing the inner error
|
||||
//! as the source. The only difference between them is that with `expect` you
|
||||
//! provide a panic error message to be printed alongside the source, whereas
|
||||
//! `unwrap` has a default message indicating only that you unwraped an `Err`.
|
||||
//!
|
||||
//! Of the two, `expect` is generally preferred since its `msg` field allows you
|
||||
//! to convey your intent and assumptions which makes tracking down the source
|
||||
//! of a panic easier. `unwrap` on the other hand can still be a good fit in
|
||||
//! situations where you can trivially show that a piece of code will never
|
||||
//! panick, such as `"127.0.0.1".parse::<std::net::IpAddr>().unwrap()` or early
|
||||
//! prototyping.
|
||||
//!
|
||||
//! # Common Message Styles
|
||||
//!
|
||||
//! There are two common styles for how people word `expect` messages. Using
|
||||
//! the message to present information to users encountering a panic
|
||||
//! ("expect as error message") or using the message to present information
|
||||
//! to developers debugging the panic ("expect as precondition").
|
||||
//!
|
||||
//! In the former case the expect message is used to describe the error that
|
||||
//! has occurred which is considered a bug. Consider the following example:
|
||||
//!
|
||||
//! ```should_panic
|
||||
//! // Read environment variable, panic if it is not present
|
||||
//! let path = std::env::var("IMPORTANT_PATH").unwrap();
|
||||
//! ```
|
||||
//!
|
||||
//! In the "expect as error message" style we would use expect to describe
|
||||
//! that the environment variable was not set when it should have been:
|
||||
//!
|
||||
//! ```should_panic
|
||||
//! let path = std::env::var("IMPORTANT_PATH")
|
||||
//! .expect("env variable `IMPORTANT_PATH` is not set");
|
||||
//! ```
|
||||
//!
|
||||
//! In the "expect as precondition" style, we would instead describe the
|
||||
//! reason we _expect_ the `Result` should be `Ok`. With this style we would
|
||||
//! prefer to write:
|
||||
//!
|
||||
//! ```should_panic
|
||||
//! let path = std::env::var("IMPORTANT_PATH")
|
||||
//! .expect("env variable `IMPORTANT_PATH` should be set by `wrapper_script.sh`");
|
||||
//! ```
|
||||
//!
|
||||
//! The "expect as error message" style does not work as well with the
|
||||
//! default output of the std panic hooks, and often ends up repeating
|
||||
//! information that is already communicated by the source error being
|
||||
//! unwrapped:
|
||||
//!
|
||||
//! ```text
|
||||
//! thread 'main' panicked at 'env variable `IMPORTANT_PATH` is not set: NotPresent', src/main.rs:4:6
|
||||
//! ```
|
||||
//!
|
||||
//! In this example we end up mentioning that an env variable is not set,
|
||||
//! followed by our source message that says the env is not present, the
|
||||
//! only additional information we're communicating is the name of the
|
||||
//! environment variable being checked.
|
||||
//!
|
||||
//! The "expect as precondition" style instead focuses on source code
|
||||
//! readability, making it easier to understand what must have gone wrong in
|
||||
//! situations where panics are being used to represent bugs exclusively.
|
||||
//! Also, by framing our expect in terms of what "SHOULD" have happened to
|
||||
//! prevent the source error, we end up introducing new information that is
|
||||
//! independent from our source error.
|
||||
//!
|
||||
//! ```text
|
||||
//! thread 'main' panicked at 'env variable `IMPORTANT_PATH` should be set by `wrapper_script.sh`: NotPresent', src/main.rs:4:6
|
||||
//! ```
|
||||
//!
|
||||
//! In this example we are communicating not only the name of the
|
||||
//! environment variable that should have been set, but also an explanation
|
||||
//! for why it should have been set, and we let the source error display as
|
||||
//! a clear contradiction to our expectation.
|
||||
//!
|
||||
//! **Hint**: If you're having trouble remembering how to phrase
|
||||
//! expect-as-precondition style error messages remember to focus on the word
|
||||
//! "should" as in "env variable should be set by blah" or "the given binary
|
||||
//! should be available and executable by the current user".
|
||||
//!
|
||||
//! [`panic_any`]: crate::panic::panic_any
|
||||
//! [`PanicInfo`]: crate::panic::PanicInfo
|
||||
//! [`catch_unwind`]: crate::panic::catch_unwind
|
||||
//! [`resume_unwind`]: crate::panic::resume_unwind
|
||||
//! [`downcast`]: crate::error::Error
|
||||
//! [`Termination`]: crate::process::Termination
|
||||
//! [`Try`]: crate::ops::Try
|
||||
//! [panic hook]: crate::panic::set_hook
|
||||
//! [`set_hook`]: crate::panic::set_hook
|
||||
//! [`take_hook`]: crate::panic::take_hook
|
||||
//! [panic-handler]: <https://doc.rust-lang.org/nomicon/panic-handler.html>
|
||||
//! [`match`]: ../../std/keyword.match.html
|
||||
//! [`?`]: ../../std/result/index.html#the-question-mark-operator-
|
||||
|
||||
#![stable(feature = "rust1", since = "1.0.0")]
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user