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
diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs
index 14e5d2a..0520c9a 100644
--- a/compiler/rustc_ast_passes/src/ast_validation.rs
+++ b/compiler/rustc_ast_passes/src/ast_validation.rs
@@ -1463,10 +1463,9 @@
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 @@
(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();
}
diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs
index f2b5c83..6478a10 100644
--- a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs
@@ -450,10 +450,10 @@
_ => 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");
}
diff --git a/compiler/rustc_codegen_ssa/src/mir/place.rs b/compiler/rustc_codegen_ssa/src/mir/place.rs
index b6a7bca..3185b95 100644
--- a/compiler/rustc_codegen_ssa/src/mir/place.rs
+++ b/compiler/rustc_codegen_ssa/src/mir/place.rs
@@ -462,7 +462,7 @@
}
};
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() {
diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs
index 4c84bd0..0c954ac 100644
--- a/compiler/rustc_const_eval/src/interpret/eval_context.rs
+++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs
@@ -520,13 +520,13 @@
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 @@
}
}
- 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!(
diff --git a/compiler/rustc_log/src/lib.rs b/compiler/rustc_log/src/lib.rs
index c152815..f2ec80b 100644
--- a/compiler/rustc_log/src/lib.rs
+++ b/compiler/rustc_log/src/lib.rs
@@ -69,13 +69,7 @@
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()
diff --git a/compiler/rustc_middle/src/mir/patch.rs b/compiler/rustc_middle/src/mir/patch.rs
index 1bc53d3..c1e1cfe 100644
--- a/compiler/rustc_middle/src/mir/patch.rs
+++ b/compiler/rustc_middle/src/mir/patch.rs
@@ -167,8 +167,7 @@
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;
diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs
index 569012e..ccbb518 100644
--- a/compiler/rustc_mir_build/src/build/expr/into.rs
+++ b/compiler/rustc_mir_build/src/build/expr/into.rs
@@ -435,11 +435,7 @@
}
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 } => {
diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs
index c15b3db..3774a39 100644
--- a/compiler/rustc_mir_build/src/build/matches/test.rs
+++ b/compiler/rustc_mir_build/src/build/matches/test.rs
@@ -264,7 +264,7 @@
);
} 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 @@
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 @@
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
diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
index a02d8c8..82e1ae9 100644
--- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
+++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
@@ -845,7 +845,7 @@
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 @@
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 @@
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 @@
.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());
diff --git a/compiler/rustc_typeck/src/check/method/suggest.rs b/compiler/rustc_typeck/src/check/method/suggest.rs
index f9c0ea8..4e54d55 100644
--- a/compiler/rustc_typeck/src/check/method/suggest.rs
+++ b/compiler/rustc_typeck/src/check/method/suggest.rs
@@ -492,7 +492,7 @@
Obligation {
cause: cause.clone(),
param_env: self.param_env,
- predicate: predicate.clone(),
+ predicate: *predicate,
recursion_depth: 0,
},
));
@@ -676,7 +676,7 @@
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 @@
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 @@
}
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 @@
{
derives.push((
self_name.clone(),
- self_span.clone(),
+ self_span,
parent_diagnostic_name.to_string(),
));
}
diff --git a/compiler/rustc_typeck/src/check/op.rs b/compiler/rustc_typeck/src/check/op.rs
index c99d9d8..637f645 100644
--- a/compiler/rustc_typeck/src/check/op.rs
+++ b/compiler/rustc_typeck/src/check/op.rs
@@ -705,7 +705,7 @@
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(
diff --git a/library/core/src/option.rs b/library/core/src/option.rs
index d074669..28ea45e 100644
--- a/library/core/src/option.rs
+++ b/library/core/src/option.rs
@@ -708,6 +708,26 @@
/// 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")]
diff --git a/library/core/src/result.rs b/library/core/src/result.rs
index 5e5f8a5..c4dc34f 100644
--- a/library/core/src/result.rs
+++ b/library/core/src/result.rs
@@ -1023,6 +1023,26 @@
/// 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")]
diff --git a/library/std/src/error.rs b/library/std/src/error.rs
index 438304e..c2d3061 100644
--- a/library/std/src/error.rs
+++ b/library/std/src/error.rs
@@ -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")]
