From dfb37cb088ce116829e3d268a13c1e6cc3601295 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 13 Apr 2022 19:08:10 -0700 Subject: [PATCH 01/15] Add section on common message styles for Result::expect --- library/core/src/result.rs | 56 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/library/core/src/result.rs b/library/core/src/result.rs index b2b132300a299..564e5e34d6dce 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1023,6 +1023,62 @@ impl Result { /// let x: Result = Err("emergency failure"); /// x.expect("Testing expect"); // panics with `Testing expect: emergency failure` /// ``` + /// + /// # 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: + /// + /// ``` + /// // 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: + /// + /// ``` + /// let path = std::env::var("IMPORTANT_PATH") + /// .expect("env variable `IMPORTANT_PATH` is not set"); + /// ``` + /// + /// In the latter style, we would instead describe the reason we _expect_ the `Result` will + /// always be `Ok`. With this style we would instead write: + /// + /// ``` + /// let path = std::env::var("IMPORTANT_PATH") + /// .expect("env variable `IMPORTANT_PATH` is always be set by `wrapper_script.sh`"); + /// ``` + /// + /// The "expect as error message" style has the advantage of giving a more user friendly error + /// message, and is more consistent with the default output of the [panic hook] provided by + /// `std`. + /// + /// ``` + /// thread 'expect_as_error_message' panicked at 'env variable `IMPORTANT_PATH` is not set: NotPresent', src/lib.rs:4:10 + /// ``` + /// + /// 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. But this extra information often looks confusing when presented + /// directly to users with the default `std` panic hook's report format: + /// + /// ``` + /// thread 'expect_as_precondition' panicked at 'env variable `IMPORTANT_PATH` is always set by `wrapper_script.sh`: NotPresent', src/lib.rs:4:10 + /// ``` + /// + /// This style works best when paired with a custom [panic hook] like the one provided by the + /// CLI working group library, [`human-panic`], which redirects panic messages to crash report + /// files while showing users a more "Oops, something went wrong!" message with a suggestion to + /// send the crash report file back to the developers. + /// + /// [panic hook]: https://doc.rust-lang.org/stable/std/panic/fn.set_hook.html + /// [`human-panic`]: https://docs.rs/human-panic #[inline] #[track_caller] #[stable(feature = "result_expect", since = "1.4.0")] From 3d951b3d214f5d913153e2e695d7e6ba907bba62 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Thu, 14 Apr 2022 11:40:27 -0700 Subject: [PATCH 02/15] add necessary text attribute to code block of panic output --- library/core/src/result.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/result.rs b/library/core/src/result.rs index 564e5e34d6dce..6189000cd36c1 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1059,7 +1059,7 @@ impl Result { /// message, and is more consistent with the default output of the [panic hook] provided by /// `std`. /// - /// ``` + /// ```text /// thread 'expect_as_error_message' panicked at 'env variable `IMPORTANT_PATH` is not set: NotPresent', src/lib.rs:4:10 /// ``` /// @@ -1068,7 +1068,7 @@ impl Result { /// represent bugs exclusively. But this extra information often looks confusing when presented /// directly to users with the default `std` panic hook's report format: /// - /// ``` + /// ```text /// thread 'expect_as_precondition' panicked at 'env variable `IMPORTANT_PATH` is always set by `wrapper_script.sh`: NotPresent', src/lib.rs:4:10 /// ``` /// From 80c362b92bda7776fc88e31c479455890b4a3688 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Thu, 14 Apr 2022 13:22:24 -0700 Subject: [PATCH 03/15] add should_panic annotations --- library/core/src/result.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/core/src/result.rs b/library/core/src/result.rs index 6189000cd36c1..8a68e3fe6d6b4 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1034,7 +1034,7 @@ impl Result { /// 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(); /// ``` @@ -1042,7 +1042,7 @@ impl Result { /// 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"); /// ``` @@ -1050,7 +1050,7 @@ impl Result { /// In the latter style, we would instead describe the reason we _expect_ the `Result` will /// always be `Ok`. With this style we would instead write: /// - /// ``` + /// ```should_panic /// let path = std::env::var("IMPORTANT_PATH") /// .expect("env variable `IMPORTANT_PATH` is always be set by `wrapper_script.sh`"); /// ``` From 5d98acb19cd9a9d77f6e931a26cdb846723b65f2 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Fri, 15 Apr 2022 10:24:34 -0700 Subject: [PATCH 04/15] update docs for option to crossreference to the result docs --- library/core/src/option.rs | 3 +++ library/core/src/result.rs | 14 +++++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index 91e4708f6a609..f280c321c9c71 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -708,6 +708,9 @@ impl Option { /// let x: Option<&str> = None; /// x.expect("fruits are healthy"); // panics with `fruits are healthy` /// ``` + /// + /// **Note**: Please refer to the documentation on [`Result::expect`] for further information + /// on common message styles. #[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 8a68e3fe6d6b4..7cbe2213b22b2 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1047,8 +1047,8 @@ impl Result { /// .expect("env variable `IMPORTANT_PATH` is not set"); /// ``` /// - /// In the latter style, we would instead describe the reason we _expect_ the `Result` will - /// always be `Ok`. With this style we would instead write: + /// In the "expect as precondition" style, we would instead describe the reason we _expect_ the + /// `Result` will always be `Ok`. With this style we would prefer to write: /// /// ```should_panic /// let path = std::env::var("IMPORTANT_PATH") @@ -1060,7 +1060,7 @@ impl Result { /// `std`. /// /// ```text - /// thread 'expect_as_error_message' panicked at 'env variable `IMPORTANT_PATH` is not set: NotPresent', src/lib.rs:4:10 + /// thread 'main' panicked at 'env variable `IMPORTANT_PATH` is not set: NotPresent', src/main.rs:4:6 /// ``` /// /// The "expect as precondition" style instead focuses on source code readability, making it @@ -1069,13 +1069,13 @@ impl Result { /// directly to users with the default `std` panic hook's report format: /// /// ```text - /// thread 'expect_as_precondition' panicked at 'env variable `IMPORTANT_PATH` is always set by `wrapper_script.sh`: NotPresent', src/lib.rs:4:10 + /// thread 'main' panicked at 'env variable `IMPORTANT_PATH` is always be set by `wrapper_script.sh`: NotPresent', src/main.rs:4:6 /// ``` /// /// This style works best when paired with a custom [panic hook] like the one provided by the - /// CLI working group library, [`human-panic`], which redirects panic messages to crash report - /// files while showing users a more "Oops, something went wrong!" message with a suggestion to - /// send the crash report file back to the developers. + /// CLI working group library, [`human-panic`], which dumps the panic messages to a crash + /// report file while showing users a more friendly "Oops, something went wrong!" message with + /// a suggestion to send the crash report file back to the developers. /// /// [panic hook]: https://doc.rust-lang.org/stable/std/panic/fn.set_hook.html /// [`human-panic`]: https://docs.rs/human-panic From eca7dd20d595c94cc43d59aaf2067ffcc0b86483 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Mon, 18 Apr 2022 16:31:13 -0700 Subject: [PATCH 05/15] Update library/core/src/result.rs Co-authored-by: Emil Thorenfeldt --- library/core/src/result.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/result.rs b/library/core/src/result.rs index 7cbe2213b22b2..47e718b6a423d 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1052,7 +1052,7 @@ impl Result { /// /// ```should_panic /// let path = std::env::var("IMPORTANT_PATH") - /// .expect("env variable `IMPORTANT_PATH` is always be set by `wrapper_script.sh`"); + /// .expect("env variable `IMPORTANT_PATH` is always set by `wrapper_script.sh`"); /// ``` /// /// The "expect as error message" style has the advantage of giving a more user friendly error From 00a315c515654698e70ca5bb1c20eae8a4f67a6a Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Mon, 18 Apr 2022 16:31:21 -0700 Subject: [PATCH 06/15] Update library/core/src/result.rs Co-authored-by: Emil Thorenfeldt --- library/core/src/result.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/result.rs b/library/core/src/result.rs index 47e718b6a423d..0409775857a8c 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1069,7 +1069,7 @@ impl Result { /// directly to users with the default `std` panic hook's report format: /// /// ```text - /// thread 'main' panicked at 'env variable `IMPORTANT_PATH` is always be set by `wrapper_script.sh`: NotPresent', src/main.rs:4:6 + /// thread 'main' panicked at 'env variable `IMPORTANT_PATH` is always set by `wrapper_script.sh`: NotPresent', src/main.rs:4:6 /// ``` /// /// This style works best when paired with a custom [panic hook] like the one provided by the From 72898acdba32e2ccc3fd1bc6deced97a8e1ad001 Mon Sep 17 00:00:00 2001 From: Jane Losare-Lusby Date: Sat, 30 Apr 2022 03:04:31 +0000 Subject: [PATCH 07/15] spicy --- library/core/src/result.rs | 68 ++++++++++++++++++++++++-------------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/library/core/src/result.rs b/library/core/src/result.rs index 0409775857a8c..99d39b7011c43 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1026,56 +1026,74 @@ impl Result { /// /// # 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"). + /// 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: + /// 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: + /// 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` will always be `Ok`. With this style we would prefer to write: + /// 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` is always set by `wrapper_script.sh`"); + /// .expect("env variable `IMPORTANT_PATH` should be set by `wrapper_script.sh`"); /// ``` /// - /// The "expect as error message" style has the advantage of giving a more user friendly error - /// message, and is more consistent with the default output of the [panic hook] provided by - /// `std`. + /// 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 /// ``` /// - /// 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. But this extra information often looks confusing when presented - /// directly to users with the default `std` panic hook's report format: + /// 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. /// - /// ```text - /// thread 'main' panicked at 'env variable `IMPORTANT_PATH` is always set by `wrapper_script.sh`: NotPresent', src/main.rs:4:6 - /// ``` + /// 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. /// - /// This style works best when paired with a custom [panic hook] like the one provided by the - /// CLI working group library, [`human-panic`], which dumps the panic messages to a crash - /// report file while showing users a more friendly "Oops, something went wrong!" message with - /// a suggestion to send the crash report file back to the developers. + /// ```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. + /// + /// For programs where panics may be user facing, either style works best + /// when paired with a custom [panic hook] like the one provided by the CLI + /// working group library, [`human-panic`]. This panic hook dumps the panic + /// messages to a crash report file while showing users a more friendly + /// "Oops, something went wrong!" message with a suggestion to send the + /// crash report file back to the developers. Panic messages should be used + /// to represent bugs, and the information provided back is context intended + /// for the developer, not the user. /// /// [panic hook]: https://doc.rust-lang.org/stable/std/panic/fn.set_hook.html /// [`human-panic`]: https://docs.rs/human-panic From 7b5dce900d6e5e8f8823720a3746441928eecb23 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Fri, 6 May 2022 15:03:25 -0700 Subject: [PATCH 08/15] This is a pretty good start if you ask me --- library/core/src/result.rs | 72 ++------------------ library/std/src/error.rs | 133 ++++++++++++++++++++++++++++++++++++- 2 files changed, 138 insertions(+), 67 deletions(-) diff --git a/library/core/src/result.rs b/library/core/src/result.rs index 99d39b7011c43..342acea29dc5b 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1024,79 +1024,19 @@ impl Result { /// x.expect("Testing expect"); // panics with `Testing expect: emergency failure` /// ``` /// - /// # Common Message Styles + /// # Recommended Message Style /// - /// 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: + /// 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`"); /// ``` /// - /// 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. - /// - /// For programs where panics may be user facing, either style works best - /// when paired with a custom [panic hook] like the one provided by the CLI - /// working group library, [`human-panic`]. This panic hook dumps the panic - /// messages to a crash report file while showing users a more friendly - /// "Oops, something went wrong!" message with a suggestion to send the - /// crash report file back to the developers. Panic messages should be used - /// to represent bugs, and the information provided back is context intended - /// for the developer, not the user. - /// - /// [panic hook]: https://doc.rust-lang.org/stable/std/panic/fn.set_hook.html - /// [`human-panic`]: https://docs.rs/human-panic + /// For more detail on expect message styles and the reasoning behind our + /// recommendation please refer to the section on ["Common Message + /// Styles"]() in the [`std::error`]() 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 4fb94908c80fd..84089075f24a7 100644 --- a/library/std/src/error.rs +++ b/library/std/src/error.rs @@ -1,4 +1,135 @@ -//! 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]`] (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 propagation 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` +//! +//! TODO: how do I bridge these two sections? +//! +//! * unwrap is used in prototyping +//! * expect is used in !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. +//! +//! For programs where panics may be user facing, either style works best +//! when paired with a custom [panic hook] like the one provided by the CLI +//! working group library, [`human-panic`]. This panic hook dumps the panic +//! messages to a crash report file while showing users a more friendly +//! "Oops, something went wrong!" message with a suggestion to send the +//! crash report file back to the developers. Panic messages should be used +//! to represent bugs, and the information provided back is context intended +//! for the developer, not the user. +//! +//! [panic hook]: crate::panic::set_hook +//! [`set_hook`]: crate::panic::set_hook +//! [`take_hook`]: crate::panic::take_hook +//! [`PanicInfo`]: crate::panic::PanicInfo +//! [`panic_any`]: crate::panic::panic_any +//! [`#[panic_handler]`]: https://doc.rust-lang.org/nomicon/panic-handler.html +//! [`catch_unwind`]: crate::panic::catch_unwind +//! [`resume_unwind`]: crate::panic::resume_unwind +//! [`Termination`]: crate::process::Termination +//! [`Try`]: crate::ops::Try +//! [`downcast`]: crate::error::Error +//! [`human-panic`]: https://docs.rs/human-panic #![stable(feature = "rust1", since = "1.0.0")] From b7838e9b1d0187152df275a55ff991ad823ad415 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 24 May 2022 15:41:24 +0200 Subject: [PATCH 09/15] Update browser-ui-test version to 0.9.5 --- .../docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version b/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version index b3ec1638fda74..03834411d1529 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version +++ b/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version @@ -1 +1 @@ -0.9.3 \ No newline at end of file +0.9.5 \ No newline at end of file From d4d1e23c6975357ef66035e6a3d8930f9fdaee4c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 24 May 2022 13:23:58 +0200 Subject: [PATCH 10/15] Allow to pass more arguments to tester.js --- src/tools/rustdoc-gui/tester.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/tools/rustdoc-gui/tester.js b/src/tools/rustdoc-gui/tester.js index 8532410a1bf3a..4599e12de5f33 100644 --- a/src/tools/rustdoc-gui/tester.js +++ b/src/tools/rustdoc-gui/tester.js @@ -19,6 +19,7 @@ function showHelp() { console.log(" --help : show this message then quit"); console.log(" --tests-folder [PATH] : location of the .GOML tests folder"); console.log(" --jobs [NUMBER] : number of threads to run tests on"); + console.log(" --executable-path [PATH] : path of the browser's executable to be used"); } function isNumeric(s) { @@ -34,6 +35,8 @@ function parseOptions(args) { "show_text": false, "no_headless": false, "jobs": -1, + "executable_path": null, + "no_sandbox": false, }; var correspondances = { "--doc-folder": "doc_folder", @@ -41,13 +44,16 @@ function parseOptions(args) { "--debug": "debug", "--show-text": "show_text", "--no-headless": "no_headless", + "--executable-path": "executable_path", + "--no-sandbox": "no_sandbox", }; for (var i = 0; i < args.length; ++i) { if (args[i] === "--doc-folder" || args[i] === "--tests-folder" || args[i] === "--file" - || args[i] === "--jobs") { + || args[i] === "--jobs" + || args[i] === "--executable-path") { i += 1; if (i >= args.length) { console.log("Missing argument after `" + args[i - 1] + "` option."); @@ -68,6 +74,9 @@ function parseOptions(args) { } else if (args[i] === "--help") { showHelp(); process.exit(0); + } else if (args[i] === "--no-sandbox") { + console.log("`--no-sandbox` is being used. Be very careful!"); + opts[correspondances[args[i]]] = true; } else if (correspondances[args[i]]) { opts[correspondances[args[i]]] = true; } else { @@ -147,10 +156,17 @@ async function main(argv) { if (opts["show_text"]) { args.push("--show-text"); } + if (opts["no_sandbox"]) { + args.push("--no-sandbox"); + } if (opts["no_headless"]) { args.push("--no-headless"); headless = false; } + if (opts["executable_path"] !== null) { + args.push("--executable-path"); + args.push(opts["executable_path"]); + } options.parseArguments(args); } catch (error) { console.error(`invalid argument: ${error}`); From 9a9dafcca462334e526c0b2bb1ac102216c61c6a Mon Sep 17 00:00:00 2001 From: Jane Losare-Lusby Date: Tue, 24 May 2022 22:51:54 +0000 Subject: [PATCH 11/15] explained unwrap vs expect --- library/std/src/error.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/library/std/src/error.rs b/library/std/src/error.rs index 84089075f24a7..c5782d4520afa 100644 --- a/library/std/src/error.rs +++ b/library/std/src/error.rs @@ -42,10 +42,18 @@ //! * `Result::unwrap` //! * `Result::expect` //! -//! TODO: how do I bridge these two sections? +//! 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`. //! -//! * unwrap is used in prototyping -//! * expect is used in !prototyping (????) +//! 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::().unwrap()` or early +//! prototyping. //! //! # Common Message Styles //! @@ -109,14 +117,10 @@ //! for why it should have been set, and we let the source error display as //! a clear contradiction to our expectation. //! -//! For programs where panics may be user facing, either style works best -//! when paired with a custom [panic hook] like the one provided by the CLI -//! working group library, [`human-panic`]. This panic hook dumps the panic -//! messages to a crash report file while showing users a more friendly -//! "Oops, something went wrong!" message with a suggestion to send the -//! crash report file back to the developers. Panic messages should be used -//! to represent bugs, and the information provided back is context intended -//! for the developer, not the user. +//! **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 hook]: crate::panic::set_hook //! [`set_hook`]: crate::panic::set_hook @@ -129,7 +133,6 @@ //! [`Termination`]: crate::process::Termination //! [`Try`]: crate::ops::Try //! [`downcast`]: crate::error::Error -//! [`human-panic`]: https://docs.rs/human-panic #![stable(feature = "rust1", since = "1.0.0")] From b6b621ec8541133f3b18669c72e00a6f7b991069 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 25 May 2022 10:46:56 -0700 Subject: [PATCH 12/15] fix links --- library/std/src/error.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/library/std/src/error.rs b/library/std/src/error.rs index c5782d4520afa..41a632cdabf9d 100644 --- a/library/std/src/error.rs +++ b/library/std/src/error.rs @@ -18,7 +18,7 @@ //! //! * [`panic!`] and [`panic_any`] (Constructing, Propagated automatically) //! * [`PanicInfo`] (Reporting) -//! * [`set_hook`], [`take_hook`], and [`#[panic_handler]`] (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 @@ -27,8 +27,8 @@ //! * [`Result`] (Propagating, Reacting) //! * The [`Error`] trait (Reporting) //! * User defined types (Constructing / Representing) -//! * `match` and [`downcast`] (Reacting) -//! * The propagation operator (`?`) (Propagating) +//! * [`match`] and [`downcast`] (Reacting) +//! * The question mark operator ([`?`]) (Propagating) //! * The partially stable [`Try`] traits (Propagating, Constructing) //! * [`Termination`] (Reporting) //! @@ -39,8 +39,8 @@ //! 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` +//! * [`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 @@ -122,17 +122,19 @@ //! "should" as in "env variable should be set by blah" or "the given binary //! should be available and executable by the current user". //! -//! [panic hook]: crate::panic::set_hook -//! [`set_hook`]: crate::panic::set_hook -//! [`take_hook`]: crate::panic::take_hook -//! [`PanicInfo`]: crate::panic::PanicInfo //! [`panic_any`]: crate::panic::panic_any -//! [`#[panic_handler]`]: https://doc.rust-lang.org/nomicon/panic-handler.html +//! [`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 -//! [`downcast`]: crate::error::Error +//! [panic hook]: crate::panic::set_hook +//! [`set_hook`]: crate::panic::set_hook +//! [`take_hook`]: crate::panic::take_hook +//! [panic-handler]: +//! [`match`]: ../../std/keyword.match.html +//! [`?`]: ../../std/result/index.html#the-question-mark-operator- #![stable(feature = "rust1", since = "1.0.0")] From 720e987ac0a2123ab00046af0a91c57258fde9aa Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 25 May 2022 11:37:39 -0700 Subject: [PATCH 13/15] update option and result references to expect message docs --- library/core/src/option.rs | 20 ++++++++++++++++++-- library/core/src/result.rs | 12 +++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index f280c321c9c71..77619d61d9f88 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -709,8 +709,24 @@ impl Option { /// x.expect("fruits are healthy"); // panics with `fruits are healthy` /// ``` /// - /// **Note**: Please refer to the documentation on [`Result::expect`] for further information - /// on common message styles. + /// # Recommended Message Style + /// + /// We recommend that `expect` messages are used to describe the reason you + /// _expect_ the `Option` should be `Some`. + /// + /// ```should_panic + /// 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 342acea29dc5b..6f204df99981b 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1034,9 +1034,15 @@ impl Result { /// .expect("env variable `IMPORTANT_PATH` should be set by `wrapper_script.sh`"); /// ``` /// - /// For more detail on expect message styles and the reasoning behind our - /// recommendation please refer to the section on ["Common Message - /// Styles"]() in the [`std::error`]() module docs. + /// **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")] From ef879c680e4a4d7a1060b27e1686bf031067a587 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 25 May 2022 12:20:48 -0700 Subject: [PATCH 14/15] fix broken doctest --- library/core/src/option.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index 77619d61d9f88..cfa77f39f5757 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -715,6 +715,7 @@ impl Option { /// _expect_ the `Option` should be `Some`. /// /// ```should_panic + /// # let slice: &[u8] = &[]; /// let item = slice.get(0) /// .expect("slice should not be empty"); /// ``` From 5fc8a8e22799b6d2d768e5cbb59003930631c2f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Thu, 26 May 2022 13:14:24 +0200 Subject: [PATCH 15/15] clippy::complexity fixes clone_on_copy useless_format bind_instead_of_map filter_map_identity useless_conversion map_flatten unnecessary_unwrap --- compiler/rustc_ast_passes/src/ast_validation.rs | 9 ++++----- .../rustc_borrowck/src/diagnostics/region_errors.rs | 6 +++--- compiler/rustc_codegen_ssa/src/mir/place.rs | 2 +- .../rustc_const_eval/src/interpret/eval_context.rs | 10 +++------- compiler/rustc_log/src/lib.rs | 8 +------- compiler/rustc_middle/src/mir/patch.rs | 3 +-- compiler/rustc_mir_build/src/build/expr/into.rs | 6 +----- compiler/rustc_mir_build/src/build/matches/test.rs | 8 ++++---- compiler/rustc_typeck/src/check/fn_ctxt/checks.rs | 9 ++++----- compiler/rustc_typeck/src/check/method/suggest.rs | 10 +++++----- compiler/rustc_typeck/src/check/op.rs | 2 +- 11 files changed, 28 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 14e5d2ae62337..0520c9ac60cf0 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -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(); } diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs index f2b5c83c5c1d0..6478a104815b6 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs @@ -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"); } diff --git a/compiler/rustc_codegen_ssa/src/mir/place.rs b/compiler/rustc_codegen_ssa/src/mir/place.rs index b6a7bcae93284..3185b952ab886 100644 --- a/compiler/rustc_codegen_ssa/src/mir/place.rs +++ b/compiler/rustc_codegen_ssa/src/mir/place.rs @@ -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() { diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 4c84bd090cb50..0c954ac6e5f6e 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 @@ 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!( diff --git a/compiler/rustc_log/src/lib.rs b/compiler/rustc_log/src/lib.rs index c152815eeca68..f2ec80b0c1b63 100644 --- a/compiler/rustc_log/src/lib.rs +++ b/compiler/rustc_log/src/lib.rs @@ -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() diff --git a/compiler/rustc_middle/src/mir/patch.rs b/compiler/rustc_middle/src/mir/patch.rs index 1bc53d3c9f1c5..c1e1cfef9f89d 100644 --- a/compiler/rustc_middle/src/mir/patch.rs +++ b/compiler/rustc_middle/src/mir/patch.rs @@ -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; diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 569012e152b30..ccbb518e72d08 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 @@ 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 } => { diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index c15b3db1caae3..3774a39503521 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 @@ 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 diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index a02d8c89772a2..82e1ae9d274c0 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 @@ 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()); diff --git a/compiler/rustc_typeck/src/check/method/suggest.rs b/compiler/rustc_typeck/src/check/method/suggest.rs index f9c0ea82e02f1..4e54d554c6ec2 100644 --- a/compiler/rustc_typeck/src/check/method/suggest.rs +++ b/compiler/rustc_typeck/src/check/method/suggest.rs @@ -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(), )); } diff --git a/compiler/rustc_typeck/src/check/op.rs b/compiler/rustc_typeck/src/check/op.rs index c99d9d8f9230d..637f6459525a2 100644 --- a/compiler/rustc_typeck/src/check/op.rs +++ b/compiler/rustc_typeck/src/check/op.rs @@ -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(