From c027844795e427e63ef917ba40c71d0559d88b79 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Tue, 6 Oct 2020 14:06:25 +0100 Subject: [PATCH 01/29] Fill in things needed to stabilize int_error_matching --- compiler/rustc_middle/src/lib.rs | 1 - compiler/rustc_middle/src/middle/limits.rs | 8 +-- library/core/src/num/error.rs | 33 ++++++------ library/core/src/num/mod.rs | 12 ++--- library/core/tests/lib.rs | 1 - library/core/tests/nonzero.rs | 4 +- library/core/tests/num/mod.rs | 63 +++++++++++++--------- library/std/src/lib.rs | 1 - library/std/src/num.rs | 7 +-- 9 files changed, 66 insertions(+), 64 deletions(-) diff --git a/compiler/rustc_middle/src/lib.rs b/compiler/rustc_middle/src/lib.rs index fa885ce2e7cdf..37bc1a305b2f8 100644 --- a/compiler/rustc_middle/src/lib.rs +++ b/compiler/rustc_middle/src/lib.rs @@ -46,7 +46,6 @@ #![feature(crate_visibility_modifier)] #![feature(associated_type_bounds)] #![feature(rustc_attrs)] -#![feature(int_error_matching)] #![recursion_limit = "512"] #[macro_use] diff --git a/compiler/rustc_middle/src/middle/limits.rs b/compiler/rustc_middle/src/middle/limits.rs index def9e5ebb527f..6b6df3a303c22 100644 --- a/compiler/rustc_middle/src/middle/limits.rs +++ b/compiler/rustc_middle/src/middle/limits.rs @@ -48,10 +48,12 @@ fn update_limit( .unwrap_or(attr.span); let error_str = match e.kind() { - IntErrorKind::Overflow => "`limit` is too large", - IntErrorKind::Empty => "`limit` must be a non-negative integer", + IntErrorKind::PosOverflow => "`limit` is too large", + IntErrorKind::Empty | IntErrorKind::OnlySign => { + "`limit` must be a non-negative integer" + } IntErrorKind::InvalidDigit => "not a valid integer", - IntErrorKind::Underflow => bug!("`limit` should never underflow"), + IntErrorKind::NegOverflow => bug!("`limit` should never underflow"), IntErrorKind::Zero => bug!("zero is a valid `limit`"), kind => bug!("unimplemented IntErrorKind variant: {:?}", kind), }; diff --git a/library/core/src/num/error.rs b/library/core/src/num/error.rs index aab1715518611..9705226ba243a 100644 --- a/library/core/src/num/error.rs +++ b/library/core/src/num/error.rs @@ -77,51 +77,47 @@ pub struct ParseIntError { /// # Example /// /// ``` -/// #![feature(int_error_matching)] -/// /// # fn main() { /// if let Err(e) = i32::from_str_radix("a12", 10) { /// println!("Failed conversion to i32: {:?}", e.kind()); /// } /// # } /// ``` -#[unstable( - feature = "int_error_matching", - reason = "it can be useful to match errors when making error messages \ - for integer parsing", - issue = "22639" -)] +#[stable(feature = "int_error_matching", since = "1.47.0")] #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] pub enum IntErrorKind { /// Value being parsed is empty. /// /// Among other causes, this variant will be constructed when parsing an empty string. + #[stable(feature = "int_error_matching", since = "1.47.0")] Empty, /// Contains an invalid digit. /// /// Among other causes, this variant will be constructed when parsing a string that /// contains a letter. + #[stable(feature = "int_error_matching", since = "1.47.0")] InvalidDigit, /// Integer is too large to store in target integer type. - Overflow, + #[stable(feature = "int_error_matching", since = "1.47.0")] + PosOverflow, /// Integer is too small to store in target integer type. - Underflow, + #[stable(feature = "int_error_matching", since = "1.47.0")] + NegOverflow, /// Value was Zero /// /// This variant will be emitted when the parsing string has a value of zero, which /// would be illegal for non-zero types. + #[stable(feature = "int_error_matching", since = "1.47.0")] Zero, + /// The value contains nothing other than signs `+` or `-`. + #[stable(feature = "int_error_matching", since = "1.47.0")] + OnlySign, } impl ParseIntError { /// Outputs the detailed cause of parsing an integer failing. - #[unstable( - feature = "int_error_matching", - reason = "it can be useful to match errors when making error messages \ - for integer parsing", - issue = "22639" - )] + #[stable(feature = "int_error_matching", since = "1.47.0")] pub fn kind(&self) -> &IntErrorKind { &self.kind } @@ -136,9 +132,10 @@ impl ParseIntError { match self.kind { IntErrorKind::Empty => "cannot parse integer from empty string", IntErrorKind::InvalidDigit => "invalid digit found in string", - IntErrorKind::Overflow => "number too large to fit in target type", - IntErrorKind::Underflow => "number too small to fit in target type", + IntErrorKind::PosOverflow => "number too large to fit in target type", + IntErrorKind::NegOverflow => "number too small to fit in target type", IntErrorKind::Zero => "number would be zero for non-zero type", + IntErrorKind::OnlySign => "only signs without digits found in string", } } } diff --git a/library/core/src/num/mod.rs b/library/core/src/num/mod.rs index 4f64e30ccf84a..67b4b885dd2ec 100644 --- a/library/core/src/num/mod.rs +++ b/library/core/src/num/mod.rs @@ -63,7 +63,7 @@ pub use nonzero::{NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, No #[stable(feature = "try_from", since = "1.34.0")] pub use error::TryFromIntError; -#[unstable(feature = "int_error_matching", issue = "22639")] +#[stable(feature = "int_error_matching", since = "1.47.0")] pub use error::IntErrorKind; macro_rules! usize_isize_to_xe_bytes_doc { @@ -836,7 +836,7 @@ fn from_str_radix(src: &str, radix: u32) -> Result(src: &str, radix: u32) -> Result result, - None => return Err(PIE { kind: Overflow }), + None => return Err(PIE { kind: PosOverflow }), }; result = match result.checked_add(x) { Some(result) => result, - None => return Err(PIE { kind: Overflow }), + None => return Err(PIE { kind: PosOverflow }), }; } } else { @@ -865,11 +865,11 @@ fn from_str_radix(src: &str, radix: u32) -> Result result, - None => return Err(PIE { kind: Underflow }), + None => return Err(PIE { kind: NegOverflow }), }; result = match result.checked_sub(x) { Some(result) => result, - None => return Err(PIE { kind: Underflow }), + None => return Err(PIE { kind: NegOverflow }), }; } } diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index d8b36beb3e085..c128691fa7525 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -37,7 +37,6 @@ #![feature(try_trait)] #![feature(slice_internals)] #![feature(slice_partition_dedup)] -#![feature(int_error_matching)] #![feature(array_value_iter)] #![feature(iter_partition_in_place)] #![feature(iter_is_partitioned)] diff --git a/library/core/tests/nonzero.rs b/library/core/tests/nonzero.rs index 825e5e63b59bc..fb1293c99bba9 100644 --- a/library/core/tests/nonzero.rs +++ b/library/core/tests/nonzero.rs @@ -135,11 +135,11 @@ fn test_from_str() { ); assert_eq!( "-129".parse::().err().map(|e| e.kind().clone()), - Some(IntErrorKind::Underflow) + Some(IntErrorKind::NegOverflow) ); assert_eq!( "257".parse::().err().map(|e| e.kind().clone()), - Some(IntErrorKind::Overflow) + Some(IntErrorKind::PosOverflow) ); } diff --git a/library/core/tests/num/mod.rs b/library/core/tests/num/mod.rs index 939f1325c8499..d6f92f25e7846 100644 --- a/library/core/tests/num/mod.rs +++ b/library/core/tests/num/mod.rs @@ -2,10 +2,11 @@ use core::cmp::PartialEq; use core::convert::{TryFrom, TryInto}; use core::fmt::Debug; use core::marker::Copy; -use core::num::TryFromIntError; +use core::num::{IntErrorKind, ParseIntError, TryFromIntError}; use core::ops::{Add, Div, Mul, Rem, Sub}; use core::option::Option; -use core::option::Option::{None, Some}; +use core::option::Option::None; +use core::str::FromStr; #[macro_use] mod int_macros; @@ -65,6 +66,14 @@ where assert_eq!(ten.rem(two), ten % two); } +fn test_parse(num_str: &str, expected: Result) +where + T: FromStr, + Result: PartialEq + Debug, +{ + assert_eq!(num_str.parse::().map_err(|e| e.kind().clone()), expected) +} + #[test] fn from_str_issue7588() { let u: Option = u8::from_str_radix("1000", 10).ok(); @@ -75,49 +84,51 @@ fn from_str_issue7588() { #[test] fn test_int_from_str_overflow() { - assert_eq!("127".parse::().ok(), Some(127i8)); - assert_eq!("128".parse::().ok(), None); + test_parse::("127", Ok(127)); + test_parse::("128", Err(IntErrorKind::PosOverflow)); - assert_eq!("-128".parse::().ok(), Some(-128i8)); - assert_eq!("-129".parse::().ok(), None); + test_parse::("-128", Ok(-128)); + test_parse::("-129", Err(IntErrorKind::NegOverflow)); - assert_eq!("32767".parse::().ok(), Some(32_767i16)); - assert_eq!("32768".parse::().ok(), None); + test_parse::("32767", Ok(32_767)); + test_parse::("32768", Err(IntErrorKind::PosOverflow)); - assert_eq!("-32768".parse::().ok(), Some(-32_768i16)); - assert_eq!("-32769".parse::().ok(), None); + test_parse::("-32768", Ok(-32_768)); + test_parse::("-32769", Err(IntErrorKind::NegOverflow)); - assert_eq!("2147483647".parse::().ok(), Some(2_147_483_647i32)); - assert_eq!("2147483648".parse::().ok(), None); + test_parse::("2147483647", Ok(2_147_483_647)); + test_parse::("2147483648", Err(IntErrorKind::PosOverflow)); - assert_eq!("-2147483648".parse::().ok(), Some(-2_147_483_648i32)); - assert_eq!("-2147483649".parse::().ok(), None); + test_parse::("-2147483648", Ok(-2_147_483_648)); + test_parse::("-2147483649", Err(IntErrorKind::NegOverflow)); - assert_eq!("9223372036854775807".parse::().ok(), Some(9_223_372_036_854_775_807i64)); - assert_eq!("9223372036854775808".parse::().ok(), None); + test_parse::("9223372036854775807", Ok(9_223_372_036_854_775_807)); + test_parse::("9223372036854775808", Err(IntErrorKind::PosOverflow)); - assert_eq!("-9223372036854775808".parse::().ok(), Some(-9_223_372_036_854_775_808i64)); - assert_eq!("-9223372036854775809".parse::().ok(), None); + test_parse::("-9223372036854775808", Ok(-9_223_372_036_854_775_808)); + test_parse::("-9223372036854775809", Err(IntErrorKind::NegOverflow)); } #[test] fn test_leading_plus() { - assert_eq!("+127".parse::().ok(), Some(127)); - assert_eq!("+9223372036854775807".parse::().ok(), Some(9223372036854775807)); + test_parse::("+127", Ok(127)); + test_parse::("+9223372036854775807", Ok(9223372036854775807)); } #[test] fn test_invalid() { - assert_eq!("--129".parse::().ok(), None); - assert_eq!("++129".parse::().ok(), None); - assert_eq!("Съешь".parse::().ok(), None); + test_parse::("--129", Err(IntErrorKind::InvalidDigit)); + test_parse::("++129", Err(IntErrorKind::InvalidDigit)); + test_parse::("Съешь", Err(IntErrorKind::InvalidDigit)); + // is this the correct error here. Maybe need a reapeat sign error here + test_parse::("--", Err(IntErrorKind::InvalidDigit)); } #[test] fn test_empty() { - assert_eq!("-".parse::().ok(), None); - assert_eq!("+".parse::().ok(), None); - assert_eq!("".parse::().ok(), None); + test_parse::("-", Err(IntErrorKind::OnlySign)); + test_parse::("+", Err(IntErrorKind::OnlySign)); + test_parse::("", Err(IntErrorKind::Empty)); } #[test] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index ac0075ad129c5..fa23229066cf1 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -264,7 +264,6 @@ #![feature(global_asm)] #![feature(hashmap_internals)] #![feature(int_error_internals)] -#![feature(int_error_matching)] #![feature(integer_atomics)] #![feature(into_future)] #![feature(lang_items)] diff --git a/library/std/src/num.rs b/library/std/src/num.rs index 0f1c596268594..ac3b055cdb050 100644 --- a/library/std/src/num.rs +++ b/library/std/src/num.rs @@ -22,12 +22,7 @@ pub use core::num::{NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, #[stable(feature = "nonzero", since = "1.28.0")] pub use core::num::{NonZeroU128, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize}; -#[unstable( - feature = "int_error_matching", - reason = "it can be useful to match errors when making error messages \ - for integer parsing", - issue = "22639" -)] +#[stable(feature = "int_error_matching", since = "1.47.0")] pub use core::num::IntErrorKind; #[cfg(test)] From 83d294f06a8f78f4956333a5285bb2c4f7b8a6a9 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Tue, 6 Oct 2020 19:05:25 +0100 Subject: [PATCH 02/29] Bring char along with InvalidDigit --- compiler/rustc_middle/src/middle/limits.rs | 2 +- library/core/src/num/error.rs | 6 +++--- library/core/src/num/mod.rs | 4 ++-- library/core/tests/nonzero.rs | 2 +- library/core/tests/num/mod.rs | 10 +++++----- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_middle/src/middle/limits.rs b/compiler/rustc_middle/src/middle/limits.rs index 6b6df3a303c22..f03f439f73b5a 100644 --- a/compiler/rustc_middle/src/middle/limits.rs +++ b/compiler/rustc_middle/src/middle/limits.rs @@ -52,7 +52,7 @@ fn update_limit( IntErrorKind::Empty | IntErrorKind::OnlySign => { "`limit` must be a non-negative integer" } - IntErrorKind::InvalidDigit => "not a valid integer", + IntErrorKind::InvalidDigit(_) => "not a valid integer", IntErrorKind::NegOverflow => bug!("`limit` should never underflow"), IntErrorKind::Zero => bug!("zero is a valid `limit`"), kind => bug!("unimplemented IntErrorKind variant: {:?}", kind), diff --git a/library/core/src/num/error.rs b/library/core/src/num/error.rs index 9705226ba243a..ba7c94656ce3f 100644 --- a/library/core/src/num/error.rs +++ b/library/core/src/num/error.rs @@ -92,12 +92,12 @@ pub enum IntErrorKind { /// Among other causes, this variant will be constructed when parsing an empty string. #[stable(feature = "int_error_matching", since = "1.47.0")] Empty, - /// Contains an invalid digit. + /// Contains an digit invalid in its context. /// /// Among other causes, this variant will be constructed when parsing a string that /// contains a letter. #[stable(feature = "int_error_matching", since = "1.47.0")] - InvalidDigit, + InvalidDigit(#[stable(feature = "int_error_matching", since = "1.47.0")] char), /// Integer is too large to store in target integer type. #[stable(feature = "int_error_matching", since = "1.47.0")] PosOverflow, @@ -131,7 +131,7 @@ impl ParseIntError { pub fn __description(&self) -> &str { match self.kind { IntErrorKind::Empty => "cannot parse integer from empty string", - IntErrorKind::InvalidDigit => "invalid digit found in string", + IntErrorKind::InvalidDigit(_) => "invalid digit found in string", IntErrorKind::PosOverflow => "number too large to fit in target type", IntErrorKind::NegOverflow => "number too small to fit in target type", IntErrorKind::Zero => "number would be zero for non-zero type", diff --git a/library/core/src/num/mod.rs b/library/core/src/num/mod.rs index 67b4b885dd2ec..a438f3161a3af 100644 --- a/library/core/src/num/mod.rs +++ b/library/core/src/num/mod.rs @@ -845,7 +845,7 @@ fn from_str_radix(src: &str, radix: u32) -> Result x, - None => return Err(PIE { kind: InvalidDigit }), + None => return Err(PIE { kind: InvalidDigit(c as char) }), }; result = match result.checked_mul(radix) { Some(result) => result, @@ -861,7 +861,7 @@ fn from_str_radix(src: &str, radix: u32) -> Result x, - None => return Err(PIE { kind: InvalidDigit }), + None => return Err(PIE { kind: InvalidDigit(c as char) }), }; result = match result.checked_mul(radix) { Some(result) => result, diff --git a/library/core/tests/nonzero.rs b/library/core/tests/nonzero.rs index fb1293c99bba9..949d4ea32f064 100644 --- a/library/core/tests/nonzero.rs +++ b/library/core/tests/nonzero.rs @@ -131,7 +131,7 @@ fn test_from_str() { assert_eq!("0".parse::().err().map(|e| e.kind().clone()), Some(IntErrorKind::Zero)); assert_eq!( "-1".parse::().err().map(|e| e.kind().clone()), - Some(IntErrorKind::InvalidDigit) + Some(IntErrorKind::InvalidDigit('-')) ); assert_eq!( "-129".parse::().err().map(|e| e.kind().clone()), diff --git a/library/core/tests/num/mod.rs b/library/core/tests/num/mod.rs index d6f92f25e7846..a93cd38160b58 100644 --- a/library/core/tests/num/mod.rs +++ b/library/core/tests/num/mod.rs @@ -117,11 +117,11 @@ fn test_leading_plus() { #[test] fn test_invalid() { - test_parse::("--129", Err(IntErrorKind::InvalidDigit)); - test_parse::("++129", Err(IntErrorKind::InvalidDigit)); - test_parse::("Съешь", Err(IntErrorKind::InvalidDigit)); - // is this the correct error here. Maybe need a reapeat sign error here - test_parse::("--", Err(IntErrorKind::InvalidDigit)); + test_parse::("--129", Err(IntErrorKind::InvalidDigit('-'))); + test_parse::("++129", Err(IntErrorKind::InvalidDigit('+'))); + test_parse::("Съешь", Err(IntErrorKind::InvalidDigit('Ð'))); + test_parse::("123Hello", Err(IntErrorKind::InvalidDigit('H'))); + test_parse::("--", Err(IntErrorKind::InvalidDigit('-'))); } #[test] From 8eaf0de1f45924a0fdbde00d4c7fe0333b377993 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Tue, 6 Oct 2020 21:03:10 +0100 Subject: [PATCH 03/29] Remove incorrect plural --- library/core/src/num/error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/num/error.rs b/library/core/src/num/error.rs index ba7c94656ce3f..a8f8a7804fd78 100644 --- a/library/core/src/num/error.rs +++ b/library/core/src/num/error.rs @@ -110,7 +110,7 @@ pub enum IntErrorKind { /// would be illegal for non-zero types. #[stable(feature = "int_error_matching", since = "1.47.0")] Zero, - /// The value contains nothing other than signs `+` or `-`. + /// The value contains nothing other than sign `+` or `-`. #[stable(feature = "int_error_matching", since = "1.47.0")] OnlySign, } @@ -135,7 +135,7 @@ impl ParseIntError { IntErrorKind::PosOverflow => "number too large to fit in target type", IntErrorKind::NegOverflow => "number too small to fit in target type", IntErrorKind::Zero => "number would be zero for non-zero type", - IntErrorKind::OnlySign => "only signs without digits found in string", + IntErrorKind::OnlySign => "only sign without digits found in string", } } } From 1e7e2e40e4992a82b9e5bc7120bd33964bcef355 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Tue, 6 Oct 2020 22:42:33 +0100 Subject: [PATCH 04/29] remove OnlySign in favour of InvalidDigit --- compiler/rustc_middle/src/middle/limits.rs | 4 +--- library/core/src/num/error.rs | 9 ++++----- library/core/src/num/mod.rs | 7 +++---- library/core/tests/num/mod.rs | 5 +++-- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_middle/src/middle/limits.rs b/compiler/rustc_middle/src/middle/limits.rs index f03f439f73b5a..e0d171fa77125 100644 --- a/compiler/rustc_middle/src/middle/limits.rs +++ b/compiler/rustc_middle/src/middle/limits.rs @@ -49,9 +49,7 @@ fn update_limit( let error_str = match e.kind() { IntErrorKind::PosOverflow => "`limit` is too large", - IntErrorKind::Empty | IntErrorKind::OnlySign => { - "`limit` must be a non-negative integer" - } + IntErrorKind::Empty => "`limit` must be a non-negative integer", IntErrorKind::InvalidDigit(_) => "not a valid integer", IntErrorKind::NegOverflow => bug!("`limit` should never underflow"), IntErrorKind::Zero => bug!("zero is a valid `limit`"), diff --git a/library/core/src/num/error.rs b/library/core/src/num/error.rs index a8f8a7804fd78..401d52eb08481 100644 --- a/library/core/src/num/error.rs +++ b/library/core/src/num/error.rs @@ -95,7 +95,10 @@ pub enum IntErrorKind { /// Contains an digit invalid in its context. /// /// Among other causes, this variant will be constructed when parsing a string that - /// contains a letter. + /// contains a non-asci char. + /// + /// This variant is also constructed when a `+` or `-` is misplaced within a sting + /// either on its own or in the middle of a number. #[stable(feature = "int_error_matching", since = "1.47.0")] InvalidDigit(#[stable(feature = "int_error_matching", since = "1.47.0")] char), /// Integer is too large to store in target integer type. @@ -110,9 +113,6 @@ pub enum IntErrorKind { /// would be illegal for non-zero types. #[stable(feature = "int_error_matching", since = "1.47.0")] Zero, - /// The value contains nothing other than sign `+` or `-`. - #[stable(feature = "int_error_matching", since = "1.47.0")] - OnlySign, } impl ParseIntError { @@ -135,7 +135,6 @@ impl ParseIntError { IntErrorKind::PosOverflow => "number too large to fit in target type", IntErrorKind::NegOverflow => "number too small to fit in target type", IntErrorKind::Zero => "number would be zero for non-zero type", - IntErrorKind::OnlySign => "only sign without digits found in string", } } } diff --git a/library/core/src/num/mod.rs b/library/core/src/num/mod.rs index a438f3161a3af..fd00a072d896c 100644 --- a/library/core/src/num/mod.rs +++ b/library/core/src/num/mod.rs @@ -830,15 +830,14 @@ fn from_str_radix(src: &str, radix: u32) -> Result { + return Err(PIE { kind: InvalidDigit(src[0] as char) }); + } b'+' => (true, &src[1..]), b'-' if is_signed_ty => (false, &src[1..]), _ => (true, src), }; - if digits.is_empty() { - return Err(PIE { kind: OnlySign }); - } - let mut result = T::from_u32(0); if is_positive { // The number is positive diff --git a/library/core/tests/num/mod.rs b/library/core/tests/num/mod.rs index a93cd38160b58..4fd9f721b823b 100644 --- a/library/core/tests/num/mod.rs +++ b/library/core/tests/num/mod.rs @@ -122,12 +122,13 @@ fn test_invalid() { test_parse::("Съешь", Err(IntErrorKind::InvalidDigit('Ð'))); test_parse::("123Hello", Err(IntErrorKind::InvalidDigit('H'))); test_parse::("--", Err(IntErrorKind::InvalidDigit('-'))); + test_parse::("-", Err(IntErrorKind::InvalidDigit('-'))); + test_parse::("+", Err(IntErrorKind::InvalidDigit('+'))); + test_parse::("-1", Err(IntErrorKind::InvalidDigit('-'))); } #[test] fn test_empty() { - test_parse::("-", Err(IntErrorKind::OnlySign)); - test_parse::("+", Err(IntErrorKind::OnlySign)); test_parse::("", Err(IntErrorKind::Empty)); } From f233abb9091e3c0999f0cb80ba652d6094e3d5b4 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Wed, 7 Oct 2020 08:02:36 +0100 Subject: [PATCH 05/29] Add comment to helper function --- library/core/tests/num/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/tests/num/mod.rs b/library/core/tests/num/mod.rs index 4fd9f721b823b..12e52252278c5 100644 --- a/library/core/tests/num/mod.rs +++ b/library/core/tests/num/mod.rs @@ -66,6 +66,7 @@ where assert_eq!(ten.rem(two), ten % two); } +/// Helper function for asserting number parsing returns a specific error fn test_parse(num_str: &str, expected: Result) where T: FromStr, From 91a9f83dd1d73cfd451f81306361df3fafad84a5 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 16 Oct 2020 09:09:20 -0700 Subject: [PATCH 06/29] Define `fs::hard_link` to not follow symlinks. POSIX leaves it implementation-defined whether `link` follows symlinks. In practice, for example, on Linux it does not and on FreeBSD it does. So, switch to `linkat`, so that we can pick a behavior rather than depending on OS defaults. Pick the option to not follow symlinks. This is somewhat arbitrary, but seems the less surprising choice because hard linking is a very low-level feature which requires the source and destination to be on the same mounted filesystem, and following a symbolic link could end up in a different mounted filesystem. --- library/std/src/fs.rs | 7 +++-- library/std/src/fs/tests.rs | 51 ++++++++++++++++++++++++++++++++++ library/std/src/sys/unix/fs.rs | 5 +++- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 161bfe3795c2c..c611bf4d74a49 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1701,10 +1701,13 @@ pub fn copy, Q: AsRef>(from: P, to: Q) -> io::Result { /// The `dst` path will be a link pointing to the `src` path. Note that systems /// often require these two paths to both be located on the same filesystem. /// +/// If `src` names a symbolic link, it is not followed. The created hard link +/// points to the symbolic link itself. +/// /// # Platform-specific behavior /// -/// This function currently corresponds to the `link` function on Unix -/// and the `CreateHardLink` function on Windows. +/// This function currently corresponds to the `linkat` function with no flags +/// on Unix and the `CreateHardLink` function on Windows. /// Note that, this [may change in the future][changes]. /// /// [changes]: io#platform-specific-behavior diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index 65a29076fefa8..8a723d3b4ae22 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -1337,3 +1337,54 @@ fn metadata_access_times() { } } } + +/// Test creating hard links to symlinks. +#[test] +fn symlink_hard_link() { + let tmpdir = tmpdir(); + + // Create "file", a file. + check!(fs::File::create(tmpdir.join("file"))); + + // Create "symlink", a symlink to "file". + check!(symlink_file("file", tmpdir.join("symlink"))); + + // Create "hard_link", a hard link to "symlink". + check!(fs::hard_link(tmpdir.join("symlink"), tmpdir.join("hard_link"))); + + // "hard_link" should appear as a symlink. + assert!(check!(fs::symlink_metadata(tmpdir.join("hard_link"))).file_type().is_symlink()); + + // We sould be able to open "file" via any of the above names. + let _ = check!(fs::File::open(tmpdir.join("file"))); + assert!(fs::File::open(tmpdir.join("file.renamed")).is_err()); + let _ = check!(fs::File::open(tmpdir.join("symlink"))); + let _ = check!(fs::File::open(tmpdir.join("hard_link"))); + + // Rename "file" to "file.renamed". + check!(fs::rename(tmpdir.join("file"), tmpdir.join("file.renamed"))); + + // Now, the symlink and the hard link should be dangling. + assert!(fs::File::open(tmpdir.join("file")).is_err()); + let _ = check!(fs::File::open(tmpdir.join("file.renamed"))); + assert!(fs::File::open(tmpdir.join("symlink")).is_err()); + assert!(fs::File::open(tmpdir.join("hard_link")).is_err()); + + // The symlink and the hard link should both still point to "file". + assert!(fs::read_link(tmpdir.join("file")).is_err()); + assert!(fs::read_link(tmpdir.join("file.renamed")).is_err()); + assert_eq!(check!(fs::read_link(tmpdir.join("symlink"))), Path::new("file")); + assert_eq!(check!(fs::read_link(tmpdir.join("hard_link"))), Path::new("file")); + + // Remove "file.renamed". + check!(fs::remove_file(tmpdir.join("file.renamed"))); + + // Now, we can't open the file by any name. + assert!(fs::File::open(tmpdir.join("file")).is_err()); + assert!(fs::File::open(tmpdir.join("file.renamed")).is_err()); + assert!(fs::File::open(tmpdir.join("symlink")).is_err()); + assert!(fs::File::open(tmpdir.join("hard_link")).is_err()); + + // "hard_link" should still appear as a symlink. + assert!(check!(fs::symlink_metadata(tmpdir.join("hard_link"))).file_type().is_symlink()); +} diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 819e8ef18415b..88693e4786c0f 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1067,7 +1067,10 @@ pub fn symlink(src: &Path, dst: &Path) -> io::Result<()> { pub fn link(src: &Path, dst: &Path) -> io::Result<()> { let src = cstr(src)?; let dst = cstr(dst)?; - cvt(unsafe { libc::link(src.as_ptr(), dst.as_ptr()) })?; + // Use `linkat` with `AT_FDCWD` instead of `link` as `link` leaves it + // implmentation-defined whether it follows symlinks. Pass 0 as the + // `linkat` flags argument so that we don't follow symlinks. + cvt(unsafe { libc::linkat(libc::AT_FDCWD, src.as_ptr(), libc::AT_FDCWD, dst.as_ptr(), 0) })?; Ok(()) } From 23a5c214150f462043ab411f87ef297309421d71 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 19 Oct 2020 07:21:41 -0700 Subject: [PATCH 07/29] Fix a typo in a comment. --- library/std/src/sys/unix/fs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 88693e4786c0f..69f6b88a3bc56 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1068,7 +1068,7 @@ pub fn link(src: &Path, dst: &Path) -> io::Result<()> { let src = cstr(src)?; let dst = cstr(dst)?; // Use `linkat` with `AT_FDCWD` instead of `link` as `link` leaves it - // implmentation-defined whether it follows symlinks. Pass 0 as the + // implementation-defined whether it follows symlinks. Pass 0 as the // `linkat` flags argument so that we don't follow symlinks. cvt(unsafe { libc::linkat(libc::AT_FDCWD, src.as_ptr(), libc::AT_FDCWD, dst.as_ptr(), 0) })?; Ok(()) From ce00b3e2e0c5c0c88fe59fb45a1c25a8ff9e1836 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 19 Oct 2020 07:47:32 -0700 Subject: [PATCH 08/29] Use `link` on platforms which lack `linkat`. --- library/std/src/sys/unix/fs.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 69f6b88a3bc56..bf4c941928719 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1067,10 +1067,20 @@ pub fn symlink(src: &Path, dst: &Path) -> io::Result<()> { pub fn link(src: &Path, dst: &Path) -> io::Result<()> { let src = cstr(src)?; let dst = cstr(dst)?; - // Use `linkat` with `AT_FDCWD` instead of `link` as `link` leaves it - // implementation-defined whether it follows symlinks. Pass 0 as the - // `linkat` flags argument so that we don't follow symlinks. - cvt(unsafe { libc::linkat(libc::AT_FDCWD, src.as_ptr(), libc::AT_FDCWD, dst.as_ptr(), 0) })?; + cfg_if::cfg_if! { + if #[cfg(any(target_os = "vxworks", target_os = "redox"))] { + // VxWorks and Redox lack `linkat`, so use `link` instead. POSIX + // leaves it implementation-defined whether `link` follows symlinks, + // so rely on the `symlink_hard_link` test in + // library/std/src/fs/tests.rs to check the behavior. + cvt(unsafe { libc::link(src.as_ptr(), dst.as_ptr()) })?; + } else { + // Use `linkat` with `AT_FDCWD` instead of `link` as `linkat` gives + // us a flag to specify how symlinks should be handled. Pass 0 as + // the flags argument, meaning don't follow symlinks. + cvt(unsafe { libc::linkat(libc::AT_FDCWD, src.as_ptr(), libc::AT_FDCWD, dst.as_ptr(), 0) })?; + } + } Ok(()) } From d0178b4f99c70d2443f9b76421429d0d23dadc45 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 20 Oct 2020 16:42:31 -0700 Subject: [PATCH 09/29] Make it platform-specific whether `hard_link` follows symlinks. Also mention that where possible, `hard_link` does not follow symlinks. --- library/std/src/fs.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index c611bf4d74a49..c256f556b3c8f 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1701,8 +1701,9 @@ pub fn copy, Q: AsRef>(from: P, to: Q) -> io::Result { /// The `dst` path will be a link pointing to the `src` path. Note that systems /// often require these two paths to both be located on the same filesystem. /// -/// If `src` names a symbolic link, it is not followed. The created hard link -/// points to the symbolic link itself. +/// If `src` names a symbolic link, it is platform-specific whether the symbolic +/// link is followed. On platforms where it's possible to not follow it, it is +/// not followed, and the created hard link points to the symbolic link itself. /// /// # Platform-specific behavior /// From 6249cda78f0cd32b60fb11702b7ffef3e3bab0b2 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 23 Oct 2020 15:39:02 -0700 Subject: [PATCH 10/29] Disable use of `linkat` on Android as well. According to [the bionic status page], `linkat` has only been available since API level 21. Since Android is based on Linux and Linux's `link` doesn't follow symlinks, just use `link` on Android. [the bionic status page]: https://android.googlesource.com/platform/bionic/+/master/docs/status.md --- library/std/src/sys/unix/fs.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index bf4c941928719..ec721fccaa622 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1068,11 +1068,11 @@ pub fn link(src: &Path, dst: &Path) -> io::Result<()> { let src = cstr(src)?; let dst = cstr(dst)?; cfg_if::cfg_if! { - if #[cfg(any(target_os = "vxworks", target_os = "redox"))] { - // VxWorks and Redox lack `linkat`, so use `link` instead. POSIX - // leaves it implementation-defined whether `link` follows symlinks, - // so rely on the `symlink_hard_link` test in - // library/std/src/fs/tests.rs to check the behavior. + if #[cfg(any(target_os = "vxworks", target_os = "redox", target_os = "android"))] { + // VxWorks, Redox, and old versions of Android lack `linkat`, so use + // `link` instead. POSIX leaves it implementation-defined whether + // `link` follows symlinks, so rely on the `symlink_hard_link` test + // in library/std/src/fs/tests.rs to check the behavior. cvt(unsafe { libc::link(src.as_ptr(), dst.as_ptr()) })?; } else { // Use `linkat` with `AT_FDCWD` instead of `link` as `linkat` gives From 199c36115f1daa71458e94db0f37ab213d01eb8a Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Mon, 26 Oct 2020 05:50:28 -0500 Subject: [PATCH 11/29] Fix spelling eror Co-authored-by: Ashley Mannix --- library/core/src/num/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/num/error.rs b/library/core/src/num/error.rs index 401d52eb08481..91a24e7740ad4 100644 --- a/library/core/src/num/error.rs +++ b/library/core/src/num/error.rs @@ -97,7 +97,7 @@ pub enum IntErrorKind { /// Among other causes, this variant will be constructed when parsing a string that /// contains a non-asci char. /// - /// This variant is also constructed when a `+` or `-` is misplaced within a sting + /// This variant is also constructed when a `+` or `-` is misplaced within a string /// either on its own or in the middle of a number. #[stable(feature = "int_error_matching", since = "1.47.0")] InvalidDigit(#[stable(feature = "int_error_matching", since = "1.47.0")] char), From 69c301f0f36696f93a90bb7a6afe9081b0d04233 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Mon, 26 Oct 2020 05:51:07 -0500 Subject: [PATCH 12/29] Small reword Co-authored-by: Ashley Mannix --- library/core/src/num/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/num/error.rs b/library/core/src/num/error.rs index 91a24e7740ad4..d1a65aa35ae25 100644 --- a/library/core/src/num/error.rs +++ b/library/core/src/num/error.rs @@ -92,7 +92,7 @@ pub enum IntErrorKind { /// Among other causes, this variant will be constructed when parsing an empty string. #[stable(feature = "int_error_matching", since = "1.47.0")] Empty, - /// Contains an digit invalid in its context. + /// Contains an invalid digit in its context. /// /// Among other causes, this variant will be constructed when parsing a string that /// contains a non-asci char. From 75e6deefee8dc68fc35bdfe5effbfdb517b03f9c Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Mon, 26 Oct 2020 05:51:22 -0500 Subject: [PATCH 13/29] asci -> ASCII Co-authored-by: Ashley Mannix --- library/core/src/num/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/num/error.rs b/library/core/src/num/error.rs index d1a65aa35ae25..e1684719ab4ec 100644 --- a/library/core/src/num/error.rs +++ b/library/core/src/num/error.rs @@ -95,7 +95,7 @@ pub enum IntErrorKind { /// Contains an invalid digit in its context. /// /// Among other causes, this variant will be constructed when parsing a string that - /// contains a non-asci char. + /// contains a non-ASCII char. /// /// This variant is also constructed when a `+` or `-` is misplaced within a string /// either on its own or in the middle of a number. From ad2d93da1f051970555a8be0baa8e6277b3a06c1 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Mon, 26 Oct 2020 18:14:12 +0000 Subject: [PATCH 14/29] Apply suggested changes --- compiler/rustc_middle/src/lib.rs | 1 + compiler/rustc_middle/src/middle/limits.rs | 6 ++++-- library/core/src/lib.rs | 1 + library/core/src/num/error.rs | 25 ++++++++++++++-------- library/core/src/num/mod.rs | 13 +++++++---- library/core/tests/lib.rs | 1 + library/core/tests/nonzero.rs | 2 +- library/core/tests/num/mod.rs | 16 +++++++------- library/std/src/lib.rs | 1 + library/std/src/num.rs | 7 +++++- 10 files changed, 48 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_middle/src/lib.rs b/compiler/rustc_middle/src/lib.rs index 37bc1a305b2f8..fa885ce2e7cdf 100644 --- a/compiler/rustc_middle/src/lib.rs +++ b/compiler/rustc_middle/src/lib.rs @@ -46,6 +46,7 @@ #![feature(crate_visibility_modifier)] #![feature(associated_type_bounds)] #![feature(rustc_attrs)] +#![feature(int_error_matching)] #![recursion_limit = "512"] #[macro_use] diff --git a/compiler/rustc_middle/src/middle/limits.rs b/compiler/rustc_middle/src/middle/limits.rs index e0d171fa77125..9ff2b7f08fe04 100644 --- a/compiler/rustc_middle/src/middle/limits.rs +++ b/compiler/rustc_middle/src/middle/limits.rs @@ -50,8 +50,10 @@ fn update_limit( let error_str = match e.kind() { IntErrorKind::PosOverflow => "`limit` is too large", IntErrorKind::Empty => "`limit` must be a non-negative integer", - IntErrorKind::InvalidDigit(_) => "not a valid integer", - IntErrorKind::NegOverflow => bug!("`limit` should never underflow"), + IntErrorKind::InvalidDigit => "not a valid integer", + IntErrorKind::NegOverflow => { + bug!("`limit` should never negatively underflow") + } IntErrorKind::Zero => bug!("zero is a valid `limit`"), kind => bug!("unimplemented IntErrorKind variant: {:?}", kind), }; diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 63ca6e517d214..d67ad30e123f9 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -151,6 +151,7 @@ #![feature(slice_ptr_get)] #![feature(no_niche)] // rust-lang/rust#68303 #![feature(unsafe_block_in_unsafe_fn)] +#![feature(int_error_matching)] #![deny(unsafe_op_in_unsafe_fn)] #[prelude_import] diff --git a/library/core/src/num/error.rs b/library/core/src/num/error.rs index e1684719ab4ec..9d8c8c862911c 100644 --- a/library/core/src/num/error.rs +++ b/library/core/src/num/error.rs @@ -77,20 +77,26 @@ pub struct ParseIntError { /// # Example /// /// ``` +/// #![feature(int_error_matching)] +/// /// # fn main() { /// if let Err(e) = i32::from_str_radix("a12", 10) { /// println!("Failed conversion to i32: {:?}", e.kind()); /// } /// # } /// ``` -#[stable(feature = "int_error_matching", since = "1.47.0")] +#[unstable( + feature = "int_error_matching", + reason = "it can be useful to match errors when making error messages \ + for integer parsing", + issue = "22639" +)] #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] pub enum IntErrorKind { /// Value being parsed is empty. /// /// Among other causes, this variant will be constructed when parsing an empty string. - #[stable(feature = "int_error_matching", since = "1.47.0")] Empty, /// Contains an invalid digit in its context. /// @@ -99,25 +105,26 @@ pub enum IntErrorKind { /// /// This variant is also constructed when a `+` or `-` is misplaced within a string /// either on its own or in the middle of a number. - #[stable(feature = "int_error_matching", since = "1.47.0")] - InvalidDigit(#[stable(feature = "int_error_matching", since = "1.47.0")] char), + InvalidDigit, /// Integer is too large to store in target integer type. - #[stable(feature = "int_error_matching", since = "1.47.0")] PosOverflow, /// Integer is too small to store in target integer type. - #[stable(feature = "int_error_matching", since = "1.47.0")] NegOverflow, /// Value was Zero /// /// This variant will be emitted when the parsing string has a value of zero, which /// would be illegal for non-zero types. - #[stable(feature = "int_error_matching", since = "1.47.0")] Zero, } impl ParseIntError { /// Outputs the detailed cause of parsing an integer failing. - #[stable(feature = "int_error_matching", since = "1.47.0")] + #[unstable( + feature = "int_error_matching", + reason = "it can be useful to match errors when making error messages \ + for integer parsing", + issue = "22639" + )] pub fn kind(&self) -> &IntErrorKind { &self.kind } @@ -131,7 +138,7 @@ impl ParseIntError { pub fn __description(&self) -> &str { match self.kind { IntErrorKind::Empty => "cannot parse integer from empty string", - IntErrorKind::InvalidDigit(_) => "invalid digit found in string", + IntErrorKind::InvalidDigit => "invalid digit found in string", IntErrorKind::PosOverflow => "number too large to fit in target type", IntErrorKind::NegOverflow => "number too small to fit in target type", IntErrorKind::Zero => "number would be zero for non-zero type", diff --git a/library/core/src/num/mod.rs b/library/core/src/num/mod.rs index fd00a072d896c..71448a622c09e 100644 --- a/library/core/src/num/mod.rs +++ b/library/core/src/num/mod.rs @@ -63,7 +63,12 @@ pub use nonzero::{NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, No #[stable(feature = "try_from", since = "1.34.0")] pub use error::TryFromIntError; -#[stable(feature = "int_error_matching", since = "1.47.0")] +#[unstable( + feature = "int_error_matching", + reason = "it can be useful to match errors when making error messages \ + for integer parsing", + issue = "22639" +)] pub use error::IntErrorKind; macro_rules! usize_isize_to_xe_bytes_doc { @@ -831,7 +836,7 @@ fn from_str_radix(src: &str, radix: u32) -> Result { - return Err(PIE { kind: InvalidDigit(src[0] as char) }); + return Err(PIE { kind: InvalidDigit }); } b'+' => (true, &src[1..]), b'-' if is_signed_ty => (false, &src[1..]), @@ -844,7 +849,7 @@ fn from_str_radix(src: &str, radix: u32) -> Result x, - None => return Err(PIE { kind: InvalidDigit(c as char) }), + None => return Err(PIE { kind: InvalidDigit }), }; result = match result.checked_mul(radix) { Some(result) => result, @@ -860,7 +865,7 @@ fn from_str_radix(src: &str, radix: u32) -> Result x, - None => return Err(PIE { kind: InvalidDigit(c as char) }), + None => return Err(PIE { kind: InvalidDigit }), }; result = match result.checked_mul(radix) { Some(result) => result, diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index c128691fa7525..d8b36beb3e085 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -37,6 +37,7 @@ #![feature(try_trait)] #![feature(slice_internals)] #![feature(slice_partition_dedup)] +#![feature(int_error_matching)] #![feature(array_value_iter)] #![feature(iter_partition_in_place)] #![feature(iter_is_partitioned)] diff --git a/library/core/tests/nonzero.rs b/library/core/tests/nonzero.rs index 949d4ea32f064..fb1293c99bba9 100644 --- a/library/core/tests/nonzero.rs +++ b/library/core/tests/nonzero.rs @@ -131,7 +131,7 @@ fn test_from_str() { assert_eq!("0".parse::().err().map(|e| e.kind().clone()), Some(IntErrorKind::Zero)); assert_eq!( "-1".parse::().err().map(|e| e.kind().clone()), - Some(IntErrorKind::InvalidDigit('-')) + Some(IntErrorKind::InvalidDigit) ); assert_eq!( "-129".parse::().err().map(|e| e.kind().clone()), diff --git a/library/core/tests/num/mod.rs b/library/core/tests/num/mod.rs index 12e52252278c5..7c25c32fb40a7 100644 --- a/library/core/tests/num/mod.rs +++ b/library/core/tests/num/mod.rs @@ -118,14 +118,14 @@ fn test_leading_plus() { #[test] fn test_invalid() { - test_parse::("--129", Err(IntErrorKind::InvalidDigit('-'))); - test_parse::("++129", Err(IntErrorKind::InvalidDigit('+'))); - test_parse::("Съешь", Err(IntErrorKind::InvalidDigit('Ð'))); - test_parse::("123Hello", Err(IntErrorKind::InvalidDigit('H'))); - test_parse::("--", Err(IntErrorKind::InvalidDigit('-'))); - test_parse::("-", Err(IntErrorKind::InvalidDigit('-'))); - test_parse::("+", Err(IntErrorKind::InvalidDigit('+'))); - test_parse::("-1", Err(IntErrorKind::InvalidDigit('-'))); + test_parse::("--129", Err(IntErrorKind::InvalidDigit)); + test_parse::("++129", Err(IntErrorKind::InvalidDigit)); + test_parse::("Съешь", Err(IntErrorKind::InvalidDigit)); + test_parse::("123Hello", Err(IntErrorKind::InvalidDigit)); + test_parse::("--", Err(IntErrorKind::InvalidDigit)); + test_parse::("-", Err(IntErrorKind::InvalidDigit)); + test_parse::("+", Err(IntErrorKind::InvalidDigit)); + test_parse::("-1", Err(IntErrorKind::InvalidDigit)); } #[test] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index fa23229066cf1..ac0075ad129c5 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -264,6 +264,7 @@ #![feature(global_asm)] #![feature(hashmap_internals)] #![feature(int_error_internals)] +#![feature(int_error_matching)] #![feature(integer_atomics)] #![feature(into_future)] #![feature(lang_items)] diff --git a/library/std/src/num.rs b/library/std/src/num.rs index ac3b055cdb050..0f1c596268594 100644 --- a/library/std/src/num.rs +++ b/library/std/src/num.rs @@ -22,7 +22,12 @@ pub use core::num::{NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, #[stable(feature = "nonzero", since = "1.28.0")] pub use core::num::{NonZeroU128, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize}; -#[stable(feature = "int_error_matching", since = "1.47.0")] +#[unstable( + feature = "int_error_matching", + reason = "it can be useful to match errors when making error messages \ + for integer parsing", + issue = "22639" +)] pub use core::num::IntErrorKind; #[cfg(test)] From e750238404f59f3f6d94142f3e12ffc4c5c0b366 Mon Sep 17 00:00:00 2001 From: Ethan Brierley Date: Mon, 26 Oct 2020 18:16:25 +0000 Subject: [PATCH 15/29] Fix typo --- compiler/rustc_middle/src/middle/limits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/middle/limits.rs b/compiler/rustc_middle/src/middle/limits.rs index 9ff2b7f08fe04..41342764ba773 100644 --- a/compiler/rustc_middle/src/middle/limits.rs +++ b/compiler/rustc_middle/src/middle/limits.rs @@ -52,7 +52,7 @@ fn update_limit( IntErrorKind::Empty => "`limit` must be a non-negative integer", IntErrorKind::InvalidDigit => "not a valid integer", IntErrorKind::NegOverflow => { - bug!("`limit` should never negatively underflow") + bug!("`limit` should never negatively overflow") } IntErrorKind::Zero => bug!("zero is a valid `limit`"), kind => bug!("unimplemented IntErrorKind variant: {:?}", kind), From e099138eb6274b8450fbbb3c1fec0389337eb992 Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Mon, 26 Oct 2020 23:31:45 +0100 Subject: [PATCH 16/29] BTreeMap: stop mistaking node for an orderly place --- .../alloc/src/collections/btree/map/tests.rs | 26 +++++++++++-- .../alloc/src/collections/btree/node/tests.rs | 37 ------------------- 2 files changed, 22 insertions(+), 41 deletions(-) diff --git a/library/alloc/src/collections/btree/map/tests.rs b/library/alloc/src/collections/btree/map/tests.rs index adb94972f5bb6..f6921e67636d9 100644 --- a/library/alloc/src/collections/btree/map/tests.rs +++ b/library/alloc/src/collections/btree/map/tests.rs @@ -42,7 +42,7 @@ fn test_all_refs<'a, T: 'a>(dummy: &mut T, iter: impl Iterator } } -impl<'a, K: 'a, V: 'a> BTreeMap { +impl BTreeMap { /// Panics if the map (or the code navigating it) is corrupted. fn check(&self) where @@ -54,14 +54,14 @@ impl<'a, K: 'a, V: 'a> BTreeMap { assert!(root_node.ascend().is_err()); root_node.assert_back_pointers(); - let counted = root_node.assert_ascending(); - assert_eq!(self.length, counted); assert_eq!(self.length, root_node.calc_length()); root_node.assert_min_len(if root_node.height() > 0 { 1 } else { 0 }); } else { assert_eq!(self.length, 0); } + + self.assert_ascending(); } /// Returns the height of the root, if any. @@ -79,10 +79,28 @@ impl<'a, K: 'a, V: 'a> BTreeMap { String::from("not yet allocated") } } + + /// Asserts that the keys are in strictly ascending order. + fn assert_ascending(&self) + where + K: Copy + Debug + Ord, + { + let mut num_seen = 0; + let mut keys = self.keys(); + if let Some(mut previous) = keys.next() { + num_seen = 1; + for next in keys { + assert!(previous < next, "{:?} >= {:?}", previous, next); + previous = next; + num_seen += 1; + } + } + assert_eq!(num_seen, self.len()); + } } impl<'a, K: 'a, V: 'a> NodeRef, K, V, marker::LeafOrInternal> { - pub fn assert_min_len(self, min_len: usize) { + fn assert_min_len(self, min_len: usize) { assert!(self.len() >= min_len, "{} < {}", self.len(), min_len); if let node::ForceResult::Internal(node) = self.force() { for idx in 0..=node.len() { diff --git a/library/alloc/src/collections/btree/node/tests.rs b/library/alloc/src/collections/btree/node/tests.rs index d6527057c5d77..38c75de34eeeb 100644 --- a/library/alloc/src/collections/btree/node/tests.rs +++ b/library/alloc/src/collections/btree/node/tests.rs @@ -17,43 +17,6 @@ impl<'a, K: 'a, V: 'a> NodeRef, K, V, marker::LeafOrInternal> } } - /// Asserts that the keys are in strictly ascending order. - /// Returns how many keys it encountered. - pub fn assert_ascending(self) -> usize - where - K: Copy + Debug + Ord, - { - struct SeriesChecker { - num_seen: usize, - previous: Option, - } - impl SeriesChecker { - fn is_ascending(&mut self, next: T) { - if let Some(previous) = self.previous { - assert!(previous < next, "{:?} >= {:?}", previous, next); - } - self.previous = Some(next); - self.num_seen += 1; - } - } - - let mut checker = SeriesChecker { num_seen: 0, previous: None }; - self.visit_nodes_in_order(|pos| match pos { - navigate::Position::Leaf(node) => { - for idx in 0..node.len() { - let key = *unsafe { node.key_at(idx) }; - checker.is_ascending(key); - } - } - navigate::Position::InternalKV(kv) => { - let key = *kv.into_kv().0; - checker.is_ascending(key); - } - navigate::Position::Internal(_) => {} - }); - checker.num_seen - } - pub fn dump_keys(self) -> String where K: Debug, From b0df3f76dc70eba57d6e0665fa6ccac89b25d3aa Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 28 Oct 2020 09:05:45 +0100 Subject: [PATCH 17/29] fix some incorrect aliasing in the BTree --- library/alloc/src/collections/btree/node.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/library/alloc/src/collections/btree/node.rs b/library/alloc/src/collections/btree/node.rs index c8d3de9e5cd5c..433074027e7f7 100644 --- a/library/alloc/src/collections/btree/node.rs +++ b/library/alloc/src/collections/btree/node.rs @@ -1608,15 +1608,19 @@ pub mod marker { unsafe fn slice_insert(slice: &mut [T], idx: usize, val: T) { unsafe { - ptr::copy(slice.as_ptr().add(idx), slice.as_mut_ptr().add(idx + 1), slice.len() - idx); - ptr::write(slice.get_unchecked_mut(idx), val); + let len = slice.len(); + let slice_ptr = slice.as_mut_ptr(); + ptr::copy(slice_ptr.add(idx), slice_ptr.add(idx + 1), len - idx); + ptr::write(slice_ptr.add(idx), val); } } unsafe fn slice_remove(slice: &mut [T], idx: usize) -> T { unsafe { - let ret = ptr::read(slice.get_unchecked(idx)); - ptr::copy(slice.as_ptr().add(idx + 1), slice.as_mut_ptr().add(idx), slice.len() - idx - 1); + let len = slice.len(); + let slice_ptr = slice.as_mut_ptr(); + let ret = ptr::read(slice_ptr.add(idx)); + ptr::copy(slice_ptr.add(idx + 1), slice_ptr.add(idx), len - idx - 1); ret } } From 54a0a98347f739ee3b9cad8760e237fa6cd8db54 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 22 Oct 2020 12:06:12 +0200 Subject: [PATCH 18/29] ci: gate on aarch64-gnu passing tests --- .github/workflows/ci.yml | 115 ++--------------------------------- src/ci/github-actions/ci.yml | 20 +----- 2 files changed, 8 insertions(+), 127 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 91241d2b214f9..90296ec32eeda 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -154,6 +154,11 @@ jobs: strategy: matrix: include: + - name: aarch64-gnu + os: + - self-hosted + - ARM64 + - linux - name: arm-android os: ubuntu-latest-xl env: {} @@ -509,116 +514,6 @@ jobs: AWS_ACCESS_KEY_ID: "${{ env.ARTIFACTS_AWS_ACCESS_KEY_ID }}" AWS_SECRET_ACCESS_KEY: "${{ secrets[format('AWS_SECRET_ACCESS_KEY_{0}', env.ARTIFACTS_AWS_ACCESS_KEY_ID)] }}" if: "success() && !env.SKIP_JOB && (github.event_name == 'push' || env.DEPLOY == '1' || env.DEPLOY_ALT == '1')" - auto-fallible: - name: auto-fallible - env: - CI_JOB_NAME: "${{ matrix.name }}" - SCCACHE_BUCKET: rust-lang-gha-caches - DEPLOY_BUCKET: rust-lang-gha - TOOLSTATE_REPO: "https://github.com/pietroalbini/rust-toolstate" - TOOLSTATE_ISSUES_API_URL: "https://api.github.com/repos/pietroalbini/rust-toolstate/issues" - TOOLSTATE_PUBLISH: 1 - CACHES_AWS_ACCESS_KEY_ID: AKIA46X5W6CZOMUQATD5 - ARTIFACTS_AWS_ACCESS_KEY_ID: AKIA46X5W6CZH5AYXDVF - CACHE_DOMAIN: ci-caches-gha.rust-lang.org - if: "github.event_name == 'push' && github.ref == 'refs/heads/auto' && github.repository == 'rust-lang-ci/rust'" - strategy: - fail-fast: false - matrix: - include: - - name: aarch64-gnu - os: - - self-hosted - - ARM64 - - linux - timeout-minutes: 600 - runs-on: "${{ matrix.os }}" - steps: - - name: disable git crlf conversion - run: git config --global core.autocrlf false - - name: checkout the source code - uses: actions/checkout@v2 - with: - fetch-depth: 2 - - name: configure the PR in which the error message will be posted - run: "echo \"[CI_PR_NUMBER=$num]\"" - env: - num: "${{ github.event.number }}" - if: "success() && !env.SKIP_JOBS && github.event_name == 'pull_request'" - - name: add extra environment variables - run: src/ci/scripts/setup-environment.sh - env: - EXTRA_VARIABLES: "${{ toJson(matrix.env) }}" - if: success() && !env.SKIP_JOB - - name: decide whether to skip this job - run: src/ci/scripts/should-skip-this.sh - if: success() && !env.SKIP_JOB - - name: configure GitHub Actions to kill the build when outdated - uses: rust-lang/simpleinfra/github-actions/cancel-outdated-builds@master - with: - github_token: "${{ secrets.github_token }}" - if: "success() && !env.SKIP_JOB && github.ref != 'refs/heads/try'" - - name: collect CPU statistics - run: src/ci/scripts/collect-cpu-stats.sh - if: success() && !env.SKIP_JOB - - name: show the current environment - run: src/ci/scripts/dump-environment.sh - if: success() && !env.SKIP_JOB - - name: install awscli - run: src/ci/scripts/install-awscli.sh - if: success() && !env.SKIP_JOB - - name: install sccache - run: src/ci/scripts/install-sccache.sh - if: success() && !env.SKIP_JOB - - name: select Xcode - run: src/ci/scripts/select-xcode.sh - if: success() && !env.SKIP_JOB - - name: install clang - run: src/ci/scripts/install-clang.sh - if: success() && !env.SKIP_JOB - - name: install WIX - run: src/ci/scripts/install-wix.sh - if: success() && !env.SKIP_JOB - - name: ensure the build happens on a partition with enough space - run: src/ci/scripts/symlink-build-dir.sh - if: success() && !env.SKIP_JOB - - name: disable git crlf conversion - run: src/ci/scripts/disable-git-crlf-conversion.sh - if: success() && !env.SKIP_JOB - - name: install MSYS2 - run: src/ci/scripts/install-msys2.sh - if: success() && !env.SKIP_JOB - - name: install MinGW - run: src/ci/scripts/install-mingw.sh - if: success() && !env.SKIP_JOB - - name: install ninja - run: src/ci/scripts/install-ninja.sh - if: success() && !env.SKIP_JOB - - name: enable ipv6 on Docker - run: src/ci/scripts/enable-docker-ipv6.sh - if: success() && !env.SKIP_JOB - - name: disable git crlf conversion - run: src/ci/scripts/disable-git-crlf-conversion.sh - if: success() && !env.SKIP_JOB - - name: checkout submodules - run: src/ci/scripts/checkout-submodules.sh - if: success() && !env.SKIP_JOB - - name: ensure line endings are correct - run: src/ci/scripts/verify-line-endings.sh - if: success() && !env.SKIP_JOB - - name: run the build - run: src/ci/scripts/run-build-from-ci.sh - env: - AWS_ACCESS_KEY_ID: "${{ env.CACHES_AWS_ACCESS_KEY_ID }}" - AWS_SECRET_ACCESS_KEY: "${{ secrets[format('AWS_SECRET_ACCESS_KEY_{0}', env.CACHES_AWS_ACCESS_KEY_ID)] }}" - TOOLSTATE_REPO_ACCESS_TOKEN: "${{ secrets.TOOLSTATE_REPO_ACCESS_TOKEN }}" - if: success() && !env.SKIP_JOB - - name: upload artifacts to S3 - run: src/ci/scripts/upload-artifacts.sh - env: - AWS_ACCESS_KEY_ID: "${{ env.ARTIFACTS_AWS_ACCESS_KEY_ID }}" - AWS_SECRET_ACCESS_KEY: "${{ secrets[format('AWS_SECRET_ACCESS_KEY_{0}', env.ARTIFACTS_AWS_ACCESS_KEY_ID)] }}" - if: "success() && !env.SKIP_JOB && (github.event_name == 'push' || env.DEPLOY == '1' || env.DEPLOY_ALT == '1')" try: name: try env: diff --git a/src/ci/github-actions/ci.yml b/src/ci/github-actions/ci.yml index 0df191f8f7404..1e28b5a253655 100644 --- a/src/ci/github-actions/ci.yml +++ b/src/ci/github-actions/ci.yml @@ -301,6 +301,9 @@ jobs: # Linux/Docker builders # ############################# + - name: aarch64-gnu + <<: *job-aarch64-linux + - name: arm-android <<: *job-linux-xl @@ -653,23 +656,6 @@ jobs: SCRIPT: python x.py dist <<: *job-windows-xl - auto-fallible: - <<: *base-ci-job - name: auto-fallible - env: - <<: [*shared-ci-variables, *dummy-variables] - if: github.event_name == 'push' && github.ref == 'refs/heads/auto' && github.repository == 'rust-lang-ci/rust' - strategy: - fail-fast: false - matrix: - include: - ############################# - # Linux/Docker builders # - ############################# - - - name: aarch64-gnu - <<: *job-aarch64-linux - try: <<: *base-ci-job name: try From 1274faed1a1379bb1521ae9884bc8ef420971636 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 22 Oct 2020 12:23:18 +0200 Subject: [PATCH 19/29] doc/rustc: promote aarch64-unknown-linux-gnu to tier 1 This also adds a note about missing stack probes support, per the discussion on RFC 2959. --- src/doc/rustc/src/platform-support.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/doc/rustc/src/platform-support.md b/src/doc/rustc/src/platform-support.md index 85c6f91f08582..8005a5f3563bf 100644 --- a/src/doc/rustc/src/platform-support.md +++ b/src/doc/rustc/src/platform-support.md @@ -34,6 +34,7 @@ Specifically they will each satisfy the following requirements: target | std | host | notes -------|-----|------|------- +`aarch64-unknown-linux-gnu` | ✓ | ✓ | ARM64 Linux (kernel 4.2, glibc 2.17+) [^missing-stack-probes] `i686-pc-windows-gnu` | ✓ | ✓ | 32-bit MinGW (Windows 7+) `i686-pc-windows-msvc` | ✓ | ✓ | 32-bit MSVC (Windows 7+) `i686-unknown-linux-gnu` | ✓ | ✓ | 32-bit Linux (kernel 2.6.32+, glibc 2.11+) @@ -42,6 +43,12 @@ target | std | host | notes `x86_64-pc-windows-msvc` | ✓ | ✓ | 64-bit MSVC (Windows 7+) `x86_64-unknown-linux-gnu` | ✓ | ✓ | 64-bit Linux (kernel 2.6.32+, glibc 2.11+) +[^missing-stack-probes]: Stack probes support is missing on + `aarch64-unknown-linux-gnu`, but it's planned to be implemented in the near + future. The implementation is tracked on [issue #77071][77071]. + +[77071]: https://github.com/rust-lang/rust/issues/77071 + ## Tier 2 Tier 2 platforms can be thought of as "guaranteed to build". Automated tests @@ -62,7 +69,6 @@ target | std | host | notes `aarch64-fuchsia` | ✓ | | ARM64 Fuchsia `aarch64-linux-android` | ✓ | | ARM64 Android `aarch64-pc-windows-msvc` | ✓ | ✓ | ARM64 Windows MSVC -`aarch64-unknown-linux-gnu` | ✓ | ✓ | ARM64 Linux (kernel 4.2, glibc 2.17) `aarch64-unknown-linux-musl` | ✓ | ✓ | ARM64 Linux with MUSL `aarch64-unknown-none` | * | | Bare ARM64, hardfloat `aarch64-unknown-none-softfloat` | * | | Bare ARM64, softfloat From 874cbb88e0379a0e7bab5b9db99e2a4c495534fb Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 22 Oct 2020 12:31:11 +0200 Subject: [PATCH 20/29] ci: build docs for aarch64-unknown-linux-gnu --- src/ci/docker/host-x86_64/dist-aarch64-linux/Dockerfile | 3 +-- src/tools/build-manifest/src/main.rs | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ci/docker/host-x86_64/dist-aarch64-linux/Dockerfile b/src/ci/docker/host-x86_64/dist-aarch64-linux/Dockerfile index df65f9df44127..95c54ca1abc06 100644 --- a/src/ci/docker/host-x86_64/dist-aarch64-linux/Dockerfile +++ b/src/ci/docker/host-x86_64/dist-aarch64-linux/Dockerfile @@ -35,6 +35,5 @@ ENV HOSTS=aarch64-unknown-linux-gnu ENV RUST_CONFIGURE_ARGS \ --enable-full-tools \ --enable-profiler \ - --enable-sanitizers \ - --disable-docs + --enable-sanitizers ENV SCRIPT python3 ../x.py dist --host $HOSTS --target $HOSTS diff --git a/src/tools/build-manifest/src/main.rs b/src/tools/build-manifest/src/main.rs index 9a8f2404e4a1a..687354dc6aee2 100644 --- a/src/tools/build-manifest/src/main.rs +++ b/src/tools/build-manifest/src/main.rs @@ -155,6 +155,7 @@ static TARGETS: &[&str] = &[ ]; static DOCS_TARGETS: &[&str] = &[ + "aarch64-unknown-linux-gnu", "i686-apple-darwin", "i686-pc-windows-gnu", "i686-pc-windows-msvc", From eed0cebea3d16d82f5517e17ab498b340001fe01 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 19 Oct 2020 14:20:14 -0400 Subject: [PATCH 21/29] Recognize `private_intra_doc_links` as a lint Previously, trying to allow this would give another error! ``` warning: unknown lint: `private_intra_doc_links` --> private.rs:1:10 | 1 | #![allow(private_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `broken_intra_doc_links` | = note: `#[warn(unknown_lints)]` on by default warning: public documentation for `DocMe` links to private item `DontDocMe` --> private.rs:2:11 | 2 | /// docs [DontDocMe] | ^^^^^^^^^ this item is private | = note: `#[warn(private_intra_doc_links)]` on by default = note: this link will resolve properly if you pass `--document-private-items` ``` --- compiler/rustc_lint_defs/src/builtin.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index a1b7c13e4c0f0..7176a66cdc1bd 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -2790,6 +2790,7 @@ declare_lint_pass! { UNSTABLE_NAME_COLLISIONS, IRREFUTABLE_LET_PATTERNS, BROKEN_INTRA_DOC_LINKS, + PRIVATE_INTRA_DOC_LINKS, INVALID_CODEBLOCK_ATTRIBUTES, MISSING_CRATE_LEVEL_DOCS, MISSING_DOC_CODE_EXAMPLES, From 47b21b84f3ed88cff31fccc78a57a66241a26496 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 5 Nov 2020 13:11:50 -0500 Subject: [PATCH 22/29] Add PRIVATE_INTRA_DOC_LINKS to rustdoc special-casing This is really starting to get out of hand. Rustdoc should instead allow all lints in the rustdoc lint group. --- src/librustdoc/core.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 45a84c4fb30d3..285a3bf8204bb 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -322,7 +322,8 @@ pub fn run_core( let cpath = Some(input.clone()); let input = Input::File(input); - let intra_link_resolution_failure_name = lint::builtin::BROKEN_INTRA_DOC_LINKS.name; + let broken_intra_doc_links = lint::builtin::BROKEN_INTRA_DOC_LINKS.name; + let private_intra_doc_links = lint::builtin::PRIVATE_INTRA_DOC_LINKS.name; let missing_docs = rustc_lint::builtin::MISSING_DOCS.name; let missing_doc_example = rustc_lint::builtin::MISSING_DOC_CODE_EXAMPLES.name; let private_doc_tests = rustc_lint::builtin::PRIVATE_DOC_TESTS.name; @@ -335,7 +336,8 @@ pub fn run_core( // In addition to those specific lints, we also need to allow those given through // command line, otherwise they'll get ignored and we don't want that. let lints_to_show = vec![ - intra_link_resolution_failure_name.to_owned(), + broken_intra_doc_links.to_owned(), + private_intra_doc_links.to_owned(), missing_docs.to_owned(), missing_doc_example.to_owned(), private_doc_tests.to_owned(), @@ -347,9 +349,8 @@ pub fn run_core( ]; let (lint_opts, lint_caps) = init_lints(lints_to_show, lint_opts, |lint| { - if lint.name == intra_link_resolution_failure_name - || lint.name == invalid_codeblock_attributes_name - { + // FIXME: why is this necessary? + if lint.name == broken_intra_doc_links || lint.name == invalid_codeblock_attributes_name { None } else { Some((lint.name_lower(), lint::Allow)) From 8a8ee1a3ed249e4077a7ce0b88903b493419e7d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 6 Nov 2020 00:00:00 +0000 Subject: [PATCH 23/29] inliner: Use substs_for_mir_body Changes from 68965 extended the kind of instances that are being inlined. For some of those, the `instance_mir` returns a MIR body that is already expressed in terms of the types found in substitution array, and doesn't need further substitution. Use `substs_for_mir_body` to take that into account. --- .../rustc_codegen_cranelift/src/common.rs | 12 ++--- compiler/rustc_codegen_ssa/src/mir/mod.rs | 14 ++--- compiler/rustc_middle/src/ty/instance.rs | 27 +++++++++- .../rustc_mir/src/interpret/eval_context.rs | 6 +-- .../rustc_mir/src/monomorphize/collector.rs | 10 ++-- compiler/rustc_mir/src/transform/inline.rs | 24 ++++----- src/test/mir-opt/inline/inline-shims.rs | 13 +++++ .../inline/inline_shims.clone.Inline.diff | 26 ++++++++++ .../inline/inline_shims.drop.Inline.diff | 52 +++++++++++++++++++ 9 files changed, 142 insertions(+), 42 deletions(-) create mode 100644 src/test/mir-opt/inline/inline-shims.rs create mode 100644 src/test/mir-opt/inline/inline_shims.clone.Inline.diff create mode 100644 src/test/mir-opt/inline/inline_shims.drop.Inline.diff diff --git a/compiler/rustc_codegen_cranelift/src/common.rs b/compiler/rustc_codegen_cranelift/src/common.rs index eda77bf19d354..466758f2f86f5 100644 --- a/compiler/rustc_codegen_cranelift/src/common.rs +++ b/compiler/rustc_codegen_cranelift/src/common.rs @@ -361,13 +361,11 @@ impl<'tcx, M: Module> FunctionCx<'_, 'tcx, M> { where T: TypeFoldable<'tcx> + Copy, { - if let Some(substs) = self.instance.substs_for_mir_body() { - self.tcx - .subst_and_normalize_erasing_regions(substs, ty::ParamEnv::reveal_all(), value) - } else { - self.tcx - .normalize_erasing_regions(ty::ParamEnv::reveal_all(), *value) - } + self.instance.subst_mir_and_normalize_erasing_regions( + self.tcx, + ty::ParamEnv::reveal_all(), + value + ) } pub(crate) fn clif_type(&self, ty: Ty<'tcx>) -> Option { diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 84e82e88e8eea..01fd1681593e8 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -92,15 +92,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { T: Copy + TypeFoldable<'tcx>, { debug!("monomorphize: self.instance={:?}", self.instance); - if let Some(substs) = self.instance.substs_for_mir_body() { - self.cx.tcx().subst_and_normalize_erasing_regions( - substs, - ty::ParamEnv::reveal_all(), - &value, - ) - } else { - self.cx.tcx().normalize_erasing_regions(ty::ParamEnv::reveal_all(), *value) - } + self.instance.subst_mir_and_normalize_erasing_regions( + self.cx.tcx(), + ty::ParamEnv::reveal_all(), + value, + ) } } diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index 8b3fb87507061..306cebd9cb722 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -1,6 +1,6 @@ use crate::middle::codegen_fn_attrs::CodegenFnAttrFlags; use crate::ty::print::{FmtPrinter, Printer}; -use crate::ty::subst::InternalSubsts; +use crate::ty::subst::{InternalSubsts, Subst}; use crate::ty::{self, SubstsRef, Ty, TyCtxt, TypeFoldable}; use rustc_errors::ErrorReported; use rustc_hir::def::Namespace; @@ -470,10 +470,33 @@ impl<'tcx> Instance<'tcx> { /// This function returns `Some(substs)` in the former case and `None` otherwise -- i.e., if /// this function returns `None`, then the MIR body does not require substitution during /// codegen. - pub fn substs_for_mir_body(&self) -> Option> { + fn substs_for_mir_body(&self) -> Option> { if self.def.has_polymorphic_mir_body() { Some(self.substs) } else { None } } + pub fn subst_mir(&self, tcx: TyCtxt<'tcx>, v: &T) -> T + where + T: TypeFoldable<'tcx> + Copy, + { + if let Some(substs) = self.substs_for_mir_body() { v.subst(tcx, substs) } else { *v } + } + + pub fn subst_mir_and_normalize_erasing_regions( + &self, + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + v: &T, + ) -> T + where + T: TypeFoldable<'tcx> + Clone, + { + if let Some(substs) = self.substs_for_mir_body() { + tcx.subst_and_normalize_erasing_regions(substs, param_env, v) + } else { + tcx.normalize_erasing_regions(param_env, v.clone()) + } + } + /// Returns a new `Instance` where generic parameters in `instance.substs` are replaced by /// identify parameters if they are determined to be unused in `instance.def`. pub fn polymorphize(self, tcx: TyCtxt<'tcx>) -> Self { diff --git a/compiler/rustc_mir/src/interpret/eval_context.rs b/compiler/rustc_mir/src/interpret/eval_context.rs index 8d0c8c18537ea..0f86a181a55f7 100644 --- a/compiler/rustc_mir/src/interpret/eval_context.rs +++ b/compiler/rustc_mir/src/interpret/eval_context.rs @@ -505,11 +505,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { frame: &Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>, value: T, ) -> T { - if let Some(substs) = frame.instance.substs_for_mir_body() { - self.tcx.subst_and_normalize_erasing_regions(substs, self.param_env, &value) - } else { - self.tcx.normalize_erasing_regions(self.param_env, value) - } + frame.instance.subst_mir_and_normalize_erasing_regions(*self.tcx, self.param_env, &value) } /// The `substs` are assumed to already be in our interpreter "universe" (param_env). diff --git a/compiler/rustc_mir/src/monomorphize/collector.rs b/compiler/rustc_mir/src/monomorphize/collector.rs index 417176564b92d..938181abff244 100644 --- a/compiler/rustc_mir/src/monomorphize/collector.rs +++ b/compiler/rustc_mir/src/monomorphize/collector.rs @@ -543,11 +543,11 @@ impl<'a, 'tcx> MirNeighborCollector<'a, 'tcx> { T: TypeFoldable<'tcx>, { debug!("monomorphize: self.instance={:?}", self.instance); - if let Some(substs) = self.instance.substs_for_mir_body() { - self.tcx.subst_and_normalize_erasing_regions(substs, ty::ParamEnv::reveal_all(), &value) - } else { - self.tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), value) - } + self.instance.subst_mir_and_normalize_erasing_regions( + self.tcx, + ty::ParamEnv::reveal_all(), + &value, + ) } } diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs index 5407226e386cb..37679c24454af 100644 --- a/compiler/rustc_mir/src/transform/inline.rs +++ b/compiler/rustc_mir/src/transform/inline.rs @@ -6,7 +6,6 @@ use rustc_index::vec::Idx; use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; use rustc_middle::mir::visit::*; use rustc_middle::mir::*; -use rustc_middle::ty::subst::Subst; use rustc_middle::ty::{self, ConstKind, Instance, InstanceDef, ParamEnv, Ty, TyCtxt}; use rustc_span::{hygiene::ExpnKind, ExpnData, Span}; use rustc_target::spec::abi::Abi; @@ -128,17 +127,15 @@ impl Inliner<'tcx> { self.tcx.instance_mir(callsite.callee.def) }; - let callee_body: &Body<'tcx> = &*callee_body; - - let callee_body = if self.consider_optimizing(callsite, callee_body) { - self.tcx.subst_and_normalize_erasing_regions( - &callsite.callee.substs, - self.param_env, - callee_body, - ) - } else { + if !self.consider_optimizing(callsite, &callee_body) { continue; - }; + } + + let callee_body = callsite.callee.subst_mir_and_normalize_erasing_regions( + self.tcx, + self.param_env, + callee_body, + ); // Copy only unevaluated constants from the callee_body into the caller_body. // Although we are only pushing `ConstKind::Unevaluated` consts to @@ -317,7 +314,7 @@ impl Inliner<'tcx> { work_list.push(target); // If the place doesn't actually need dropping, treat it like // a regular goto. - let ty = place.ty(callee_body, tcx).subst(tcx, callsite.callee.substs).ty; + let ty = callsite.callee.subst_mir(self.tcx, &place.ty(callee_body, tcx).ty); if ty.needs_drop(tcx, self.param_env) { cost += CALL_PENALTY; if let Some(unwind) = unwind { @@ -379,8 +376,7 @@ impl Inliner<'tcx> { let ptr_size = tcx.data_layout.pointer_size.bytes(); for v in callee_body.vars_and_temps_iter() { - let v = &callee_body.local_decls[v]; - let ty = v.ty.subst(tcx, callsite.callee.substs); + let ty = callsite.callee.subst_mir(self.tcx, &callee_body.local_decls[v].ty); // Cost of the var is the size in machine-words, if we know // it. if let Some(size) = type_size_of(tcx, self.param_env, ty) { diff --git a/src/test/mir-opt/inline/inline-shims.rs b/src/test/mir-opt/inline/inline-shims.rs new file mode 100644 index 0000000000000..7c8618f71e5f5 --- /dev/null +++ b/src/test/mir-opt/inline/inline-shims.rs @@ -0,0 +1,13 @@ +// ignore-wasm32-bare compiled with panic=abort by default +#![crate_type = "lib"] + +// EMIT_MIR inline_shims.clone.Inline.diff +pub fn clone(f: fn(A, B)) -> fn(A, B) { + f.clone() +} + +// EMIT_MIR inline_shims.drop.Inline.diff +pub fn drop(a: *mut Vec, b: *mut Option) { + unsafe { std::ptr::drop_in_place(a) } + unsafe { std::ptr::drop_in_place(b) } +} diff --git a/src/test/mir-opt/inline/inline_shims.clone.Inline.diff b/src/test/mir-opt/inline/inline_shims.clone.Inline.diff new file mode 100644 index 0000000000000..3bdd4f4ff56cc --- /dev/null +++ b/src/test/mir-opt/inline/inline_shims.clone.Inline.diff @@ -0,0 +1,26 @@ +- // MIR for `clone` before Inline ++ // MIR for `clone` after Inline + + fn clone(_1: fn(A, B)) -> fn(A, B) { + debug f => _1; // in scope 0 at $DIR/inline-shims.rs:5:20: 5:21 + let mut _0: fn(A, B); // return place in scope 0 at $DIR/inline-shims.rs:5:36: 5:44 + let mut _2: &fn(A, B); // in scope 0 at $DIR/inline-shims.rs:6:5: 6:6 ++ scope 1 (inlined ::clone - shim(fn(A, B))) { // at $DIR/inline-shims.rs:6:5: 6:14 ++ } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/inline-shims.rs:6:5: 6:6 + _2 = &_1; // scope 0 at $DIR/inline-shims.rs:6:5: 6:6 +- _0 = ::clone(move _2) -> bb1; // scope 0 at $DIR/inline-shims.rs:6:5: 6:14 +- // mir::Constant +- // + span: $DIR/inline-shims.rs:6:7: 6:12 +- // + literal: Const { ty: for<'r> fn(&'r fn(A, B)) -> fn(A, B) {::clone}, val: Value(Scalar()) } +- } +- +- bb1: { ++ _0 = (*_2); // scope 1 at $DIR/inline-shims.rs:6:5: 6:14 + StorageDead(_2); // scope 0 at $DIR/inline-shims.rs:6:13: 6:14 + return; // scope 0 at $DIR/inline-shims.rs:7:2: 7:2 + } + } + diff --git a/src/test/mir-opt/inline/inline_shims.drop.Inline.diff b/src/test/mir-opt/inline/inline_shims.drop.Inline.diff new file mode 100644 index 0000000000000..503d8bc6b7a68 --- /dev/null +++ b/src/test/mir-opt/inline/inline_shims.drop.Inline.diff @@ -0,0 +1,52 @@ +- // MIR for `drop` before Inline ++ // MIR for `drop` after Inline + + fn drop(_1: *mut Vec, _2: *mut Option) -> () { + debug a => _1; // in scope 0 at $DIR/inline-shims.rs:10:19: 10:20 + debug b => _2; // in scope 0 at $DIR/inline-shims.rs:10:35: 10:36 + let mut _0: (); // return place in scope 0 at $DIR/inline-shims.rs:10:54: 10:54 + let _3: (); // in scope 0 at $DIR/inline-shims.rs:11:14: 11:40 + let mut _4: *mut std::vec::Vec; // in scope 0 at $DIR/inline-shims.rs:11:38: 11:39 + let mut _5: *mut std::option::Option; // in scope 0 at $DIR/inline-shims.rs:12:38: 12:39 + scope 1 { + } + scope 2 { ++ scope 3 (inlined drop_in_place::> - shim(Some(Option))) { // at $DIR/inline-shims.rs:12:14: 12:40 ++ let mut _6: isize; // in scope 3 at $DIR/inline-shims.rs:12:14: 12:40 ++ let mut _7: isize; // in scope 3 at $DIR/inline-shims.rs:12:14: 12:40 ++ } + } + + bb0: { + StorageLive(_3); // scope 0 at $DIR/inline-shims.rs:11:5: 11:42 + StorageLive(_4); // scope 1 at $DIR/inline-shims.rs:11:38: 11:39 + _4 = _1; // scope 1 at $DIR/inline-shims.rs:11:38: 11:39 + _3 = drop_in_place::>(move _4) -> bb1; // scope 1 at $DIR/inline-shims.rs:11:14: 11:40 + // mir::Constant + // + span: $DIR/inline-shims.rs:11:14: 11:37 + // + literal: Const { ty: unsafe fn(*mut std::vec::Vec) {std::intrinsics::drop_in_place::>}, val: Value(Scalar()) } + } + + bb1: { + StorageDead(_4); // scope 1 at $DIR/inline-shims.rs:11:39: 11:40 + StorageDead(_3); // scope 0 at $DIR/inline-shims.rs:11:41: 11:42 + StorageLive(_5); // scope 2 at $DIR/inline-shims.rs:12:38: 12:39 + _5 = _2; // scope 2 at $DIR/inline-shims.rs:12:38: 12:39 +- _0 = drop_in_place::>(move _5) -> bb2; // scope 2 at $DIR/inline-shims.rs:12:14: 12:40 +- // mir::Constant +- // + span: $DIR/inline-shims.rs:12:14: 12:37 +- // + literal: Const { ty: unsafe fn(*mut std::option::Option) {std::intrinsics::drop_in_place::>}, val: Value(Scalar()) } ++ _6 = discriminant((*_5)); // scope 3 at $DIR/inline-shims.rs:12:14: 12:40 ++ switchInt(move _6) -> [0_isize: bb2, otherwise: bb3]; // scope 3 at $DIR/inline-shims.rs:12:14: 12:40 + } + + bb2: { + StorageDead(_5); // scope 2 at $DIR/inline-shims.rs:12:39: 12:40 + return; // scope 0 at $DIR/inline-shims.rs:13:2: 13:2 ++ } ++ ++ bb3: { ++ drop((((*_5) as Some).0: B)) -> bb2; // scope 3 at $DIR/inline-shims.rs:12:14: 12:40 + } + } + From 3a7a997323436ecf255c39898667320935445f62 Mon Sep 17 00:00:00 2001 From: Fabian Zaiser Date: Wed, 4 Nov 2020 16:32:52 +0000 Subject: [PATCH 24/29] Implement destructuring assignment for tuples Co-authored-by: varkor --- compiler/rustc_ast_lowering/src/expr.rs | 131 +++++++++++++++++- compiler/rustc_ast_lowering/src/lib.rs | 17 ++- compiler/rustc_ast_lowering/src/pat.rs | 9 +- compiler/rustc_feature/src/active.rs | 3 + compiler/rustc_hir/src/hir.rs | 6 + .../src/thir/pattern/check_match.rs | 1 + compiler/rustc_span/src/symbol.rs | 1 + compiler/rustc_typeck/src/check/expr.rs | 37 ++--- compiler/rustc_typeck/src/check/pat.rs | 5 + compiler/rustc_typeck/src/check/regionck.rs | 2 +- src/test/ui/bad/bad-expr-lhs.rs | 6 +- src/test/ui/bad/bad-expr-lhs.stderr | 47 +++++-- .../default-match-bindings-forbidden.rs | 7 + .../default-match-bindings-forbidden.stderr | 14 ++ .../note-unsupported.rs | 13 +- .../note-unsupported.stderr | 48 ++----- .../tuple_destructure.rs | 37 +++++ .../tuple_destructure_fail.rs | 10 ++ .../tuple_destructure_fail.stderr | 31 +++++ .../warn-unused-duplication.rs | 23 +++ .../warn-unused-duplication.stderr | 15 ++ .../feature-gate-destructuring_assignment.rs | 4 + ...ature-gate-destructuring_assignment.stderr | 14 ++ 23 files changed, 395 insertions(+), 86 deletions(-) create mode 100644 src/test/ui/destructuring-assignment/default-match-bindings-forbidden.rs create mode 100644 src/test/ui/destructuring-assignment/default-match-bindings-forbidden.stderr create mode 100644 src/test/ui/destructuring-assignment/tuple_destructure.rs create mode 100644 src/test/ui/destructuring-assignment/tuple_destructure_fail.rs create mode 100644 src/test/ui/destructuring-assignment/tuple_destructure_fail.stderr create mode 100644 src/test/ui/destructuring-assignment/warn-unused-duplication.rs create mode 100644 src/test/ui/destructuring-assignment/warn-unused-duplication.stderr create mode 100644 src/test/ui/feature-gates/feature-gate-destructuring_assignment.rs create mode 100644 src/test/ui/feature-gates/feature-gate-destructuring_assignment.stderr diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index a6ac056b93b5e..1f2aba2b27e68 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -9,6 +9,7 @@ use rustc_data_structures::thin_vec::ThinVec; use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def::Res; +use rustc_session::parse::feature_err; use rustc_span::hygiene::ForLoopLoc; use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned}; use rustc_span::symbol::{sym, Ident, Symbol}; @@ -146,7 +147,7 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ExprKind::Block(self.lower_block(blk, opt_label.is_some()), opt_label) } ExprKind::Assign(ref el, ref er, span) => { - hir::ExprKind::Assign(self.lower_expr(el), self.lower_expr(er), span) + self.lower_expr_assign(el, er, span, e.span) } ExprKind::AssignOp(op, ref el, ref er) => hir::ExprKind::AssignOp( self.lower_binop(op), @@ -840,6 +841,134 @@ impl<'hir> LoweringContext<'_, 'hir> { }) } + /// Destructure the LHS of complex assignments. + /// For instance, lower `(a, b) = t` to `{ let (lhs1, lhs2) = t; a = lhs1; b = lhs2; }`. + fn lower_expr_assign( + &mut self, + lhs: &Expr, + rhs: &Expr, + eq_sign_span: Span, + whole_span: Span, + ) -> hir::ExprKind<'hir> { + // Return early in case of an ordinary assignment. + fn is_ordinary(lhs: &Expr) -> bool { + match &lhs.kind { + ExprKind::Tup(..) => false, + ExprKind::Paren(e) => { + match e.kind { + // We special-case `(..)` for consistency with patterns. + ExprKind::Range(None, None, RangeLimits::HalfOpen) => false, + _ => is_ordinary(e), + } + } + _ => true, + } + } + if is_ordinary(lhs) { + return hir::ExprKind::Assign(self.lower_expr(lhs), self.lower_expr(rhs), eq_sign_span); + } + if !self.sess.features_untracked().destructuring_assignment { + feature_err( + &self.sess.parse_sess, + sym::destructuring_assignment, + eq_sign_span, + "destructuring assignments are unstable", + ) + .span_label(lhs.span, "cannot assign to this expression") + .emit(); + } + + let mut assignments = vec![]; + + // The LHS becomes a pattern: `(lhs1, lhs2)`. + let pat = self.destructure_assign(lhs, eq_sign_span, &mut assignments); + let rhs = self.lower_expr(rhs); + + // Introduce a `let` for destructuring: `let (lhs1, lhs2) = t`. + let destructure_let = self.stmt_let_pat( + ThinVec::new(), + whole_span, + Some(rhs), + pat, + hir::LocalSource::AssignDesugar(eq_sign_span), + ); + + // `a = lhs1; b = lhs2;`. + let stmts = self + .arena + .alloc_from_iter(std::iter::once(destructure_let).chain(assignments.into_iter())); + + // Wrap everything in a block. + hir::ExprKind::Block(&self.block_all(whole_span, stmts, None), None) + } + + /// Convert the LHS of a destructuring assignment to a pattern. + /// Each sub-assignment is recorded in `assignments`. + fn destructure_assign( + &mut self, + lhs: &Expr, + eq_sign_span: Span, + assignments: &mut Vec>, + ) -> &'hir hir::Pat<'hir> { + match &lhs.kind { + // Tuples. + ExprKind::Tup(elements) => { + let (pats, rest) = + self.destructure_sequence(elements, "tuple", eq_sign_span, assignments); + let tuple_pat = hir::PatKind::Tuple(pats, rest.map(|r| r.0)); + return self.pat_without_dbm(lhs.span, tuple_pat); + } + ExprKind::Paren(e) => { + // We special-case `(..)` for consistency with patterns. + if let ExprKind::Range(None, None, RangeLimits::HalfOpen) = e.kind { + let tuple_pat = hir::PatKind::Tuple(&[], Some(0)); + return self.pat_without_dbm(lhs.span, tuple_pat); + } else { + return self.destructure_assign(e, eq_sign_span, assignments); + } + } + _ => {} + } + // Treat all other cases as normal lvalue. + let ident = Ident::new(sym::lhs, lhs.span); + let (pat, binding) = self.pat_ident(lhs.span, ident); + let ident = self.expr_ident(lhs.span, ident, binding); + let assign = hir::ExprKind::Assign(self.lower_expr(lhs), ident, eq_sign_span); + let expr = self.expr(lhs.span, assign, ThinVec::new()); + assignments.push(self.stmt_expr(lhs.span, expr)); + pat + } + + /// Destructure a sequence of expressions occurring on the LHS of an assignment. + /// Such a sequence occurs in a tuple (struct)/slice. + /// Return a sequence of corresponding patterns, and the index and the span of `..` if it + /// exists. + /// Each sub-assignment is recorded in `assignments`. + fn destructure_sequence( + &mut self, + elements: &[AstP], + ctx: &str, + eq_sign_span: Span, + assignments: &mut Vec>, + ) -> (&'hir [&'hir hir::Pat<'hir>], Option<(usize, Span)>) { + let mut rest = None; + let elements = + self.arena.alloc_from_iter(elements.iter().enumerate().filter_map(|(i, e)| { + // Check for `..` pattern. + if let ExprKind::Range(None, None, RangeLimits::HalfOpen) = e.kind { + if let Some((_, prev_span)) = rest { + self.ban_extra_rest_pat(e.span, prev_span, ctx); + } else { + rest = Some((i, e.span)); + } + None + } else { + Some(self.destructure_assign(e, eq_sign_span, assignments)) + } + })); + (elements, rest) + } + /// Desugar `..=` into `std::ops::RangeInclusive::new(, )`. fn lower_expr_range_closed(&mut self, span: Span, e1: &Expr, e2: &Expr) -> hir::ExprKind<'hir> { let e1 = self.lower_expr_mut(e1); diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 599599f415f1c..af2f96d5e6253 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -2531,6 +2531,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { hir_id, kind: hir::PatKind::Binding(bm, hir_id, ident.with_span_pos(span), None), span, + default_binding_modes: true, }), hir_id, ) @@ -2541,7 +2542,21 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } fn pat(&mut self, span: Span, kind: hir::PatKind<'hir>) -> &'hir hir::Pat<'hir> { - self.arena.alloc(hir::Pat { hir_id: self.next_id(), kind, span }) + self.arena.alloc(hir::Pat { + hir_id: self.next_id(), + kind, + span, + default_binding_modes: true, + }) + } + + fn pat_without_dbm(&mut self, span: Span, kind: hir::PatKind<'hir>) -> &'hir hir::Pat<'hir> { + self.arena.alloc(hir::Pat { + hir_id: self.next_id(), + kind, + span, + default_binding_modes: false, + }) } fn ty_path( diff --git a/compiler/rustc_ast_lowering/src/pat.rs b/compiler/rustc_ast_lowering/src/pat.rs index a1cbcde1f4291..e4e7b24d29e52 100644 --- a/compiler/rustc_ast_lowering/src/pat.rs +++ b/compiler/rustc_ast_lowering/src/pat.rs @@ -273,11 +273,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { /// Construct a `Pat` with the `HirId` of `p.id` lowered. fn pat_with_node_id_of(&mut self, p: &Pat, kind: hir::PatKind<'hir>) -> &'hir hir::Pat<'hir> { - self.arena.alloc(hir::Pat { hir_id: self.lower_node_id(p.id), kind, span: p.span }) + self.arena.alloc(hir::Pat { + hir_id: self.lower_node_id(p.id), + kind, + span: p.span, + default_binding_modes: true, + }) } /// Emit a friendly error for extra `..` patterns in a tuple/tuple struct/slice pattern. - fn ban_extra_rest_pat(&self, sp: Span, prev_sp: Span, ctx: &str) { + crate fn ban_extra_rest_pat(&self, sp: Span, prev_sp: Span, ctx: &str) { self.diagnostic() .struct_span_err(sp, &format!("`..` can only be used once per {} pattern", ctx)) .span_label(sp, &format!("can only be used once per {} pattern", ctx)) diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index ad926a810e6bf..84114fc773533 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -610,6 +610,9 @@ declare_features! ( /// Allows unsized fn parameters. (active, unsized_fn_params, "1.49.0", Some(48055), None), + /// Allows the use of destructuring assignments. + (active, destructuring_assignment, "1.49.0", Some(71126), None), + // ------------------------------------------------------------------------- // feature-group-end: actual feature gates // ------------------------------------------------------------------------- diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index b9ec18688c5f2..6767041ecee83 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -732,6 +732,9 @@ pub struct Pat<'hir> { pub hir_id: HirId, pub kind: PatKind<'hir>, pub span: Span, + // Whether to use default binding modes. + // At present, this is false only for destructuring assignment. + pub default_binding_modes: bool, } impl Pat<'_> { @@ -1680,6 +1683,9 @@ pub enum LocalSource { AsyncFn, /// A desugared `.await`. AwaitDesugar, + /// A desugared `expr = expr`, where the LHS is a tuple, struct or array. + /// The span is that of the `=` sign. + AssignDesugar(Span), } /// Hints at the original code for a `match _ { .. }`. 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 30b700a1d4f63..04d456936eba6 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -69,6 +69,7 @@ impl<'tcx> Visitor<'tcx> for MatchVisitor<'_, 'tcx> { hir::LocalSource::ForLoopDesugar => ("`for` loop binding", None), hir::LocalSource::AsyncFn => ("async fn binding", None), hir::LocalSource::AwaitDesugar => ("`await` future binding", None), + hir::LocalSource::AssignDesugar(_) => ("destructuring assignment binding", None), }; self.check_irrefutable(&loc.pat, msg, sp); self.check_patterns(&loc.pat); diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 1a6c45b6c80d2..2324dba80f543 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -434,6 +434,7 @@ symbols! { deref_mut, deref_target, derive, + destructuring_assignment, diagnostic, direct, discriminant_kind, diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index 324aa1a66a6d5..af19ad08c1d08 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -718,39 +718,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } - fn is_destructuring_place_expr(&self, expr: &'tcx hir::Expr<'tcx>) -> bool { - match &expr.kind { - ExprKind::Array(comps) | ExprKind::Tup(comps) => { - comps.iter().all(|e| self.is_destructuring_place_expr(e)) - } - ExprKind::Struct(_path, fields, rest) => { - rest.as_ref().map(|e| self.is_destructuring_place_expr(e)).unwrap_or(true) - && fields.iter().all(|f| self.is_destructuring_place_expr(&f.expr)) - } - _ => expr.is_syntactic_place_expr(), - } - } - pub(crate) fn check_lhs_assignable( &self, lhs: &'tcx hir::Expr<'tcx>, err_code: &'static str, expr_span: &Span, ) { - if !lhs.is_syntactic_place_expr() { - // FIXME: Make this use SessionDiagnostic once error codes can be dynamically set. - let mut err = self.tcx.sess.struct_span_err_with_code( - *expr_span, - "invalid left-hand side of assignment", - DiagnosticId::Error(err_code.into()), - ); - err.span_label(lhs.span, "cannot assign to this expression"); - if self.is_destructuring_place_expr(lhs) { - err.note("destructuring assignments are not currently supported"); - err.note("for more information, see https://github.com/rust-lang/rfcs/issues/372"); - } - err.emit(); + if lhs.is_syntactic_place_expr() { + return; } + + // FIXME: Make this use SessionDiagnostic once error codes can be dynamically set. + let mut err = self.tcx.sess.struct_span_err_with_code( + *expr_span, + "invalid left-hand side of assignment", + DiagnosticId::Error(err_code.into()), + ); + err.span_label(lhs.span, "cannot assign to this expression"); + err.emit(); } /// Type check assignment expression `expr` of form `lhs = rhs`. diff --git a/compiler/rustc_typeck/src/check/pat.rs b/compiler/rustc_typeck/src/check/pat.rs index 53bc2069b76ce..d0e6edfd9db30 100644 --- a/compiler/rustc_typeck/src/check/pat.rs +++ b/compiler/rustc_typeck/src/check/pat.rs @@ -270,6 +270,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// /// When the pattern is a path pattern, `opt_path_res` must be `Some(res)`. fn calc_adjust_mode(&self, pat: &'tcx Pat<'tcx>, opt_path_res: Option) -> AdjustMode { + // When we perform destructuring assignment, we disable default match bindings, which are + // unintuitive in this context. + if !pat.default_binding_modes { + return AdjustMode::Reset; + } match &pat.kind { // Type checking these product-like types successfully always require // that the expected type be of those types and not reference types. diff --git a/compiler/rustc_typeck/src/check/regionck.rs b/compiler/rustc_typeck/src/check/regionck.rs index ba0f22513a146..7b31b9f3915f4 100644 --- a/compiler/rustc_typeck/src/check/regionck.rs +++ b/compiler/rustc_typeck/src/check/regionck.rs @@ -577,7 +577,7 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> { fn link_pattern(&self, discr_cmt: PlaceWithHirId<'tcx>, root_pat: &hir::Pat<'_>) { debug!("link_pattern(discr_cmt={:?}, root_pat={:?})", discr_cmt, root_pat); ignore_err!(self.with_mc(|mc| { - mc.cat_pattern(discr_cmt, root_pat, |sub_cmt, hir::Pat { kind, span, hir_id }| { + mc.cat_pattern(discr_cmt, root_pat, |sub_cmt, hir::Pat { kind, span, hir_id, .. }| { // `ref x` pattern if let PatKind::Binding(..) = kind { if let Some(ty::BindByReference(mutbl)) = diff --git a/src/test/ui/bad/bad-expr-lhs.rs b/src/test/ui/bad/bad-expr-lhs.rs index d7cf1b7700514..39536f12e3bb5 100644 --- a/src/test/ui/bad/bad-expr-lhs.rs +++ b/src/test/ui/bad/bad-expr-lhs.rs @@ -1,10 +1,12 @@ fn main() { 1 = 2; //~ ERROR invalid left-hand side of assignment 1 += 2; //~ ERROR invalid left-hand side of assignment - (1, 2) = (3, 4); //~ ERROR invalid left-hand side of assignment + (1, 2) = (3, 4); //~ ERROR destructuring assignments are unstable + //~| ERROR invalid left-hand side of assignment + //~| ERROR invalid left-hand side of assignment let (a, b) = (1, 2); - (a, b) = (3, 4); //~ ERROR invalid left-hand side of assignment + (a, b) = (3, 4); //~ ERROR destructuring assignments are unstable None = Some(3); //~ ERROR invalid left-hand side of assignment } diff --git a/src/test/ui/bad/bad-expr-lhs.stderr b/src/test/ui/bad/bad-expr-lhs.stderr index a195e1054d099..d4b2193d09fc2 100644 --- a/src/test/ui/bad/bad-expr-lhs.stderr +++ b/src/test/ui/bad/bad-expr-lhs.stderr @@ -1,3 +1,25 @@ +error[E0658]: destructuring assignments are unstable + --> $DIR/bad-expr-lhs.rs:4:12 + | +LL | (1, 2) = (3, 4); + | ------ ^ + | | + | cannot assign to this expression + | + = note: see issue #71126 for more information + = help: add `#![feature(destructuring_assignment)]` to the crate attributes to enable + +error[E0658]: destructuring assignments are unstable + --> $DIR/bad-expr-lhs.rs:9:12 + | +LL | (a, b) = (3, 4); + | ------ ^ + | | + | cannot assign to this expression + | + = note: see issue #71126 for more information + = help: add `#![feature(destructuring_assignment)]` to the crate attributes to enable + error[E0070]: invalid left-hand side of assignment --> $DIR/bad-expr-lhs.rs:2:7 | @@ -18,30 +40,27 @@ error[E0070]: invalid left-hand side of assignment --> $DIR/bad-expr-lhs.rs:4:12 | LL | (1, 2) = (3, 4); - | ------ ^ - | | - | cannot assign to this expression + | - ^ + | | + | cannot assign to this expression error[E0070]: invalid left-hand side of assignment - --> $DIR/bad-expr-lhs.rs:7:12 - | -LL | (a, b) = (3, 4); - | ------ ^ - | | - | cannot assign to this expression + --> $DIR/bad-expr-lhs.rs:4:12 | - = note: destructuring assignments are not currently supported - = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 +LL | (1, 2) = (3, 4); + | - ^ + | | + | cannot assign to this expression error[E0070]: invalid left-hand side of assignment - --> $DIR/bad-expr-lhs.rs:9:10 + --> $DIR/bad-expr-lhs.rs:11:10 | LL | None = Some(3); | ---- ^ | | | cannot assign to this expression -error: aborting due to 5 previous errors +error: aborting due to 7 previous errors -Some errors have detailed explanations: E0067, E0070. +Some errors have detailed explanations: E0067, E0070, E0658. For more information about an error, try `rustc --explain E0067`. diff --git a/src/test/ui/destructuring-assignment/default-match-bindings-forbidden.rs b/src/test/ui/destructuring-assignment/default-match-bindings-forbidden.rs new file mode 100644 index 0000000000000..adecd0ff291f9 --- /dev/null +++ b/src/test/ui/destructuring-assignment/default-match-bindings-forbidden.rs @@ -0,0 +1,7 @@ +#![feature(destructuring_assignment)] + +fn main() { + let mut x = &0; + let mut y = &0; + (x, y) = &(1, 2); //~ ERROR mismatched types +} diff --git a/src/test/ui/destructuring-assignment/default-match-bindings-forbidden.stderr b/src/test/ui/destructuring-assignment/default-match-bindings-forbidden.stderr new file mode 100644 index 0000000000000..e6161fdfa2441 --- /dev/null +++ b/src/test/ui/destructuring-assignment/default-match-bindings-forbidden.stderr @@ -0,0 +1,14 @@ +error[E0308]: mismatched types + --> $DIR/default-match-bindings-forbidden.rs:6:5 + | +LL | (x, y) = &(1, 2); + | ^^^^^^ ------- this expression has type `&({integer}, {integer})` + | | + | expected reference, found tuple + | + = note: expected type `&({integer}, {integer})` + found tuple `(_, _)` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/destructuring-assignment/note-unsupported.rs b/src/test/ui/destructuring-assignment/note-unsupported.rs index 876c9efea2647..e0cb9dc9158e2 100644 --- a/src/test/ui/destructuring-assignment/note-unsupported.rs +++ b/src/test/ui/destructuring-assignment/note-unsupported.rs @@ -3,23 +3,24 @@ struct S { x: u8, y: u8 } fn main() { let (a, b) = (1, 2); - (a, b) = (3, 4); //~ ERROR invalid left-hand side of assignment + (a, b) = (3, 4); //~ ERROR destructuring assignments are unstable (a, b) += (3, 4); //~ ERROR invalid left-hand side of assignment - //~^ ERROR binary assignment operation `+=` cannot be applied + //~| ERROR binary assignment operation `+=` cannot be applied [a, b] = [3, 4]; //~ ERROR invalid left-hand side of assignment [a, b] += [3, 4]; //~ ERROR invalid left-hand side of assignment - //~^ ERROR binary assignment operation `+=` cannot be applied + //~| ERROR binary assignment operation `+=` cannot be applied let s = S { x: 3, y: 4 }; S { x: a, y: b } = s; //~ ERROR invalid left-hand side of assignment S { x: a, y: b } += s; //~ ERROR invalid left-hand side of assignment - //~^ ERROR binary assignment operation `+=` cannot be applied + //~| ERROR binary assignment operation `+=` cannot be applied - S { x: a, ..s } = S { x: 3, y: 4 }; //~ ERROR invalid left-hand side of assignment + S { x: a, ..s } = S { x: 3, y: 4 }; + //~^ ERROR invalid left-hand side of assignment let c = 3; - ((a, b), c) = ((3, 4), 5); //~ ERROR invalid left-hand side of assignment + ((a, b), c) = ((3, 4), 5); //~ ERROR destructuring assignments are unstable } diff --git a/src/test/ui/destructuring-assignment/note-unsupported.stderr b/src/test/ui/destructuring-assignment/note-unsupported.stderr index d4e25930d22d7..c5543fab825eb 100644 --- a/src/test/ui/destructuring-assignment/note-unsupported.stderr +++ b/src/test/ui/destructuring-assignment/note-unsupported.stderr @@ -1,4 +1,4 @@ -error[E0070]: invalid left-hand side of assignment +error[E0658]: destructuring assignments are unstable --> $DIR/note-unsupported.rs:6:12 | LL | (a, b) = (3, 4); @@ -6,8 +6,19 @@ LL | (a, b) = (3, 4); | | | cannot assign to this expression | - = note: destructuring assignments are not currently supported - = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 + = note: see issue #71126 for more information + = help: add `#![feature(destructuring_assignment)]` to the crate attributes to enable + +error[E0658]: destructuring assignments are unstable + --> $DIR/note-unsupported.rs:25:17 + | +LL | ((a, b), c) = ((3, 4), 5); + | ----------- ^ + | | + | cannot assign to this expression + | + = note: see issue #71126 for more information + = help: add `#![feature(destructuring_assignment)]` to the crate attributes to enable error[E0368]: binary assignment operation `+=` cannot be applied to type `({integer}, {integer})` --> $DIR/note-unsupported.rs:7:5 @@ -24,9 +35,6 @@ LL | (a, b) += (3, 4); | ------ ^^ | | | cannot assign to this expression - | - = note: destructuring assignments are not currently supported - = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 error[E0070]: invalid left-hand side of assignment --> $DIR/note-unsupported.rs:10:12 @@ -35,9 +43,6 @@ LL | [a, b] = [3, 4]; | ------ ^ | | | cannot assign to this expression - | - = note: destructuring assignments are not currently supported - = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 error[E0368]: binary assignment operation `+=` cannot be applied to type `[{integer}; 2]` --> $DIR/note-unsupported.rs:11:5 @@ -54,9 +59,6 @@ LL | [a, b] += [3, 4]; | ------ ^^ | | | cannot assign to this expression - | - = note: destructuring assignments are not currently supported - = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 error[E0070]: invalid left-hand side of assignment --> $DIR/note-unsupported.rs:16:22 @@ -65,9 +67,6 @@ LL | S { x: a, y: b } = s; | ---------------- ^ | | | cannot assign to this expression - | - = note: destructuring assignments are not currently supported - = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 error[E0368]: binary assignment operation `+=` cannot be applied to type `S` --> $DIR/note-unsupported.rs:17:5 @@ -86,9 +85,6 @@ LL | S { x: a, y: b } += s; | ---------------- ^^ | | | cannot assign to this expression - | - = note: destructuring assignments are not currently supported - = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 error[E0070]: invalid left-hand side of assignment --> $DIR/note-unsupported.rs:20:21 @@ -97,22 +93,8 @@ LL | S { x: a, ..s } = S { x: 3, y: 4 }; | --------------- ^ | | | cannot assign to this expression - | - = note: destructuring assignments are not currently supported - = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 - -error[E0070]: invalid left-hand side of assignment - --> $DIR/note-unsupported.rs:24:17 - | -LL | ((a, b), c) = ((3, 4), 5); - | ----------- ^ - | | - | cannot assign to this expression - | - = note: destructuring assignments are not currently supported - = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 error: aborting due to 11 previous errors -Some errors have detailed explanations: E0067, E0070, E0368. +Some errors have detailed explanations: E0067, E0070, E0368, E0658. For more information about an error, try `rustc --explain E0067`. diff --git a/src/test/ui/destructuring-assignment/tuple_destructure.rs b/src/test/ui/destructuring-assignment/tuple_destructure.rs new file mode 100644 index 0000000000000..16aafc4693f3f --- /dev/null +++ b/src/test/ui/destructuring-assignment/tuple_destructure.rs @@ -0,0 +1,37 @@ +// run-pass + +#![feature(destructuring_assignment)] + +fn main() { + let (mut a, mut b); + (a, b) = (0, 1); + assert_eq!((a, b), (0, 1)); + (b, a) = (a, b); + assert_eq!((a, b), (1, 0)); + (a, .., b) = (1, 2); + assert_eq!((a, b), (1, 2)); + (.., a) = (1, 2); + assert_eq!((a, b), (2, 2)); + (..) = (3, 4); + assert_eq!((a, b), (2, 2)); + (b, ..) = (5, 6, 7); + assert_eq!(b, 5); + + // Test for a non-Copy type (String): + let (mut c, mut d); + (c, d) = ("c".to_owned(), "d".to_owned()); + assert_eq!(c, "c"); + assert_eq!(d, "d"); + (d, c) = (c, d); + assert_eq!(c, "d"); + assert_eq!(d, "c"); + + // Test nesting/parentheses: + ((a, b)) = (0, 1); + assert_eq!((a, b), (0, 1)); + (((a, b)), (c)) = ((2, 3), d); + assert_eq!((a, b), (2, 3)); + assert_eq!(c, "c"); + ((a, .., b), .., (..)) = ((4, 5), ()); + assert_eq!((a, b), (4, 5)); +} diff --git a/src/test/ui/destructuring-assignment/tuple_destructure_fail.rs b/src/test/ui/destructuring-assignment/tuple_destructure_fail.rs new file mode 100644 index 0000000000000..b76f4968e6249 --- /dev/null +++ b/src/test/ui/destructuring-assignment/tuple_destructure_fail.rs @@ -0,0 +1,10 @@ +#![feature(destructuring_assignment)] + +const C: i32 = 1; + +fn main() { + let (mut a, mut b); + (a, .., b, ..) = (0, 1); //~ ERROR `..` can only be used once per tuple pattern + (a, a, b) = (1, 2); //~ ERROR mismatched types + (C, ..) = (0,1); //~ ERROR invalid left-hand side of assignment +} diff --git a/src/test/ui/destructuring-assignment/tuple_destructure_fail.stderr b/src/test/ui/destructuring-assignment/tuple_destructure_fail.stderr new file mode 100644 index 0000000000000..a60e1cb1eec62 --- /dev/null +++ b/src/test/ui/destructuring-assignment/tuple_destructure_fail.stderr @@ -0,0 +1,31 @@ +error: `..` can only be used once per tuple pattern + --> $DIR/tuple_destructure_fail.rs:7:16 + | +LL | (a, .., b, ..) = (0, 1); + | -- ^^ can only be used once per tuple pattern + | | + | previously used here + +error[E0308]: mismatched types + --> $DIR/tuple_destructure_fail.rs:8:5 + | +LL | (a, a, b) = (1, 2); + | ^^^^^^^^^ ------ this expression has type `({integer}, {integer})` + | | + | expected a tuple with 2 elements, found one with 3 elements + | + = note: expected type `({integer}, {integer})` + found tuple `(_, _, _)` + +error[E0070]: invalid left-hand side of assignment + --> $DIR/tuple_destructure_fail.rs:9:13 + | +LL | (C, ..) = (0,1); + | - ^ + | | + | cannot assign to this expression + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0070, E0308. +For more information about an error, try `rustc --explain E0070`. diff --git a/src/test/ui/destructuring-assignment/warn-unused-duplication.rs b/src/test/ui/destructuring-assignment/warn-unused-duplication.rs new file mode 100644 index 0000000000000..c1c5c2cd3cebb --- /dev/null +++ b/src/test/ui/destructuring-assignment/warn-unused-duplication.rs @@ -0,0 +1,23 @@ +// run-pass + +#![feature(destructuring_assignment)] + +#![warn(unused_assignments)] + +fn main() { + let mut a; + // Assignment occurs left-to-right. + // However, we emit warnings when this happens, so it is clear that this is happening. + (a, a) = (0, 1); //~ WARN value assigned to `a` is never read + assert_eq!(a, 1); + + // We can't always tell when a variable is being assigned to twice, which is why we don't try + // to emit an error, which would be fallible. + let mut x = 1; + (*foo(&mut x), *foo(&mut x)) = (5, 6); + assert_eq!(x, 6); +} + +fn foo<'a>(x: &'a mut u32) -> &'a mut u32 { + x +} diff --git a/src/test/ui/destructuring-assignment/warn-unused-duplication.stderr b/src/test/ui/destructuring-assignment/warn-unused-duplication.stderr new file mode 100644 index 0000000000000..b87ef6f1571c1 --- /dev/null +++ b/src/test/ui/destructuring-assignment/warn-unused-duplication.stderr @@ -0,0 +1,15 @@ +warning: value assigned to `a` is never read + --> $DIR/warn-unused-duplication.rs:11:6 + | +LL | (a, a) = (0, 1); + | ^ + | +note: the lint level is defined here + --> $DIR/warn-unused-duplication.rs:5:9 + | +LL | #![warn(unused_assignments)] + | ^^^^^^^^^^^^^^^^^^ + = help: maybe it is overwritten before being read? + +warning: 1 warning emitted + diff --git a/src/test/ui/feature-gates/feature-gate-destructuring_assignment.rs b/src/test/ui/feature-gates/feature-gate-destructuring_assignment.rs new file mode 100644 index 0000000000000..e7801f0e8ec2b --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-destructuring_assignment.rs @@ -0,0 +1,4 @@ +fn main() { + let (a, b) = (0, 1); + (a, b) = (2, 3); //~ ERROR destructuring assignments are unstable +} diff --git a/src/test/ui/feature-gates/feature-gate-destructuring_assignment.stderr b/src/test/ui/feature-gates/feature-gate-destructuring_assignment.stderr new file mode 100644 index 0000000000000..62e71decb32a0 --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-destructuring_assignment.stderr @@ -0,0 +1,14 @@ +error[E0658]: destructuring assignments are unstable + --> $DIR/feature-gate-destructuring_assignment.rs:3:12 + | +LL | (a, b) = (2, 3); + | ------ ^ + | | + | cannot assign to this expression + | + = note: see issue #71126 for more information + = help: add `#![feature(destructuring_assignment)]` to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. From 67d0db6b008f98c1a1ba8ed6c267105433250fc9 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 21 Oct 2020 22:00:32 -0400 Subject: [PATCH 25/29] Fix handling of item names for HIR - Handle variants, fields, macros in `Node::ident()` - Handle the crate root in `opt_item_name` - Factor out `item_name_from_def_id` to reduce duplication - Look at HIR before the DefId for `opt_item_name` This gives accurate spans, which are not available from serialized metadata. - Don't panic on the crate root in `opt_item_name` - Add comments --- compiler/rustc_hir/src/hir.rs | 3 ++ compiler/rustc_middle/src/hir/map/mod.rs | 2 +- compiler/rustc_middle/src/ty/mod.rs | 65 ++++++++++++++++-------- 3 files changed, 49 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index b9ec18688c5f2..3c72937ad3134 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2677,6 +2677,9 @@ impl<'hir> Node<'hir> { Node::TraitItem(TraitItem { ident, .. }) | Node::ImplItem(ImplItem { ident, .. }) | Node::ForeignItem(ForeignItem { ident, .. }) + | Node::Field(StructField { ident, .. }) + | Node::Variant(Variant { ident, .. }) + | Node::MacroDef(MacroDef { ident, .. }) | Node::Item(Item { ident, .. }) => Some(*ident), _ => None, } diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index 106fa8c78fa28..d86e898719557 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -478,7 +478,7 @@ impl<'hir> Map<'hir> { } pub fn get_if_local(&self, id: DefId) -> Option> { - id.as_local().map(|id| self.get(self.local_def_id_to_hir_id(id))) + id.as_local().and_then(|id| self.find(self.local_def_id_to_hir_id(id))) } pub fn get_generics(&self, id: DefId) -> Option<&'hir Generics<'hir>> { diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index aa5a696b09c3b..c69dabda54937 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -2795,10 +2795,52 @@ impl<'tcx> TyCtxt<'tcx> { .filter(|item| item.kind == AssocKind::Fn && item.defaultness.has_value()) } + fn item_name_from_hir(self, def_id: DefId) -> Option { + self.hir().get_if_local(def_id).and_then(|node| node.ident()) + } + + fn item_name_from_def_id(self, def_id: DefId) -> Option { + if def_id.index == CRATE_DEF_INDEX { + Some(self.original_crate_name(def_id.krate)) + } else { + let def_key = self.def_key(def_id); + match def_key.disambiguated_data.data { + // The name of a constructor is that of its parent. + rustc_hir::definitions::DefPathData::Ctor => self.item_name_from_def_id(DefId { + krate: def_id.krate, + index: def_key.parent.unwrap(), + }), + _ => def_key.disambiguated_data.data.get_opt_name(), + } + } + } + + /// Look up the name of an item across crates. This does not look at HIR. + /// + /// When possible, this function should be used for cross-crate lookups over + /// [`opt_item_name`] to avoid invalidating the incremental cache. If you + /// need to handle items without a name, or HIR items that will not be + /// serialized cross-crate, or if you need the span of the item, use + /// [`opt_item_name`] instead. + /// + /// [`opt_item_name`]: Self::opt_item_name + pub fn item_name(self, id: DefId) -> Symbol { + // Look at cross-crate items first to avoid invalidating the incremental cache + // unless we have to. + self.item_name_from_def_id(id) + .or_else(|| self.item_name_from_hir(id).map(|ident| ident.name)) + .unwrap_or_else(|| { + bug!("item_name: no name for {:?}", self.def_path(id)); + }) + } + + /// Look up the name and span of an item or [`Node`]. + /// + /// See [`item_name`][Self::item_name] for more information. pub fn opt_item_name(self, def_id: DefId) -> Option { - def_id - .as_local() - .and_then(|def_id| self.hir().get(self.hir().local_def_id_to_hir_id(def_id)).ident()) + // Look at the HIR first so the span will be correct if this is a local item. + self.item_name_from_hir(def_id) + .or_else(|| self.item_name_from_def_id(def_id).map(Ident::with_dummy_span)) } pub fn opt_associated_item(self, def_id: DefId) -> Option<&'tcx AssocItem> { @@ -2921,23 +2963,6 @@ impl<'tcx> TyCtxt<'tcx> { } } - pub fn item_name(self, id: DefId) -> Symbol { - if id.index == CRATE_DEF_INDEX { - self.original_crate_name(id.krate) - } else { - let def_key = self.def_key(id); - match def_key.disambiguated_data.data { - // The name of a constructor is that of its parent. - rustc_hir::definitions::DefPathData::Ctor => { - self.item_name(DefId { krate: id.krate, index: def_key.parent.unwrap() }) - } - _ => def_key.disambiguated_data.data.get_opt_name().unwrap_or_else(|| { - bug!("item_name: no name for {:?}", self.def_path(id)); - }), - } - } - } - /// Returns the possibly-auto-generated MIR of a `(DefId, Subst)` pair. pub fn instance_mir(self, instance: ty::InstanceDef<'tcx>) -> &'tcx Body<'tcx> { match instance { From f60fd4963207bd6ac4a1c93c7c2674a7c321ffa8 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 7 Nov 2020 10:34:00 -0500 Subject: [PATCH 26/29] Remove unused `from_hir` call --- compiler/rustc_middle/src/ty/mod.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index c69dabda54937..0042b4a3a4279 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -2827,11 +2827,9 @@ impl<'tcx> TyCtxt<'tcx> { pub fn item_name(self, id: DefId) -> Symbol { // Look at cross-crate items first to avoid invalidating the incremental cache // unless we have to. - self.item_name_from_def_id(id) - .or_else(|| self.item_name_from_hir(id).map(|ident| ident.name)) - .unwrap_or_else(|| { - bug!("item_name: no name for {:?}", self.def_path(id)); - }) + self.item_name_from_def_id(id).unwrap_or_else(|| { + bug!("item_name: no name for {:?}", self.def_path(id)); + }) } /// Look up the name and span of an item or [`Node`]. From 9dc5dfb97504c538bc72f367a77bb9f714c30097 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 7 Nov 2020 19:01:06 -0700 Subject: [PATCH 27/29] Fix tab focus on restyled switches Setting a checkbox to `display:none` makes it impossible to tab onto it, which makes the rustdoc settings page completely keyboard inaccessible. --- src/librustdoc/html/static/settings.css | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/static/settings.css b/src/librustdoc/html/static/settings.css index 4bacd7b245bbc..fb8990b30e2ed 100644 --- a/src/librustdoc/html/static/settings.css +++ b/src/librustdoc/html/static/settings.css @@ -26,7 +26,8 @@ } .toggle input { - display: none; + opacity: 0; + position: absolute; } .select-wrapper { @@ -90,7 +91,7 @@ input:checked + .slider { } input:focus + .slider { - box-shadow: 0 0 1px #2196F3; + box-shadow: 0 0 0 2px #0a84ff, 0 0 0 6px rgba(10, 132, 255, 0.3); } input:checked + .slider:before { From b13817a79566cf313b4bebed3a5caf5ea8879f4e Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Sun, 8 Nov 2020 09:43:51 -0500 Subject: [PATCH 28/29] Avoid overlapping cfg attributes when both macOS and aarch64 --- .../sys/unix/process/process_common/tests.rs | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/library/std/src/sys/unix/process/process_common/tests.rs b/library/std/src/sys/unix/process/process_common/tests.rs index e72fbf0beb4a5..10aa34e9443b7 100644 --- a/library/std/src/sys/unix/process/process_common/tests.rs +++ b/library/std/src/sys/unix/process/process_common/tests.rs @@ -14,17 +14,22 @@ macro_rules! t { }; } -// See #14232 for more information, but it appears that signal delivery to a -// newly spawned process may just be raced in the macOS, so to prevent this -// test from being flaky we ignore it on macOS. #[test] -#[cfg_attr(target_os = "macos", ignore)] -// When run under our current QEMU emulation test suite this test fails, -// although the reason isn't very clear as to why. For now this test is -// ignored there. -#[cfg_attr(target_arch = "arm", ignore)] -#[cfg_attr(target_arch = "aarch64", ignore)] -#[cfg_attr(target_arch = "riscv64", ignore)] +#[cfg_attr( + any( + // See #14232 for more information, but it appears that signal delivery to a + // newly spawned process may just be raced in the macOS, so to prevent this + // test from being flaky we ignore it on macOS. + target_os = "macos", + // When run under our current QEMU emulation test suite this test fails, + // although the reason isn't very clear as to why. For now this test is + // ignored there. + target_arch = "arm", + target_arch = "aarch64", + target_arch = "riscv64", + ), + ignore +)] fn test_process_mask() { unsafe { // Test to make sure that a signal mask does not get inherited. From 39046172ab91805efb79a55870c2ced2d61cfc3a Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sun, 8 Nov 2020 16:40:00 +0100 Subject: [PATCH 29/29] Nicer hunk headers for rust files I found this trick at Before the hunk headers for changes in methods would refer to the impl: ```diff diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 1c6937e685c..fa4264d729b 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -678,7 +678,7 @@ impl<'a, 'tcx> HashStable> for TypeckResults<'tcx> { ref closure_captures, ref generator_interior_types, } = *self; - + // foo hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| { type_dependent_defs.hash_stable(hcx, hasher); field_indices.hash_stable(hcx, hasher); ``` After the hunk headers refer to the actual function signature: ```diff diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 1c6937e685c..fa4264d729b 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -678,7 +678,7 @@ fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHas ref closure_captures, ref generator_interior_types, } = *self; - + // foo hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| { type_dependent_defs.hash_stable(hcx, hasher); field_indices.hash_stable(hcx, hasher); ``` When the function signature is visible, it will use the function signature of the previous method as hunk header: ```diff diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 1c6937e685c..63058dfc837 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -992,6 +992,7 @@ pub fn typeck_opt_const_arg( } pub fn alloc_steal_mir(self, mir: Body<'tcx>) -> &'tcx Steal> { + // foo self.arena.alloc(Steal::new(mir)) } ``` --- .gitattributes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitattributes b/.gitattributes index a7de7ce85593c..2c5c37007d5bc 100644 --- a/.gitattributes +++ b/.gitattributes @@ -3,7 +3,7 @@ * text=auto eol=lf *.cpp rust *.h rust -*.rs rust +*.rs rust diff=rust *.fixed linguist-language=Rust src/etc/installer/gfx/* binary *.woff binary