From af53809c874e0afb7be966df4d3cfcaa05277c53 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 9 Mar 2022 21:30:52 -0800 Subject: [PATCH 1/7] Format core and std macro rules, removing needless surrounding blocks --- library/core/src/macros/mod.rs | 76 ++++++++++++------- library/std/src/macros.rs | 28 ++++--- src/test/pretty/dollar-crate.pp | 2 +- src/test/ui/macros/trace-macro.stderr | 2 +- src/test/ui/parser/issues/issue-62894.stderr | 2 +- .../tuple-struct-nonexhaustive.stderr | 2 +- 6 files changed, 72 insertions(+), 40 deletions(-) diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index ba7ae55ec6f4b..74c94680e47e5 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -34,7 +34,7 @@ macro_rules! panic { #[cfg_attr(not(test), rustc_diagnostic_item = "assert_eq_macro")] #[allow_internal_unstable(core_panic)] macro_rules! assert_eq { - ($left:expr, $right:expr $(,)?) => ({ + ($left:expr, $right:expr $(,)?) => { match (&$left, &$right) { (left_val, right_val) => { if !(*left_val == *right_val) { @@ -46,8 +46,8 @@ macro_rules! assert_eq { } } } - }); - ($left:expr, $right:expr, $($arg:tt)+) => ({ + }; + ($left:expr, $right:expr, $($arg:tt)+) => { match (&$left, &$right) { (left_val, right_val) => { if !(*left_val == *right_val) { @@ -59,7 +59,7 @@ macro_rules! assert_eq { } } } - }); + }; } /// Asserts that two expressions are not equal to each other (using [`PartialEq`]). @@ -84,7 +84,7 @@ macro_rules! assert_eq { #[cfg_attr(not(test), rustc_diagnostic_item = "assert_ne_macro")] #[allow_internal_unstable(core_panic)] macro_rules! assert_ne { - ($left:expr, $right:expr $(,)?) => ({ + ($left:expr, $right:expr $(,)?) => { match (&$left, &$right) { (left_val, right_val) => { if *left_val == *right_val { @@ -96,8 +96,8 @@ macro_rules! assert_ne { } } } - }); - ($left:expr, $right:expr, $($arg:tt)+) => ({ + }; + ($left:expr, $right:expr, $($arg:tt)+) => { match (&($left), &($right)) { (left_val, right_val) => { if *left_val == *right_val { @@ -109,7 +109,7 @@ macro_rules! assert_ne { } } } - }); + }; } /// Asserts that an expression matches any of the given patterns. @@ -142,7 +142,7 @@ macro_rules! assert_ne { #[allow_internal_unstable(core_panic)] #[rustc_macro_transparency = "semitransparent"] pub macro assert_matches { - ($left:expr, $(|)? $( $pattern:pat_param )|+ $( if $guard: expr )? $(,)?) => ({ + ($left:expr, $(|)? $( $pattern:pat_param )|+ $( if $guard: expr )? $(,)?) => { match $left { $( $pattern )|+ $( if $guard )? => {} ref left_val => { @@ -153,8 +153,8 @@ pub macro assert_matches { ); } } - }), - ($left:expr, $(|)? $( $pattern:pat_param )|+ $( if $guard: expr )?, $($arg:tt)+) => ({ + }, + ($left:expr, $(|)? $( $pattern:pat_param )|+ $( if $guard: expr )?, $($arg:tt)+) => { match $left { $( $pattern )|+ $( if $guard )? => {} ref left_val => { @@ -165,7 +165,7 @@ pub macro assert_matches { ); } } - }), + }, } /// Asserts that a boolean expression is `true` at runtime. @@ -214,7 +214,11 @@ pub macro assert_matches { #[rustc_diagnostic_item = "debug_assert_macro"] #[allow_internal_unstable(edition_panic)] macro_rules! debug_assert { - ($($arg:tt)*) => (if $crate::cfg!(debug_assertions) { $crate::assert!($($arg)*); }) + ($($arg:tt)*) => { + if $crate::cfg!(debug_assertions) { + $crate::assert!($($arg)*); + } + }; } /// Asserts that two expressions are equal to each other. @@ -240,7 +244,11 @@ macro_rules! debug_assert { #[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "debug_assert_eq_macro")] macro_rules! debug_assert_eq { - ($($arg:tt)*) => (if $crate::cfg!(debug_assertions) { $crate::assert_eq!($($arg)*); }) + ($($arg:tt)*) => { + if $crate::cfg!(debug_assertions) { + $crate::assert_eq!($($arg)*); + } + }; } /// Asserts that two expressions are not equal to each other. @@ -266,7 +274,11 @@ macro_rules! debug_assert_eq { #[stable(feature = "assert_ne", since = "1.13.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "debug_assert_ne_macro")] macro_rules! debug_assert_ne { - ($($arg:tt)*) => (if $crate::cfg!(debug_assertions) { $crate::assert_ne!($($arg)*); }) + ($($arg:tt)*) => { + if $crate::cfg!(debug_assertions) { + $crate::assert_ne!($($arg)*); + } + }; } /// Asserts that an expression matches any of the given patterns. @@ -305,7 +317,9 @@ macro_rules! debug_assert_ne { #[allow_internal_unstable(assert_matches)] #[rustc_macro_transparency = "semitransparent"] pub macro debug_assert_matches($($arg:tt)*) { - if $crate::cfg!(debug_assertions) { $crate::assert_matches::assert_matches!($($arg)*); } + if $crate::cfg!(debug_assertions) { + $crate::assert_matches::assert_matches!($($arg)*); + } } /// Returns whether the given expression matches any of the given patterns. @@ -331,7 +345,7 @@ macro_rules! matches { $( $pattern )|+ $( if $guard )? => true, _ => false } - } + }; } /// Unwraps a result or propagates its error. @@ -482,7 +496,9 @@ macro_rules! r#try { #[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "write_macro")] macro_rules! write { - ($dst:expr, $($arg:tt)*) => ($dst.write_fmt($crate::format_args!($($arg)*))) + ($dst:expr, $($arg:tt)*) => { + $dst.write_fmt($crate::format_args!($($arg)*)) + }; } /// Write formatted data into a buffer, with a newline appended. @@ -534,12 +550,12 @@ macro_rules! write { #[cfg_attr(not(test), rustc_diagnostic_item = "writeln_macro")] #[allow_internal_unstable(format_args_nl)] macro_rules! writeln { - ($dst:expr $(,)?) => ( + ($dst:expr $(,)?) => { $crate::write!($dst, "\n") - ); - ($dst:expr, $($arg:tt)*) => ( + }; + ($dst:expr, $($arg:tt)*) => { $dst.write_fmt($crate::format_args_nl!($($arg)*)) - ); + }; } /// Indicates unreachable code. @@ -683,8 +699,12 @@ macro_rules! unreachable { #[cfg_attr(not(test), rustc_diagnostic_item = "unimplemented_macro")] #[allow_internal_unstable(core_panic)] macro_rules! unimplemented { - () => ($crate::panicking::panic("not implemented")); - ($($arg:tt)+) => ($crate::panic!("not implemented: {}", $crate::format_args!($($arg)+))); + () => { + $crate::panicking::panic("not implemented") + }; + ($($arg:tt)+) => { + $crate::panic!("not implemented: {}", $crate::format_args!($($arg)+)) + }; } /// Indicates unfinished code. @@ -746,8 +766,12 @@ macro_rules! unimplemented { #[cfg_attr(not(test), rustc_diagnostic_item = "todo_macro")] #[allow_internal_unstable(core_panic)] macro_rules! todo { - () => ($crate::panicking::panic("not yet implemented")); - ($($arg:tt)+) => ($crate::panic!("not yet implemented: {}", $crate::format_args!($($arg)+))); + () => { + $crate::panicking::panic("not yet implemented") + }; + ($($arg:tt)+) => { + $crate::panic!("not yet implemented: {}", $crate::format_args!($($arg)+)) + }; } /// Definitions of built-in macros. diff --git a/library/std/src/macros.rs b/library/std/src/macros.rs index 23cbfaeef485a..c597fb5df45d2 100644 --- a/library/std/src/macros.rs +++ b/library/std/src/macros.rs @@ -60,7 +60,9 @@ macro_rules! panic { #[cfg_attr(not(test), rustc_diagnostic_item = "print_macro")] #[allow_internal_unstable(print_internals)] macro_rules! print { - ($($arg:tt)*) => ($crate::io::_print($crate::format_args!($($arg)*))); + ($($arg:tt)*) => { + $crate::io::_print($crate::format_args!($($arg)*)) + }; } /// Prints to the standard output, with a newline. @@ -94,10 +96,12 @@ macro_rules! print { #[cfg_attr(not(test), rustc_diagnostic_item = "println_macro")] #[allow_internal_unstable(print_internals, format_args_nl)] macro_rules! println { - () => ($crate::print!("\n")); - ($($arg:tt)*) => ({ - $crate::io::_print($crate::format_args_nl!($($arg)*)); - }) + () => { + $crate::print!("\n") + }; + ($($arg:tt)*) => { + $crate::io::_print($crate::format_args_nl!($($arg)*)) + }; } /// Prints to the standard error. @@ -126,7 +130,9 @@ macro_rules! println { #[cfg_attr(not(test), rustc_diagnostic_item = "eprint_macro")] #[allow_internal_unstable(print_internals)] macro_rules! eprint { - ($($arg:tt)*) => ($crate::io::_eprint($crate::format_args!($($arg)*))); + ($($arg:tt)*) => { + $crate::io::_eprint($crate::format_args!($($arg)*)) + }; } /// Prints to the standard error, with a newline. @@ -155,10 +161,12 @@ macro_rules! eprint { #[cfg_attr(not(test), rustc_diagnostic_item = "eprintln_macro")] #[allow_internal_unstable(print_internals, format_args_nl)] macro_rules! eprintln { - () => ($crate::eprint!("\n")); - ($($arg:tt)*) => ({ - $crate::io::_eprint($crate::format_args_nl!($($arg)*)); - }) + () => { + $crate::eprint!("\n") + }; + ($($arg:tt)*) => { + $crate::io::_eprint($crate::format_args_nl!($($arg)*)) + }; } /// Prints and returns the value of a given expression for quick and dirty diff --git a/src/test/pretty/dollar-crate.pp b/src/test/pretty/dollar-crate.pp index 3af37955f2380..0c96fb593e659 100644 --- a/src/test/pretty/dollar-crate.pp +++ b/src/test/pretty/dollar-crate.pp @@ -9,5 +9,5 @@ // pp-exact:dollar-crate.pp fn main() { - { ::std::io::_print(::core::fmt::Arguments::new_v1(&["rust\n"], &[])); }; + ::std::io::_print(::core::fmt::Arguments::new_v1(&["rust\n"], &[])); } diff --git a/src/test/ui/macros/trace-macro.stderr b/src/test/ui/macros/trace-macro.stderr index 43272248c280e..c8a0fd684304e 100644 --- a/src/test/ui/macros/trace-macro.stderr +++ b/src/test/ui/macros/trace-macro.stderr @@ -5,5 +5,5 @@ LL | println!("Hello, World!"); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: expanding `println! { "Hello, World!" }` - = note: to `{ $crate :: io :: _print($crate :: format_args_nl! ("Hello, World!")) ; }` + = note: to `$crate :: io :: _print($crate :: format_args_nl! ("Hello, World!"))` diff --git a/src/test/ui/parser/issues/issue-62894.stderr b/src/test/ui/parser/issues/issue-62894.stderr index 9b7bd1559cddf..ed5e863bd9def 100644 --- a/src/test/ui/parser/issues/issue-62894.stderr +++ b/src/test/ui/parser/issues/issue-62894.stderr @@ -45,7 +45,7 @@ LL | fn main() {} | ::: $SRC_DIR/core/src/macros/mod.rs:LL:COL | -LL | ($left:expr, $right:expr $(,)?) => ({ +LL | ($left:expr, $right:expr $(,)?) => { | ---------- while parsing argument for this `expr` macro fragment error: aborting due to 4 previous errors diff --git a/src/test/ui/pattern/usefulness/tuple-struct-nonexhaustive.stderr b/src/test/ui/pattern/usefulness/tuple-struct-nonexhaustive.stderr index fc0430d06fa1c..e2a65ff852404 100644 --- a/src/test/ui/pattern/usefulness/tuple-struct-nonexhaustive.stderr +++ b/src/test/ui/pattern/usefulness/tuple-struct-nonexhaustive.stderr @@ -12,7 +12,7 @@ LL | struct Foo(isize, isize); = note: the matched value is of type `Foo` help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown | -LL ~ Foo(2, b) => println!("{}", b) +LL ~ Foo(2, b) => println!("{}", b), LL + Foo(_, _) => todo!() | From 896b113ec3b777c72fd240c24a47792c90e65be4 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Tue, 15 Mar 2022 19:13:56 +0900 Subject: [PATCH 2/7] use `format_args_capture` in some parts of rustc_parse --- compiler/rustc_parse/src/parser/attr.rs | 4 +- .../rustc_parse/src/parser/diagnostics.rs | 28 +++++----- compiler/rustc_parse/src/parser/expr.rs | 36 ++++++------- compiler/rustc_parse/src/parser/item.rs | 53 +++++++++---------- 4 files changed, 60 insertions(+), 61 deletions(-) diff --git a/compiler/rustc_parse/src/parser/attr.rs b/compiler/rustc_parse/src/parser/attr.rs index 379e47077ea18..8f759ae84fa69 100644 --- a/compiler/rustc_parse/src/parser/attr.rs +++ b/compiler/rustc_parse/src/parser/attr.rs @@ -139,7 +139,7 @@ impl<'a> Parser<'a> { Ok(attr::mk_attr_from_item(item, None, style, attr_sp)) } else { let token_str = pprust::token_to_string(&this.token); - let msg = &format!("expected `#`, found `{}`", token_str); + let msg = &format!("expected `#`, found `{token_str}`"); Err(this.struct_span_err(this.token.span, msg)) } }) @@ -421,7 +421,7 @@ impl<'a> Parser<'a> { } let found = pprust::token_to_string(&self.token); - let msg = format!("expected unsuffixed literal or identifier, found `{}`", found); + let msg = format!("expected unsuffixed literal or identifier, found `{found}`"); Err(self.struct_span_err(self.token.span, &msg)) } } diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 21d5bec65f03a..1909e9ee74949 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -327,8 +327,8 @@ impl<'a> Parser<'a> { expect.clone() }; ( - format!("expected one of {}, found {}", expect, actual), - (self.prev_token.span.shrink_to_hi(), format!("expected one of {}", short_expect)), + format!("expected one of {expect}, found {actual}"), + (self.prev_token.span.shrink_to_hi(), format!("expected one of {short_expect}")), ) } else if expected.is_empty() { ( @@ -337,8 +337,8 @@ impl<'a> Parser<'a> { ) } else { ( - format!("expected {}, found {}", expect, actual), - (self.prev_token.span.shrink_to_hi(), format!("expected {}", expect)), + format!("expected {expect}, found {actual}"), + (self.prev_token.span.shrink_to_hi(), format!("expected {expect}")), ) }; self.last_unexpected_token_span = Some(self.token.span); @@ -421,7 +421,7 @@ impl<'a> Parser<'a> { String::new(), Applicability::MachineApplicable, ); - err.note(&format!("the raw string started with {} `#`s", n_hashes)); + err.note(&format!("the raw string started with {n_hashes} `#`s")); true } _ => false, @@ -1191,7 +1191,7 @@ impl<'a> Parser<'a> { _ => None, }; if let Some(name) = previous_item_kind_name { - err.help(&format!("{} declarations are not followed by a semicolon", name)); + err.help(&format!("{name} declarations are not followed by a semicolon")); } } err.emit(); @@ -1226,12 +1226,12 @@ impl<'a> Parser<'a> { "expected `{}`, found {}", token_str, match (&self.token.kind, self.subparser_name) { - (token::Eof, Some(origin)) => format!("end of {}", origin), + (token::Eof, Some(origin)) => format!("end of {origin}"), _ => this_token_str, }, ); let mut err = self.struct_span_err(sp, &msg); - let label_exp = format!("expected `{}`", token_str); + let label_exp = format!("expected `{token_str}`"); match self.recover_closing_delimiter(&[t.clone()], err) { Err(e) => err = e, Ok(recovered) => { @@ -1368,7 +1368,7 @@ impl<'a> Parser<'a> { Applicability::MachineApplicable, ); } - err.span_suggestion(lo.shrink_to_lo(), &format!("{}you can still access the deprecated `try!()` macro using the \"raw identifier\" syntax", prefix), "r#".to_string(), Applicability::MachineApplicable); + err.span_suggestion(lo.shrink_to_lo(), &format!("{prefix}you can still access the deprecated `try!()` macro using the \"raw identifier\" syntax"), "r#".to_string(), Applicability::MachineApplicable); err.emit(); Ok(self.mk_expr_err(lo.to(hi))) } else { @@ -1504,7 +1504,7 @@ impl<'a> Parser<'a> { delim.retain(|c| c != '`'); err.span_suggestion_short( self.prev_token.span.shrink_to_hi(), - &format!("`{}` may belong here", delim), + &format!("`{delim}` may belong here"), delim, Applicability::MaybeIncorrect, ); @@ -1698,7 +1698,7 @@ impl<'a> Parser<'a> { ( ident, "self: ".to_string(), - format!("{}: &{}TypeName", ident, mutab), + format!("{ident}: &{mutab}TypeName"), "_: ".to_string(), pat.span.shrink_to_lo(), pat.span, @@ -1826,7 +1826,7 @@ impl<'a> Parser<'a> { let (span, msg) = match (&self.token.kind, self.subparser_name) { (&token::Eof, Some(origin)) => { let sp = self.sess.source_map().next_point(self.prev_token.span); - (sp, format!("expected expression, found end of {}", origin)) + (sp, format!("expected expression, found end of {origin}")) } _ => ( self.token.span, @@ -1975,8 +1975,8 @@ impl<'a> Parser<'a> { (ty_generics, self.sess.source_map().span_to_snippet(param.span())) { let (span, sugg) = match &generics.params[..] { - [] => (generics.span, format!("<{}>", snippet)), - [.., generic] => (generic.span().shrink_to_hi(), format!(", {}", snippet)), + [] => (generics.span, format!("<{snippet}>")), + [.., generic] => (generic.span().shrink_to_hi(), format!(", {snippet}")), }; err.multipart_suggestion( "`const` parameters must be declared for the `impl`", diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index b993d48c995b4..550d79a898c11 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -223,7 +223,7 @@ impl<'a> Parser<'a> { AssocOp::NotEqual => "!=", _ => unreachable!(), }; - self.struct_span_err(sp, &format!("invalid comparison operator `{}=`", sugg)) + self.struct_span_err(sp, &format!("invalid comparison operator `{sugg}=`")) .span_suggestion_short( sp, &format!("`{s}=` is not a valid comparison operator, use `{s}`", s = sugg), @@ -441,10 +441,10 @@ impl<'a> Parser<'a> { /// Error on `and` and `or` suggesting `&&` and `||` respectively. fn error_bad_logical_op(&self, bad: &str, good: &str, english: &str) { - self.struct_span_err(self.token.span, &format!("`{}` is not a logical operator", bad)) + self.struct_span_err(self.token.span, &format!("`{bad}` is not a logical operator")) .span_suggestion_short( self.token.span, - &format!("use `{}` to perform logical {}", good, english), + &format!("use `{good}` to perform logical {english}"), good.to_string(), Applicability::MachineApplicable, ) @@ -766,9 +766,9 @@ impl<'a> Parser<'a> { self.look_ahead(1, |t| t.span).to(span_after_type), "interpreted as generic arguments", ) - .span_label(self.token.span, format!("not interpreted as {}", op_noun)) + .span_label(self.token.span, format!("not interpreted as {op_noun}")) .multipart_suggestion( - &format!("try {} the cast value", op_verb), + &format!("try {op_verb} the cast value"), vec![ (expr.span.shrink_to_lo(), "(".to_string()), (expr.span.shrink_to_hi(), ")".to_string()), @@ -970,7 +970,7 @@ impl<'a> Parser<'a> { fn error_unexpected_after_dot(&self) { // FIXME Could factor this out into non_fatal_unexpected or something. let actual = pprust::token_to_string(&self.token); - self.struct_span_err(self.token.span, &format!("unexpected token: `{}`", actual)).emit(); + self.struct_span_err(self.token.span, &format!("unexpected token: `{actual}`")).emit(); } // We need an identifier or integer, but the next token is a float. @@ -1151,7 +1151,7 @@ impl<'a> Parser<'a> { mem::replace(err, replacement_err).cancel(); err.multipart_suggestion( - &format!("if `{}` is a struct, use braces as delimiters", name), + &format!("if `{name}` is a struct, use braces as delimiters"), vec![ (open_paren, " { ".to_string()), (close_paren, " }".to_string()), @@ -1159,7 +1159,7 @@ impl<'a> Parser<'a> { Applicability::MaybeIncorrect, ); err.multipart_suggestion( - &format!("if `{}` is a function, use the arguments directly", name), + &format!("if `{name}` is a function, use the arguments directly"), fields .into_iter() .map(|field| (field.span.until(field.expr.span), String::new())) @@ -1776,9 +1776,9 @@ impl<'a> Parser<'a> { ) .emit(); } else { - let msg = format!("invalid suffix `{}` for number literal", suf); + let msg = format!("invalid suffix `{suf}` for number literal"); self.struct_span_err(span, &msg) - .span_label(span, format!("invalid suffix `{}`", suf)) + .span_label(span, format!("invalid suffix `{suf}`")) .help("the suffix must be one of the numeric types (`u32`, `isize`, `f32`, etc.)") .emit(); } @@ -1791,9 +1791,9 @@ impl<'a> Parser<'a> { let msg = format!("invalid width `{}` for float literal", &suf[1..]); self.struct_span_err(span, &msg).help("valid widths are 32 and 64").emit(); } else { - let msg = format!("invalid suffix `{}` for float literal", suf); + let msg = format!("invalid suffix `{suf}` for float literal"); self.struct_span_err(span, &msg) - .span_label(span, format!("invalid suffix `{}`", suf)) + .span_label(span, format!("invalid suffix `{suf}`")) .help("valid suffixes are `f32` and `f64`") .emit(); } @@ -1805,7 +1805,7 @@ impl<'a> Parser<'a> { 2 => "binary", _ => unreachable!(), }; - self.struct_span_err(span, &format!("{} float literal is not supported", descr)) + self.struct_span_err(span, &format!("{descr} float literal is not supported")) .span_label(span, "not supported") .emit(); } @@ -1825,7 +1825,7 @@ impl<'a> Parser<'a> { let mut err = self .sess .span_diagnostic - .struct_span_warn(sp, &format!("suffixes on {} are invalid", kind)); + .struct_span_warn(sp, &format!("suffixes on {kind} are invalid")); err.note(&format!( "`{}` is *temporarily* accepted on tuple index fields as it was \ incorrectly accepted on stable for a few releases", @@ -1842,10 +1842,10 @@ impl<'a> Parser<'a> { ); err } else { - self.struct_span_err(sp, &format!("suffixes on {} are invalid", kind)) + self.struct_span_err(sp, &format!("suffixes on {kind} are invalid")) .forget_guarantee() }; - err.span_label(sp, format!("invalid suffix `{}`", suf)); + err.span_label(sp, format!("invalid suffix `{suf}`")); err.emit(); } } @@ -2211,7 +2211,7 @@ impl<'a> Parser<'a> { let ctx = if is_ctx_else { "else" } else { "if" }; self.struct_span_err(last, "outer attributes are not allowed on `if` and `else` branches") .span_label(branch_span, "the attributes are attached to this branch") - .span_label(ctx_span, format!("the branch belongs to this `{}`", ctx)) + .span_label(ctx_span, format!("the branch belongs to this `{ctx}`")) .span_suggestion( span, "remove the attributes", @@ -2391,7 +2391,7 @@ impl<'a> Parser<'a> { err.span_label(arrow_span, "while parsing the `match` arm starting here"); if stmts.len() > 1 { err.multipart_suggestion( - &format!("surround the statement{} with a body", s), + &format!("surround the statement{s} with a body"), vec![ (span.shrink_to_lo(), "{ ".to_string()), (span.shrink_to_hi(), " }".to_string()), diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 178bb62e0b25a..b582f060395c3 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -66,7 +66,7 @@ impl<'a> Parser<'a> { if !self.eat(term) { let token_str = super::token_descr(&self.token); if !self.maybe_consume_incorrect_semicolon(&items) { - let msg = &format!("expected item, found {}", token_str); + let msg = &format!("expected item, found {token_str}"); let mut err = self.struct_span_err(self.token.span, msg); err.span_label(self.token.span, "expected item"); return Err(err); @@ -163,9 +163,9 @@ impl<'a> Parser<'a> { } let vs = pprust::vis_to_string(&vis); let vs = vs.trim_end(); - self.struct_span_err(vis.span, &format!("visibility `{}` is not followed by an item", vs)) + self.struct_span_err(vis.span, &format!("visibility `{vs}` is not followed by an item")) .span_label(vis.span, "the visibility") - .help(&format!("you likely meant to define an item, e.g., `{} fn foo() {{}}`", vs)) + .help(&format!("you likely meant to define an item, e.g., `{vs} fn foo() {{}}`")) .emit(); } @@ -327,7 +327,7 @@ impl<'a> Parser<'a> { if self.look_ahead(1, |t| *t == token::OpenDelim(token::Brace)) { // possible public struct definition where `struct` was forgotten let ident = self.parse_ident().unwrap(); - let msg = format!("add `struct` here to parse `{}` as a public struct", ident); + let msg = format!("add `struct` here to parse `{ident}` as a public struct"); let mut err = self.struct_span_err(sp, "missing `struct` for struct definition"); err.span_suggestion_short( sp, @@ -355,16 +355,16 @@ impl<'a> Parser<'a> { ("fn` or `struct", "function or struct", true) }; - let msg = format!("missing `{}` for {} definition", kw, kw_name); + let msg = format!("missing `{kw}` for {kw_name} definition"); let mut err = self.struct_span_err(sp, &msg); if !ambiguous { self.consume_block(token::Brace, ConsumeClosingDelim::Yes); let suggestion = - format!("add `{}` here to parse `{}` as a public {}", kw, ident, kw_name); + format!("add `{kw}` here to parse `{ident}` as a public {kw_name}"); err.span_suggestion_short( sp, &suggestion, - format!(" {} ", kw), + format!(" {kw} "), Applicability::MachineApplicable, ); } else if let Ok(snippet) = self.span_to_snippet(ident_sp) { @@ -393,12 +393,12 @@ impl<'a> Parser<'a> { } else { ("fn` or `struct", "function or struct", true) }; - let msg = format!("missing `{}` for {} definition", kw, kw_name); + let msg = format!("missing `{kw}` for {kw_name} definition"); let mut err = self.struct_span_err(sp, &msg); if !ambiguous { err.span_suggestion_short( sp, - &format!("add `{}` here to parse `{}` as a public {}", kw, ident, kw_name), + &format!("add `{kw}` here to parse `{ident}` as a public {kw_name}"), format!(" {} ", kw), Applicability::MachineApplicable, ); @@ -1031,8 +1031,8 @@ impl<'a> Parser<'a> { fn error_bad_item_kind(&self, span: Span, kind: &ItemKind, ctx: &str) -> Option { let span = self.sess.source_map().guess_head_span(span); let descr = kind.descr(); - self.struct_span_err(span, &format!("{} is not supported in {}", descr, ctx)) - .help(&format!("consider moving the {} out to a nearby module scope", descr)) + self.struct_span_err(span, &format!("{descr} is not supported in {ctx}")) + .help(&format!("consider moving the {descr} out to a nearby module scope")) .emit(); None } @@ -1161,11 +1161,11 @@ impl<'a> Parser<'a> { Some(Mutability::Not) => "static", None => "const", }; - let mut err = self.struct_span_err(id.span, &format!("missing type for `{}` item", kind)); + let mut err = self.struct_span_err(id.span, &format!("missing type for `{kind}` item")); err.span_suggestion( id.span, "provide a type for the item", - format!("{}: ", id), + format!("{id}: "), Applicability::HasPlaceholders, ); err.stash(id.span, StashKey::ItemNoType); @@ -1282,8 +1282,7 @@ impl<'a> Parser<'a> { } else { let token_str = super::token_descr(&self.token); let msg = &format!( - "expected `where`, `{{`, `(`, or `;` after struct name, found {}", - token_str + "expected `where`, `{{`, `(`, or `;` after struct name, found {token_str}" ); let mut err = self.struct_span_err(self.token.span, msg); err.span_label(self.token.span, "expected `where`, `{`, `(`, or `;` after struct name"); @@ -1310,7 +1309,7 @@ impl<'a> Parser<'a> { VariantData::Struct(fields, recovered) } else { let token_str = super::token_descr(&self.token); - let msg = &format!("expected `where` or `{{` after union name, found {}", token_str); + let msg = &format!("expected `where` or `{{` after union name, found {token_str}"); let mut err = self.struct_span_err(self.token.span, msg); err.span_label(self.token.span, "expected `where` or `{` after union name"); return Err(err); @@ -1591,7 +1590,7 @@ impl<'a> Parser<'a> { } let mut err = self.struct_span_err( lo.to(self.prev_token.span), - &format!("functions are not allowed in {} definitions", adt_ty), + &format!("functions are not allowed in {adt_ty} definitions"), ); err.help("unlike in C++, Java, and C#, functions are declared in `impl` blocks"); err.help("see https://doc.rust-lang.org/book/ch05-03-method-syntax.html for more information"); @@ -1706,7 +1705,7 @@ impl<'a> Parser<'a> { let vstr = pprust::vis_to_string(vis); let vstr = vstr.trim_end(); if macro_rules { - let msg = format!("can't qualify macro_rules invocation with `{}`", vstr); + let msg = format!("can't qualify macro_rules invocation with `{vstr}`"); self.struct_span_err(vis.span, &msg) .span_suggestion( vis.span, @@ -1723,7 +1722,7 @@ impl<'a> Parser<'a> { String::new(), Applicability::MachineApplicable, ) - .help(&format!("try adjusting the macro to put `{}` inside the invocation", vstr)) + .help(&format!("try adjusting the macro to put `{vstr}` inside the invocation")) .emit(); } } @@ -1781,11 +1780,11 @@ impl<'a> Parser<'a> { self.struct_span_err( kw_token.span, - &format!("`{}` definition cannot be nested inside `{}`", kw_str, keyword), + &format!("`{kw_str}` definition cannot be nested inside `{keyword}`"), ) .span_suggestion( item.unwrap().span, - &format!("consider creating a new `{}` definition instead of nesting", kw_str), + &format!("consider creating a new `{kw_str}` definition instead of nesting"), String::new(), Applicability::MaybeIncorrect, ) @@ -2045,11 +2044,11 @@ impl<'a> Parser<'a> { err.span_suggestion( self.token.uninterpolated_span(), - &format!("`{}` already used earlier, remove this one", original_kw), + &format!("`{original_kw}` already used earlier, remove this one"), "".to_string(), Applicability::MachineApplicable, ) - .span_note(original_sp, &format!("`{}` first seen here", original_kw)); + .span_note(original_sp, &format!("`{original_kw}` first seen here")); } // The keyword has not been seen yet, suggest correct placement in the function front matter else if let Some(WrongKw::Misplaced(correct_pos_sp)) = wrong_kw { @@ -2060,8 +2059,8 @@ impl<'a> Parser<'a> { err.span_suggestion( correct_pos_sp.to(misplaced_qual_sp), - &format!("`{}` must come before `{}`", misplaced_qual, current_qual), - format!("{} {}", misplaced_qual, current_qual), + &format!("`{misplaced_qual}` must come before `{current_qual}`"), + format!("{misplaced_qual} {current_qual}"), Applicability::MachineApplicable, ).note("keyword order for functions declaration is `default`, `pub`, `const`, `async`, `unsafe`, `extern`"); } @@ -2084,8 +2083,8 @@ impl<'a> Parser<'a> { if matches!(orig_vis.kind, VisibilityKind::Inherited) { err.span_suggestion( sp_start.to(self.prev_token.span), - &format!("visibility `{}` must come before `{}`", vs, snippet), - format!("{} {}", vs, snippet), + &format!("visibility `{vs}` must come before `{snippet}`"), + format!("{vs} {snippet}"), Applicability::MachineApplicable, ); } From 7da07ff48bee36fabce6a6683eabd27003e32b8d Mon Sep 17 00:00:00 2001 From: "zed.zy" Date: Tue, 15 Mar 2022 19:37:52 +0800 Subject: [PATCH 3/7] Improve the explanation about the behaviour of read_line --- library/std/src/io/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 6005270a75fec..e53e1f4930935 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2110,7 +2110,7 @@ pub trait BufRead: Read { } /// Read all bytes until a newline (the `0xA` byte) is reached, and append - /// them to the provided buffer. + /// them to the provided buffer (without clear buffer before appending to it). /// /// This function will read bytes from the underlying stream until the /// newline delimiter (the `0xA` byte) or EOF is found. Once found, all bytes From 261d5fc95bf1ad5ec45e464492f7d6583d0af80f Mon Sep 17 00:00:00 2001 From: Caio Date: Tue, 15 Mar 2022 17:00:16 -0300 Subject: [PATCH 4/7] Ensure that `let_else` does not interact with `let_chains` --- ...-else-does-not-interact-with-let-chains.rs | 54 ++++++++ ...e-does-not-interact-with-let-chains.stderr | 119 ++++++++++++++++++ 2 files changed, 173 insertions(+) create mode 100644 src/test/ui/rfc-2497-if-let-chains/ensure-that-let-else-does-not-interact-with-let-chains.rs create mode 100644 src/test/ui/rfc-2497-if-let-chains/ensure-that-let-else-does-not-interact-with-let-chains.stderr diff --git a/src/test/ui/rfc-2497-if-let-chains/ensure-that-let-else-does-not-interact-with-let-chains.rs b/src/test/ui/rfc-2497-if-let-chains/ensure-that-let-else-does-not-interact-with-let-chains.rs new file mode 100644 index 0000000000000..e24649ea044f5 --- /dev/null +++ b/src/test/ui/rfc-2497-if-let-chains/ensure-that-let-else-does-not-interact-with-let-chains.rs @@ -0,0 +1,54 @@ +#![feature(let_chains, let_else)] + +fn main() { + let opt = Some(1i32); + + let Some(n) = opt else { + return; + }; + let Some(n) = opt && n == 1 else { + //~^ ERROR a `&&` expression cannot be directly assigned in `let...else` + //~| ERROR mismatched types + //~| ERROR mismatched types + return; + }; + let Some(n) = opt && let another = n else { + //~^ ERROR a `&&` expression cannot be directly assigned in `let...else` + //~| ERROR `let` expressions are not supported here + //~| ERROR mismatched types + //~| ERROR mismatched types + return; + }; + + if let Some(n) = opt else { + //~^ ERROR missing condition for `if` expression + return; + }; + if let Some(n) = opt && n == 1 else { + //~^ ERROR missing condition for `if` expression + return; + }; + if let Some(n) = opt && let another = n else { + //~^ ERROR missing condition for `if` expression + return; + }; + + { + while let Some(n) = opt else { + //~^ ERROR expected `{`, found keyword `else` + return; + }; + } + { + while let Some(n) = opt && n == 1 else { + //~^ ERROR expected `{`, found keyword `else` + return; + }; + } + { + while let Some(n) = opt && let another = n else { + //~^ ERROR expected `{`, found keyword `else` + return; + }; + } +} diff --git a/src/test/ui/rfc-2497-if-let-chains/ensure-that-let-else-does-not-interact-with-let-chains.stderr b/src/test/ui/rfc-2497-if-let-chains/ensure-that-let-else-does-not-interact-with-let-chains.stderr new file mode 100644 index 0000000000000..992c34eb402d8 --- /dev/null +++ b/src/test/ui/rfc-2497-if-let-chains/ensure-that-let-else-does-not-interact-with-let-chains.stderr @@ -0,0 +1,119 @@ +error: a `&&` expression cannot be directly assigned in `let...else` + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:9:19 + | +LL | let Some(n) = opt && n == 1 else { + | ^^^^^^^^^^^^^ + | +help: wrap the expression in parentheses + | +LL | let Some(n) = (opt && n == 1) else { + | + + + +error: a `&&` expression cannot be directly assigned in `let...else` + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:15:19 + | +LL | let Some(n) = opt && let another = n else { + | ^^^^^^^^^^^^^^^^^^^^^^ + | +help: wrap the expression in parentheses + | +LL | let Some(n) = (opt && let another = n) else { + | + + + +error: missing condition for `if` expression + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:23:7 + | +LL | if let Some(n) = opt else { + | ^ expected if condition here + +error: missing condition for `if` expression + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:27:7 + | +LL | if let Some(n) = opt && n == 1 else { + | ^ expected if condition here + +error: missing condition for `if` expression + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:31:7 + | +LL | if let Some(n) = opt && let another = n else { + | ^ expected if condition here + +error: expected `{`, found keyword `else` + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:37:33 + | +LL | while let Some(n) = opt else { + | ----- ----------------- ^^^^ expected `{` + | | | + | | this `while` condition successfully parsed + | while parsing the body of this `while` expression + +error: expected `{`, found keyword `else` + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:43:43 + | +LL | while let Some(n) = opt && n == 1 else { + | ----- --------------------------- ^^^^ expected `{` + | | | + | | this `while` condition successfully parsed + | while parsing the body of this `while` expression + +error: expected `{`, found keyword `else` + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:49:52 + | +LL | while let Some(n) = opt && let another = n else { + | ----- ------------------------------------ ^^^^ expected `{` + | | | + | | this `while` condition successfully parsed + | while parsing the body of this `while` expression + +error: `let` expressions are not supported here + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:15:26 + | +LL | let Some(n) = opt && let another = n else { + | ^^^^^^^^^^^^^^^ + | + = note: only supported directly in conditions of `if` and `while` expressions + = note: as well as when nested within `&&` and parentheses in those conditions + +error[E0308]: mismatched types + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:9:19 + | +LL | let Some(n) = opt && n == 1 else { + | ^^^ expected `bool`, found enum `Option` + | + = note: expected type `bool` + found enum `Option` + +error[E0308]: mismatched types + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:9:9 + | +LL | let Some(n) = opt && n == 1 else { + | ^^^^^^^ ------------- this expression has type `bool` + | | + | expected `bool`, found enum `Option` + | + = note: expected type `bool` + found enum `Option<_>` + +error[E0308]: mismatched types + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:15:19 + | +LL | let Some(n) = opt && let another = n else { + | ^^^ expected `bool`, found enum `Option` + | + = note: expected type `bool` + found enum `Option` + +error[E0308]: mismatched types + --> $DIR/ensure-that-let-else-does-not-interact-with-let-chains.rs:15:9 + | +LL | let Some(n) = opt && let another = n else { + | ^^^^^^^ ---------------------- this expression has type `bool` + | | + | expected `bool`, found enum `Option` + | + = note: expected type `bool` + found enum `Option<_>` + +error: aborting due to 13 previous errors + +For more information about this error, try `rustc --explain E0308`. From 0f4c81a1a9c6c45a2b45c336dd77b3bb9fdfc62e Mon Sep 17 00:00:00 2001 From: est31 Date: Tue, 15 Mar 2022 03:48:53 +0100 Subject: [PATCH 5/7] Extend the irrefutable_let_patterns lint to let chains Only look for complete suffixes or prefixes of irrefutable let patterns, so that an irrefutable let pattern in a chain surrounded by refutable ones is not linted, as it is an useful pattern. --- .../src/thir/pattern/check_match.rs | 215 +++++++++++++++--- src/test/ui/mir/mir_let_chains_drop_order.rs | 1 + .../ast-lowering-does-not-wrap-let-chains.rs | 1 + .../irrefutable-lets.disallowed.stderr | 115 ++++++++++ .../irrefutable-lets.rs | 52 ++++- 5 files changed, 339 insertions(+), 45 deletions(-) create mode 100644 src/test/ui/rfc-2497-if-let-chains/irrefutable-lets.disallowed.stderr diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index e2c56df60148b..8a3a46c11903a 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -158,7 +158,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> { self.check_patterns(pat, Refutable); let mut cx = self.new_cx(scrutinee.hir_id); let tpat = self.lower_pattern(&mut cx, pat, &mut false); - check_let_reachability(&mut cx, pat.hir_id, tpat, span); + self.check_let_reachability(&mut cx, pat.hir_id, tpat, span); } fn check_match( @@ -176,7 +176,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> { if let Some(hir::Guard::IfLet(ref pat, _)) = arm.guard { self.check_patterns(pat, Refutable); let tpat = self.lower_pattern(&mut cx, pat, &mut false); - check_let_reachability(&mut cx, pat.hir_id, tpat, tpat.span()); + self.check_let_reachability(&mut cx, pat.hir_id, tpat, tpat.span()); } } @@ -224,6 +224,157 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> { } } + fn check_let_reachability( + &mut self, + cx: &mut MatchCheckCtxt<'p, 'tcx>, + pat_id: HirId, + pat: &'p DeconstructedPat<'p, 'tcx>, + span: Span, + ) { + if self.check_let_chain(cx, pat_id) { + return; + } + + if is_let_irrefutable(cx, pat_id, pat) { + irrefutable_let_pattern(cx.tcx, pat_id, span); + } + } + + fn check_let_chain(&mut self, cx: &mut MatchCheckCtxt<'p, 'tcx>, pat_id: HirId) -> bool { + let hir = self.tcx.hir(); + let parent = hir.get_parent_node(pat_id); + + // First, figure out if the given pattern is part of a let chain, + // and if so, obtain the top node of the chain. + let mut top = parent; + let mut part_of_chain = false; + loop { + let new_top = hir.get_parent_node(top); + if let hir::Node::Expr( + hir::Expr { + kind: hir::ExprKind::Binary(Spanned { node: hir::BinOpKind::And, .. }, lhs, rhs), + .. + }, + .., + ) = hir.get(new_top) + { + // If this isn't the first iteration, we need to check + // if there is a let expr before us in the chain, so + // that we avoid doubly checking the let chain. + + // The way a chain of &&s is encoded is ((let ... && let ...) && let ...) && let ... + // as && is left-to-right associative. Thus, we need to check rhs. + if part_of_chain && matches!(rhs.kind, hir::ExprKind::Let(..)) { + return true; + } + // If there is a let at the lhs, and we provide the rhs, we don't do any checking either. + if !part_of_chain && matches!(lhs.kind, hir::ExprKind::Let(..)) && rhs.hir_id == top + { + return true; + } + } else { + // We've reached the top. + break; + } + + // Since this function is called within a let context, it is reasonable to assume that any parent + // `&&` infers a let chain + part_of_chain = true; + top = new_top; + } + if !part_of_chain { + return false; + } + + // Second, obtain the refutabilities of all exprs in the chain, + // and record chain members that aren't let exprs. + let mut chain_refutabilities = Vec::new(); + let hir::Node::Expr(top_expr) = hir.get(top) else { + // We ensure right above that it's an Expr + unreachable!() + }; + let mut cur_expr = top_expr; + loop { + let mut add = |expr: &hir::Expr<'tcx>| { + let refutability = match expr.kind { + hir::ExprKind::Let(hir::Let { pat, init, span, .. }) => { + let mut ncx = self.new_cx(init.hir_id); + let tpat = self.lower_pattern(&mut ncx, pat, &mut false); + + let refutable = !is_let_irrefutable(&mut ncx, pat.hir_id, tpat); + Some((*span, refutable)) + } + _ => None, + }; + chain_refutabilities.push(refutability); + }; + if let hir::Expr { + kind: hir::ExprKind::Binary(Spanned { node: hir::BinOpKind::And, .. }, lhs, rhs), + .. + } = cur_expr + { + add(rhs); + cur_expr = lhs; + } else { + add(cur_expr); + break; + } + } + chain_refutabilities.reverse(); + + // Third, emit the actual warnings. + + if chain_refutabilities.iter().all(|r| matches!(*r, Some((_, false)))) { + // The entire chain is made up of irrefutable `let` statements + let let_source = let_source_parent(self.tcx, top, None); + irrefutable_let_patterns( + cx.tcx, + top, + let_source, + chain_refutabilities.len(), + top_expr.span, + ); + return true; + } + let lint_affix = |affix: &[Option<(Span, bool)>], kind, suggestion| { + let span_start = affix[0].unwrap().0; + let span_end = affix.last().unwrap().unwrap().0; + let span = span_start.to(span_end); + let cnt = affix.len(); + cx.tcx.struct_span_lint_hir(IRREFUTABLE_LET_PATTERNS, top, span, |lint| { + let s = pluralize!(cnt); + let mut diag = lint.build(&format!("{kind} irrefutable pattern{s} in let chain")); + diag.note(&format!( + "{these} pattern{s} will always match", + these = pluralize!("this", cnt), + )); + diag.help(&format!( + "consider moving {} {suggestion}", + if cnt > 1 { "them" } else { "it" } + )); + diag.emit() + }); + }; + if let Some(until) = chain_refutabilities.iter().position(|r| !matches!(*r, Some((_, false)))) && until > 0 { + // The chain has a non-zero prefix of irrefutable `let` statements. + + // Check if the let source is while, for there is no alternative place to put a prefix, + // and we shouldn't lint. + let let_source = let_source_parent(self.tcx, top, None); + if !matches!(let_source, LetSource::WhileLet) { + // Emit the lint + let prefix = &chain_refutabilities[..until]; + lint_affix(prefix, "leading", "outside of the construct"); + } + } + if let Some(from) = chain_refutabilities.iter().rposition(|r| !matches!(*r, Some((_, false)))) && from != (chain_refutabilities.len() - 1) { + // The chain has a non-empty suffix of irrefutable `let` statements + let suffix = &chain_refutabilities[from + 1..]; + lint_affix(suffix, "trailing", "into the body"); + } + true + } + fn check_irrefutable(&self, pat: &'tcx Pat<'tcx>, origin: &str, sp: Option) { let mut cx = self.new_cx(pat.hir_id); @@ -453,6 +604,17 @@ fn unreachable_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, catchall: Option< } fn irrefutable_let_pattern(tcx: TyCtxt<'_>, id: HirId, span: Span) { + let source = let_source(tcx, id); + irrefutable_let_patterns(tcx, id, source, 1, span); +} + +fn irrefutable_let_patterns( + tcx: TyCtxt<'_>, + id: HirId, + source: LetSource, + count: usize, + span: Span, +) { macro_rules! emit_diag { ( $lint:expr, @@ -460,14 +622,15 @@ fn irrefutable_let_pattern(tcx: TyCtxt<'_>, id: HirId, span: Span) { $note_sufix:expr, $help_sufix:expr ) => {{ - let mut diag = $lint.build(concat!("irrefutable ", $source_name, " pattern")); - diag.note(concat!("this pattern will always match, so the ", $note_sufix)); + let s = pluralize!(count); + let these = pluralize!("this", count); + let mut diag = $lint.build(&format!("irrefutable {} pattern{s}", $source_name)); + diag.note(&format!("{these} pattern{s} will always match, so the {}", $note_sufix)); diag.help(concat!("consider ", $help_sufix)); diag.emit() }}; } - let source = let_source(tcx, id); let span = match source { LetSource::LetElse(span) => span, _ => span, @@ -511,16 +674,11 @@ fn irrefutable_let_pattern(tcx: TyCtxt<'_>, id: HirId, span: Span) { }); } -fn check_let_reachability<'p, 'tcx>( +fn is_let_irrefutable<'p, 'tcx>( cx: &mut MatchCheckCtxt<'p, 'tcx>, pat_id: HirId, pat: &'p DeconstructedPat<'p, 'tcx>, - span: Span, -) { - if is_let_chain(cx.tcx, pat_id) { - return; - } - +) -> bool { let arms = [MatchArm { pat, hir_id: pat_id, has_guard: false }]; let report = compute_match_usefulness(&cx, &arms, pat_id, pat.ty()); @@ -529,10 +687,9 @@ fn check_let_reachability<'p, 'tcx>( // `is_uninhabited` check. report_arm_reachability(&cx, &report); - if report.non_exhaustiveness_witnesses.is_empty() { - // The match is exhaustive, i.e. the `if let` pattern is irrefutable. - irrefutable_let_pattern(cx.tcx, pat_id, span); - } + // If the list of witnesses is empty, the match is exhaustive, + // i.e. the `if let` pattern is irrefutable. + report.non_exhaustiveness_witnesses.is_empty() } /// Report unreachable arms, if any. @@ -941,13 +1098,19 @@ fn let_source(tcx: TyCtxt<'_>, pat_id: HirId) -> LetSource { let hir = tcx.hir(); let parent = hir.get_parent_node(pat_id); + let_source_parent(tcx, parent, Some(pat_id)) +} + +fn let_source_parent(tcx: TyCtxt<'_>, parent: HirId, pat_id: Option) -> LetSource { + let hir = tcx.hir(); + let parent_node = hir.get(parent); match parent_node { hir::Node::Arm(hir::Arm { guard: Some(hir::Guard::IfLet(&hir::Pat { hir_id, .. }, _)), .. - }) if hir_id == pat_id => { + }) if Some(hir_id) == pat_id => { return LetSource::IfLetGuard; } hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Let(..), span, .. }) => { @@ -980,21 +1143,3 @@ fn let_source(tcx: TyCtxt<'_>, pat_id: HirId) -> LetSource { LetSource::GenericLet } - -// Since this function is called within a let context, it is reasonable to assume that any parent -// `&&` infers a let chain -fn is_let_chain(tcx: TyCtxt<'_>, pat_id: HirId) -> bool { - let hir = tcx.hir(); - let parent = hir.get_parent_node(pat_id); - let parent_parent = hir.get_parent_node(parent); - matches!( - hir.get(parent_parent), - hir::Node::Expr( - hir::Expr { - kind: hir::ExprKind::Binary(Spanned { node: hir::BinOpKind::And, .. }, ..), - .. - }, - .. - ) - ) -} diff --git a/src/test/ui/mir/mir_let_chains_drop_order.rs b/src/test/ui/mir/mir_let_chains_drop_order.rs index 01f943c87dd7f..6498a51957194 100644 --- a/src/test/ui/mir/mir_let_chains_drop_order.rs +++ b/src/test/ui/mir/mir_let_chains_drop_order.rs @@ -5,6 +5,7 @@ // See `mir_drop_order.rs` for more information #![feature(let_chains)] +#![allow(irrefutable_let_patterns)] use std::cell::RefCell; use std::panic; diff --git a/src/test/ui/rfc-2497-if-let-chains/ast-lowering-does-not-wrap-let-chains.rs b/src/test/ui/rfc-2497-if-let-chains/ast-lowering-does-not-wrap-let-chains.rs index 708bcdd0aefe3..d851fac8e644f 100644 --- a/src/test/ui/rfc-2497-if-let-chains/ast-lowering-does-not-wrap-let-chains.rs +++ b/src/test/ui/rfc-2497-if-let-chains/ast-lowering-does-not-wrap-let-chains.rs @@ -1,6 +1,7 @@ // run-pass #![feature(let_chains)] +#![allow(irrefutable_let_patterns)] fn main() { let first = Some(1); diff --git a/src/test/ui/rfc-2497-if-let-chains/irrefutable-lets.disallowed.stderr b/src/test/ui/rfc-2497-if-let-chains/irrefutable-lets.disallowed.stderr new file mode 100644 index 0000000000000..d1d5288aea31c --- /dev/null +++ b/src/test/ui/rfc-2497-if-let-chains/irrefutable-lets.disallowed.stderr @@ -0,0 +1,115 @@ +error: leading irrefutable pattern in let chain + --> $DIR/irrefutable-lets.rs:13:8 + | +LL | if let first = &opt && let Some(ref second) = first && let None = second.start {} + | ^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/irrefutable-lets.rs:6:30 + | +LL | #![cfg_attr(disallowed, deny(irrefutable_let_patterns))] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this pattern will always match + = help: consider moving it outside of the construct + +error: irrefutable `if let` patterns + --> $DIR/irrefutable-lets.rs:19:8 + | +LL | if let first = &opt && let (a, b) = (1, 2) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: these patterns will always match, so the `if let` is useless + = help: consider replacing the `if let` with a `let` + +error: leading irrefutable pattern in let chain + --> $DIR/irrefutable-lets.rs:22:8 + | +LL | if let first = &opt && let Some(ref second) = first && let None = second.start && let v = 0 {} + | ^^^^^^^^^^^^^^^^ + | + = note: this pattern will always match + = help: consider moving it outside of the construct + +error: trailing irrefutable pattern in let chain + --> $DIR/irrefutable-lets.rs:22:87 + | +LL | if let first = &opt && let Some(ref second) = first && let None = second.start && let v = 0 {} + | ^^^^^^^^^ + | + = note: this pattern will always match + = help: consider moving it into the body + +error: trailing irrefutable patterns in let chain + --> $DIR/irrefutable-lets.rs:26:37 + | +LL | if let Some(ref first) = opt && let second = first && let _third = second {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: these patterns will always match + = help: consider moving them into the body + +error: leading irrefutable pattern in let chain + --> $DIR/irrefutable-lets.rs:29:8 + | +LL | if let Range { start: local_start, end: _ } = (None..Some(1)) && let None = local_start {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this pattern will always match + = help: consider moving it outside of the construct + +error: leading irrefutable pattern in let chain + --> $DIR/irrefutable-lets.rs:32:8 + | +LL | if let (a, b, c) = (Some(1), Some(1), Some(1)) && let None = Some(1) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this pattern will always match + = help: consider moving it outside of the construct + +error: leading irrefutable pattern in let chain + --> $DIR/irrefutable-lets.rs:35:8 + | +LL | if let first = &opt && let None = Some(1) {} + | ^^^^^^^^^^^^^^^^ + | + = note: this pattern will always match + = help: consider moving it outside of the construct + +error: irrefutable `let` patterns + --> $DIR/irrefutable-lets.rs:44:28 + | +LL | Some(ref first) if let second = first && let _third = second && let v = 4 + 4 => {}, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: these patterns will always match, so the `let` is useless + = help: consider removing `let` + +error: leading irrefutable pattern in let chain + --> $DIR/irrefutable-lets.rs:50:28 + | +LL | Some(ref first) if let Range { start: local_start, end: _ } = first + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this pattern will always match + = help: consider moving it outside of the construct + +error: irrefutable `while let` patterns + --> $DIR/irrefutable-lets.rs:59:11 + | +LL | while let first = &opt && let (a, b) = (1, 2) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: these patterns will always match, so the loop will never exit + = help: consider instead using a `loop { ... }` with a `let` inside it + +error: trailing irrefutable patterns in let chain + --> $DIR/irrefutable-lets.rs:62:40 + | +LL | while let Some(ref first) = opt && let second = first && let _third = second {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: these patterns will always match + = help: consider moving them into the body + +error: aborting due to 12 previous errors + diff --git a/src/test/ui/rfc-2497-if-let-chains/irrefutable-lets.rs b/src/test/ui/rfc-2497-if-let-chains/irrefutable-lets.rs index 945c665e35d28..3d1626e8ffb9a 100644 --- a/src/test/ui/rfc-2497-if-let-chains/irrefutable-lets.rs +++ b/src/test/ui/rfc-2497-if-let-chains/irrefutable-lets.rs @@ -1,35 +1,67 @@ -// check-pass +// revisions: allowed disallowed +//[allowed] check-pass #![feature(if_let_guard, let_chains)] +#![cfg_attr(allowed, allow(irrefutable_let_patterns))] +#![cfg_attr(disallowed, deny(irrefutable_let_patterns))] use std::ops::Range; fn main() { let opt = Some(None..Some(1)); - if let first = &opt && let Some(ref second) = first && let None = second.start { - } - if let Some(ref first) = opt && let second = first && let _third = second { - } + if let first = &opt && let Some(ref second) = first && let None = second.start {} + //[disallowed]~^ ERROR leading irrefutable pattern in let chain + + // No lint as the irrefutable pattern is surrounded by other stuff + if 4 * 2 == 0 && let first = &opt && let Some(ref second) = first && let None = second.start {} + + if let first = &opt && let (a, b) = (1, 2) {} + //[disallowed]~^ ERROR irrefutable `if let` patterns + + if let first = &opt && let Some(ref second) = first && let None = second.start && let v = 0 {} + //[disallowed]~^ ERROR leading irrefutable pattern in let chain + //[disallowed]~^^ ERROR trailing irrefutable pattern in let chain + + if let Some(ref first) = opt && let second = first && let _third = second {} + //[disallowed]~^ ERROR trailing irrefutable patterns in let chain + + if let Range { start: local_start, end: _ } = (None..Some(1)) && let None = local_start {} + //[disallowed]~^ ERROR leading irrefutable pattern in let chain + + if let (a, b, c) = (Some(1), Some(1), Some(1)) && let None = Some(1) {} + //[disallowed]~^ ERROR leading irrefutable pattern in let chain + + if let first = &opt && let None = Some(1) {} + //[disallowed]~^ ERROR leading irrefutable pattern in let chain + if let Some(ref first) = opt && let Range { start: local_start, end: _ } = first && let None = local_start { } match opt { - Some(ref first) if let second = first && let _third = second => {}, + Some(ref first) if let second = first && let _third = second && let v = 4 + 4 => {}, + //[disallowed]~^ ERROR irrefutable `let` patterns _ => {} } + match opt { Some(ref first) if let Range { start: local_start, end: _ } = first + //[disallowed]~^ ERROR leading irrefutable pattern in let chain && let None = local_start => {}, _ => {} } - while let first = &opt && let Some(ref second) = first && let None = second.start { - } - while let Some(ref first) = opt && let second = first && let _third = second { - } + // No error, despite the prefix being irrefutable + while let first = &opt && let Some(ref second) = first && let None = second.start {} + + while let first = &opt && let (a, b) = (1, 2) {} + //[disallowed]~^ ERROR irrefutable `while let` patterns + + while let Some(ref first) = opt && let second = first && let _third = second {} + //[disallowed]~^ ERROR trailing irrefutable patterns in let chain + while let Some(ref first) = opt && let Range { start: local_start, end: _ } = first && let None = local_start { From ac5c657a0801db84b29ea9b3ae322107756575b0 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Tue, 15 Mar 2022 18:10:44 -0700 Subject: [PATCH 6/7] Bless coverage-reports after core macro blocks change --- .../coverage-reports/expected_show_coverage.closure.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure.txt index e463099a5ee47..8bf9c97fddad4 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure.txt @@ -116,8 +116,8 @@ 116| 1| 117| 1| let 118| 1| _unused_closure - 119| | = - 120| | | + 119| 1| = + 120| 1| | 121| | mut countdown 122| | | 123| 0| { @@ -173,7 +173,7 @@ 169| | ; 170| | 171| 1| let short_used_not_covered_closure_line_break_no_block_embedded_branch = - 172| | | _unused_arg: u8 | + 172| 1| | _unused_arg: u8 | 173| 0| println!( 174| 0| "not called: {}", 175| 0| if is_true { "check" } else { "me" } @@ -191,7 +191,7 @@ 187| | ; 188| | 189| 1| let short_used_covered_closure_line_break_no_block_embedded_branch = - 190| 1| | _unused_arg: u8 | + 190| | | _unused_arg: u8 | 191| 1| println!( 192| 1| "not called: {}", 193| 1| if is_true { "check" } else { "me" } From 2c06c861de2f9674719cd1e5eac571efc359d8df Mon Sep 17 00:00:00 2001 From: Dylan DPC <99973273+Dylan-DPC@users.noreply.github.com> Date: Wed, 16 Mar 2022 03:04:40 +0100 Subject: [PATCH 7/7] changed wording --- library/std/src/io/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index e53e1f4930935..cd2197fca350e 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2110,7 +2110,8 @@ pub trait BufRead: Read { } /// Read all bytes until a newline (the `0xA` byte) is reached, and append - /// them to the provided buffer (without clear buffer before appending to it). + /// them to the provided buffer. You do not need to clear the buffer before + /// appending. /// /// This function will read bytes from the underlying stream until the /// newline delimiter (the `0xA` byte) or EOF is found. Once found, all bytes