From d72d5f48c2c534de7382a601e4619de923b7f6a9 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 21 Oct 2020 17:56:09 -0700 Subject: [PATCH 01/42] Dogfood Duration API in std::time tests This expands time's test suite to use more and in more places the range of methods and constants added to Duration in recent proposals for the sake of testing more API surface area and improving legibility. --- library/core/tests/time.rs | 90 +++++++++++++++++------------------ library/std/src/lib.rs | 1 + library/std/src/time/tests.rs | 32 ++++++------- 3 files changed, 62 insertions(+), 61 deletions(-) diff --git a/library/core/tests/time.rs b/library/core/tests/time.rs index 7c43885040b3e..6508e78429c0e 100644 --- a/library/core/tests/time.rs +++ b/library/core/tests/time.rs @@ -108,24 +108,26 @@ fn sub() { #[test] fn checked_sub() { - let zero = Duration::new(0, 0); - let one_nano = Duration::new(0, 1); - let one_sec = Duration::new(1, 0); - assert_eq!(one_nano.checked_sub(zero), Some(Duration::new(0, 1))); - assert_eq!(one_sec.checked_sub(one_nano), Some(Duration::new(0, 999_999_999))); - assert_eq!(zero.checked_sub(one_nano), None); - assert_eq!(zero.checked_sub(one_sec), None); + let zero = Duration::zero(); + assert_eq!(Duration::NANOSECOND.checked_sub(zero), Some(Duration::NANOSECOND)); + assert_eq!( + Duration::SECOND.checked_sub(Duration::NANOSECOND), + Some(Duration::new(0, 999_999_999)) + ); + assert_eq!(zero.checked_sub(Duration::NANOSECOND), None); + assert_eq!(zero.checked_sub(Duration::SECOND), None); } #[test] fn saturating_sub() { let zero = Duration::new(0, 0); - let one_nano = Duration::new(0, 1); - let one_sec = Duration::new(1, 0); - assert_eq!(one_nano.saturating_sub(zero), Duration::new(0, 1)); - assert_eq!(one_sec.saturating_sub(one_nano), Duration::new(0, 999_999_999)); - assert_eq!(zero.saturating_sub(one_nano), Duration::MIN); - assert_eq!(zero.saturating_sub(one_sec), Duration::MIN); + assert_eq!(Duration::NANOSECOND.saturating_sub(zero), Duration::NANOSECOND); + assert_eq!( + Duration::SECOND.saturating_sub(Duration::NANOSECOND), + Duration::new(0, 999_999_999) + ); + assert_eq!(zero.saturating_sub(Duration::NANOSECOND), Duration::MIN); + assert_eq!(zero.saturating_sub(Duration::SECOND), Duration::MIN); } #[test] @@ -343,80 +345,78 @@ fn duration_const() { const IS_ZERO: bool = ZERO.is_zero(); assert!(IS_ZERO); - const ONE: Duration = Duration::new(1, 0); - - const SECONDS: u64 = ONE.as_secs(); + const SECONDS: u64 = Duration::SECOND.as_secs(); assert_eq!(SECONDS, 1); const FROM_SECONDS: Duration = Duration::from_secs(1); - assert_eq!(FROM_SECONDS, ONE); + assert_eq!(FROM_SECONDS, Duration::SECOND); - const SECONDS_F32: f32 = ONE.as_secs_f32(); + const SECONDS_F32: f32 = Duration::SECOND.as_secs_f32(); assert_eq!(SECONDS_F32, 1.0); const FROM_SECONDS_F32: Duration = Duration::from_secs_f32(1.0); - assert_eq!(FROM_SECONDS_F32, ONE); + assert_eq!(FROM_SECONDS_F32, Duration::SECOND); - const SECONDS_F64: f64 = ONE.as_secs_f64(); + const SECONDS_F64: f64 = Duration::SECOND.as_secs_f64(); assert_eq!(SECONDS_F64, 1.0); const FROM_SECONDS_F64: Duration = Duration::from_secs_f64(1.0); - assert_eq!(FROM_SECONDS_F64, ONE); + assert_eq!(FROM_SECONDS_F64, Duration::SECOND); - const MILLIS: u128 = ONE.as_millis(); + const MILLIS: u128 = Duration::SECOND.as_millis(); assert_eq!(MILLIS, 1_000); const FROM_MILLIS: Duration = Duration::from_millis(1_000); - assert_eq!(FROM_MILLIS, ONE); + assert_eq!(FROM_MILLIS, Duration::SECOND); - const MICROS: u128 = ONE.as_micros(); + const MICROS: u128 = Duration::SECOND.as_micros(); assert_eq!(MICROS, 1_000_000); const FROM_MICROS: Duration = Duration::from_micros(1_000_000); - assert_eq!(FROM_MICROS, ONE); + assert_eq!(FROM_MICROS, Duration::SECOND); - const NANOS: u128 = ONE.as_nanos(); + const NANOS: u128 = Duration::SECOND.as_nanos(); assert_eq!(NANOS, 1_000_000_000); const FROM_NANOS: Duration = Duration::from_nanos(1_000_000_000); - assert_eq!(FROM_NANOS, ONE); + assert_eq!(FROM_NANOS, Duration::SECOND); const MAX: Duration = Duration::new(u64::MAX, 999_999_999); - const CHECKED_ADD: Option = MAX.checked_add(ONE); + const CHECKED_ADD: Option = MAX.checked_add(Duration::SECOND); assert_eq!(CHECKED_ADD, None); - const CHECKED_SUB: Option = ZERO.checked_sub(ONE); + const CHECKED_SUB: Option = ZERO.checked_sub(Duration::SECOND); assert_eq!(CHECKED_SUB, None); - const CHECKED_MUL: Option = ONE.checked_mul(1); - assert_eq!(CHECKED_MUL, Some(ONE)); + const CHECKED_MUL: Option = Duration::SECOND.checked_mul(1); + assert_eq!(CHECKED_MUL, Some(Duration::SECOND)); - const MUL_F32: Duration = ONE.mul_f32(1.0); - assert_eq!(MUL_F32, ONE); + const MUL_F32: Duration = Duration::SECOND.mul_f32(1.0); + assert_eq!(MUL_F32, Duration::SECOND); - const MUL_F64: Duration = ONE.mul_f64(1.0); - assert_eq!(MUL_F64, ONE); + const MUL_F64: Duration = Duration::SECOND.mul_f64(1.0); + assert_eq!(MUL_F64, Duration::SECOND); - const CHECKED_DIV: Option = ONE.checked_div(1); - assert_eq!(CHECKED_DIV, Some(ONE)); + const CHECKED_DIV: Option = Duration::SECOND.checked_div(1); + assert_eq!(CHECKED_DIV, Some(Duration::SECOND)); - const DIV_F32: Duration = ONE.div_f32(1.0); - assert_eq!(DIV_F32, ONE); + const DIV_F32: Duration = Duration::SECOND.div_f32(1.0); + assert_eq!(DIV_F32, Duration::SECOND); - const DIV_F64: Duration = ONE.div_f64(1.0); - assert_eq!(DIV_F64, ONE); + const DIV_F64: Duration = Duration::SECOND.div_f64(1.0); + assert_eq!(DIV_F64, Duration::SECOND); - const DIV_DURATION_F32: f32 = ONE.div_duration_f32(ONE); + const DIV_DURATION_F32: f32 = Duration::SECOND.div_duration_f32(Duration::SECOND); assert_eq!(DIV_DURATION_F32, 1.0); - const DIV_DURATION_F64: f64 = ONE.div_duration_f64(ONE); + const DIV_DURATION_F64: f64 = Duration::SECOND.div_duration_f64(Duration::SECOND); assert_eq!(DIV_DURATION_F64, 1.0); - const SATURATING_ADD: Duration = MAX.saturating_add(ONE); + const SATURATING_ADD: Duration = MAX.saturating_add(Duration::SECOND); assert_eq!(SATURATING_ADD, MAX); - const SATURATING_SUB: Duration = ZERO.saturating_sub(ONE); + const SATURATING_SUB: Duration = ZERO.saturating_sub(Duration::SECOND); assert_eq!(SATURATING_SUB, ZERO); const SATURATING_MUL: Duration = MAX.saturating_mul(2); diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 30e7a7f3c3b10..881e387c7c81d 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -255,6 +255,7 @@ #![feature(doc_spotlight)] #![feature(dropck_eyepatch)] #![feature(duration_constants)] +#![feature(duration_zero)] #![feature(exact_size_is_empty)] #![feature(exhaustive_patterns)] #![feature(extend_one)] diff --git a/library/std/src/time/tests.rs b/library/std/src/time/tests.rs index 783bf49f31544..44c06bac9507c 100644 --- a/library/std/src/time/tests.rs +++ b/library/std/src/time/tests.rs @@ -5,7 +5,7 @@ macro_rules! assert_almost_eq { let (a, b) = ($a, $b); if a != b { let (a, b) = if a > b { (a, b) } else { (b, a) }; - assert!(a - Duration::new(0, 1000) <= b, "{:?} is not almost equal to {:?}", a, b); + assert!(a - Duration::from_micros(1) <= b, "{:?} is not almost equal to {:?}", a, b); } }}; } @@ -34,7 +34,7 @@ fn instant_math() { assert_almost_eq!(b - dur, a); assert_almost_eq!(a + dur, b); - let second = Duration::new(1, 0); + let second = Duration::SECOND; assert_almost_eq!(a - second + second, a); assert_almost_eq!(a.checked_sub(second).unwrap().checked_add(second).unwrap(), a); @@ -65,24 +65,24 @@ fn instant_math_is_associative() { #[should_panic] fn instant_duration_since_panic() { let a = Instant::now(); - (a - Duration::new(1, 0)).duration_since(a); + (a - Duration::SECOND).duration_since(a); } #[test] fn instant_checked_duration_since_nopanic() { let now = Instant::now(); - let earlier = now - Duration::new(1, 0); - let later = now + Duration::new(1, 0); + let earlier = now - Duration::SECOND; + let later = now + Duration::SECOND; assert_eq!(earlier.checked_duration_since(now), None); - assert_eq!(later.checked_duration_since(now), Some(Duration::new(1, 0))); - assert_eq!(now.checked_duration_since(now), Some(Duration::new(0, 0))); + assert_eq!(later.checked_duration_since(now), Some(Duration::SECOND)); + assert_eq!(now.checked_duration_since(now), Some(Duration::zero())); } #[test] fn instant_saturating_duration_since_nopanic() { let a = Instant::now(); - let ret = (a - Duration::new(1, 0)).saturating_duration_since(a); - assert_eq!(ret, Duration::new(0, 0)); + let ret = (a - Duration::SECOND).saturating_duration_since(a); + assert_eq!(ret, Duration::zero()); } #[test] @@ -90,7 +90,7 @@ fn system_time_math() { let a = SystemTime::now(); let b = SystemTime::now(); match b.duration_since(a) { - Ok(dur) if dur == Duration::new(0, 0) => { + Ok(dur) if dur == Duration::zero() => { assert_almost_eq!(a, b); } Ok(dur) => { @@ -106,16 +106,16 @@ fn system_time_math() { } } - let second = Duration::new(1, 0); + let second = Duration::SECOND; assert_almost_eq!(a.duration_since(a - second).unwrap(), second); assert_almost_eq!(a.duration_since(a + second).unwrap_err().duration(), second); assert_almost_eq!(a - second + second, a); assert_almost_eq!(a.checked_sub(second).unwrap().checked_add(second).unwrap(), a); - let one_second_from_epoch = UNIX_EPOCH + Duration::new(1, 0); + let one_second_from_epoch = UNIX_EPOCH + Duration::SECOND; let one_second_from_epoch2 = - UNIX_EPOCH + Duration::new(0, 500_000_000) + Duration::new(0, 500_000_000); + UNIX_EPOCH + Duration::from_millis(500) + Duration::from_millis(500); assert_eq!(one_second_from_epoch, one_second_from_epoch2); // checked_add_duration will not panic on overflow @@ -141,12 +141,12 @@ fn system_time_elapsed() { #[test] fn since_epoch() { let ts = SystemTime::now(); - let a = ts.duration_since(UNIX_EPOCH + Duration::new(1, 0)).unwrap(); + let a = ts.duration_since(UNIX_EPOCH + Duration::SECOND).unwrap(); let b = ts.duration_since(UNIX_EPOCH).unwrap(); assert!(b > a); - assert_eq!(b - a, Duration::new(1, 0)); + assert_eq!(b - a, Duration::SECOND); - let thirty_years = Duration::new(1, 0) * 60 * 60 * 24 * 365 * 30; + let thirty_years = Duration::SECOND * 60 * 60 * 24 * 365 * 30; // Right now for CI this test is run in an emulator, and apparently the // aarch64 emulator's sense of time is that we're still living in the From ef027a1eed4cc8d34d1a4bf9222369e7fef48e93 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 21 Oct 2020 18:18:18 -0700 Subject: [PATCH 02/42] Duration::zero() -> Duration::ZERO Duration::ZERO composes better with match and various other things, at the cost of an occasional parens, and results in less work for the optimizer, so let's use that instead. --- library/core/src/time.rs | 35 ++++++++++++++++------------------- library/core/tests/time.rs | 25 ++++++++++--------------- library/std/src/time/tests.rs | 6 +++--- 3 files changed, 29 insertions(+), 37 deletions(-) diff --git a/library/core/src/time.rs b/library/core/src/time.rs index 6dc542dee58e6..cc9afaff888ae 100644 --- a/library/core/src/time.rs +++ b/library/core/src/time.rs @@ -108,6 +108,21 @@ impl Duration { #[unstable(feature = "duration_constants", issue = "57391")] pub const NANOSECOND: Duration = Duration::from_nanos(1); + /// A duration of zero time. + /// + /// # Examples + /// + /// ``` + /// #![feature(duration_zero)] + /// use std::time::Duration; + /// + /// let duration = Duration::ZERO; + /// assert!(duration.is_zero()); + /// assert_eq!(duration.as_nanos(), 0); + /// ``` + #[unstable(feature = "duration_zero", issue = "73544")] + pub const ZERO: Duration = Duration::from_nanos(0); + /// The minimum duration. /// /// # Examples @@ -166,24 +181,6 @@ impl Duration { Duration { secs, nanos } } - /// Creates a new `Duration` that spans no time. - /// - /// # Examples - /// - /// ``` - /// #![feature(duration_zero)] - /// use std::time::Duration; - /// - /// let duration = Duration::zero(); - /// assert!(duration.is_zero()); - /// assert_eq!(duration.as_nanos(), 0); - /// ``` - #[unstable(feature = "duration_zero", issue = "73544")] - #[inline] - pub const fn zero() -> Duration { - Duration { secs: 0, nanos: 0 } - } - /// Creates a new `Duration` from the specified number of whole seconds. /// /// # Examples @@ -277,7 +274,7 @@ impl Duration { /// #![feature(duration_zero)] /// use std::time::Duration; /// - /// assert!(Duration::zero().is_zero()); + /// assert!(Duration::ZERO.is_zero()); /// assert!(Duration::new(0, 0).is_zero()); /// assert!(Duration::from_nanos(0).is_zero()); /// assert!(Duration::from_secs(0).is_zero()); diff --git a/library/core/tests/time.rs b/library/core/tests/time.rs index 6508e78429c0e..dffbdd0372c15 100644 --- a/library/core/tests/time.rs +++ b/library/core/tests/time.rs @@ -108,26 +108,24 @@ fn sub() { #[test] fn checked_sub() { - let zero = Duration::zero(); - assert_eq!(Duration::NANOSECOND.checked_sub(zero), Some(Duration::NANOSECOND)); + assert_eq!(Duration::NANOSECOND.checked_sub(Duration::ZERO), Some(Duration::NANOSECOND)); assert_eq!( Duration::SECOND.checked_sub(Duration::NANOSECOND), Some(Duration::new(0, 999_999_999)) ); - assert_eq!(zero.checked_sub(Duration::NANOSECOND), None); - assert_eq!(zero.checked_sub(Duration::SECOND), None); + assert_eq!(Duration::ZERO.checked_sub(Duration::NANOSECOND), None); + assert_eq!(Duration::ZERO.checked_sub(Duration::SECOND), None); } #[test] fn saturating_sub() { - let zero = Duration::new(0, 0); - assert_eq!(Duration::NANOSECOND.saturating_sub(zero), Duration::NANOSECOND); + assert_eq!(Duration::NANOSECOND.saturating_sub(Duration::ZERO), Duration::NANOSECOND); assert_eq!( Duration::SECOND.saturating_sub(Duration::NANOSECOND), Duration::new(0, 999_999_999) ); - assert_eq!(zero.saturating_sub(Duration::NANOSECOND), Duration::MIN); - assert_eq!(zero.saturating_sub(Duration::SECOND), Duration::MIN); + assert_eq!(Duration::ZERO.saturating_sub(Duration::NANOSECOND), Duration::MIN); + assert_eq!(Duration::ZERO.saturating_sub(Duration::SECOND), Duration::MIN); } #[test] @@ -339,10 +337,7 @@ fn duration_const() { const SUB_SEC_NANOS: u32 = DURATION.subsec_nanos(); assert_eq!(SUB_SEC_NANOS, 123_456_789); - const ZERO: Duration = Duration::zero(); - assert_eq!(ZERO, Duration::new(0, 0)); - - const IS_ZERO: bool = ZERO.is_zero(); + const IS_ZERO: bool = Duration::ZERO.is_zero(); assert!(IS_ZERO); const SECONDS: u64 = Duration::SECOND.as_secs(); @@ -386,7 +381,7 @@ fn duration_const() { const CHECKED_ADD: Option = MAX.checked_add(Duration::SECOND); assert_eq!(CHECKED_ADD, None); - const CHECKED_SUB: Option = ZERO.checked_sub(Duration::SECOND); + const CHECKED_SUB: Option = (Duration::ZERO).checked_sub(Duration::SECOND); assert_eq!(CHECKED_SUB, None); const CHECKED_MUL: Option = Duration::SECOND.checked_mul(1); @@ -416,8 +411,8 @@ fn duration_const() { const SATURATING_ADD: Duration = MAX.saturating_add(Duration::SECOND); assert_eq!(SATURATING_ADD, MAX); - const SATURATING_SUB: Duration = ZERO.saturating_sub(Duration::SECOND); - assert_eq!(SATURATING_SUB, ZERO); + const SATURATING_SUB: Duration = (Duration::ZERO).saturating_sub(Duration::SECOND); + assert_eq!(SATURATING_SUB, Duration::ZERO); const SATURATING_MUL: Duration = MAX.saturating_mul(2); assert_eq!(SATURATING_MUL, MAX); diff --git a/library/std/src/time/tests.rs b/library/std/src/time/tests.rs index 44c06bac9507c..20c813fdc70ff 100644 --- a/library/std/src/time/tests.rs +++ b/library/std/src/time/tests.rs @@ -75,14 +75,14 @@ fn instant_checked_duration_since_nopanic() { let later = now + Duration::SECOND; assert_eq!(earlier.checked_duration_since(now), None); assert_eq!(later.checked_duration_since(now), Some(Duration::SECOND)); - assert_eq!(now.checked_duration_since(now), Some(Duration::zero())); + assert_eq!(now.checked_duration_since(now), Some(Duration::ZERO)); } #[test] fn instant_saturating_duration_since_nopanic() { let a = Instant::now(); let ret = (a - Duration::SECOND).saturating_duration_since(a); - assert_eq!(ret, Duration::zero()); + assert_eq!(ret, Duration::ZERO); } #[test] @@ -90,7 +90,7 @@ fn system_time_math() { let a = SystemTime::now(); let b = SystemTime::now(); match b.duration_since(a) { - Ok(dur) if dur == Duration::zero() => { + Ok(Duration::ZERO) => { assert_almost_eq!(a, b); } Ok(dur) => { From b989d46b48471c70a6d5fb1c1aff4feadaeef922 Mon Sep 17 00:00:00 2001 From: 12101111 Date: Sun, 25 Oct 2020 20:13:14 +0800 Subject: [PATCH 03/42] Support enable/disable sanitizers/profiler per target --- config.toml.example | 9 +++++++++ src/bootstrap/builder.rs | 4 ++-- src/bootstrap/check.rs | 4 ++-- src/bootstrap/compile.rs | 4 ++-- src/bootstrap/config.rs | 22 ++++++++++++++++++++++ src/bootstrap/doc.rs | 8 ++++++-- src/bootstrap/lib.rs | 11 +++++++---- src/bootstrap/sanity.rs | 2 +- src/bootstrap/test.rs | 8 ++++---- 9 files changed, 55 insertions(+), 17 deletions(-) diff --git a/config.toml.example b/config.toml.example index e7e37c679e545..e47002c049066 100644 --- a/config.toml.example +++ b/config.toml.example @@ -582,6 +582,15 @@ changelog-seen = 2 # build native code. #android-ndk = "/path/to/ndk" +# Build the sanitizer runtimes for this target. +# This option will override the same option under [build] section. +#sanitizers = false + +# Build the profiler runtime for this target(required when compiling with options that depend +# on this runtime, such as `-C profile-generate` or `-Z instrument-coverage`). +# This option will override the same option under [build] section. +#profiler = false + # Force static or dynamic linkage of the standard library for this target. If # this target is a host for rustc, this will also affect the linkage of the # compiler itself. This is useful for building rustc on targets that normally diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 707c1ff3efad9..bc2d503c8c4ce 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -264,7 +264,7 @@ impl<'a> ShouldRun<'a> { /// `all_krates` should probably be removed at some point. pub fn all_krates(mut self, name: &str) -> Self { let mut set = BTreeSet::new(); - for krate in self.builder.in_tree_crates(name) { + for krate in self.builder.in_tree_crates(name, None) { let path = krate.local_path(self.builder); set.insert(path); } @@ -277,7 +277,7 @@ impl<'a> ShouldRun<'a> { /// /// `make_run` will be called separately for each matching command-line path. pub fn krate(mut self, name: &str) -> Self { - for krate in self.builder.in_tree_crates(name) { + for krate in self.builder.in_tree_crates(name, None) { let path = krate.local_path(self.builder); self.paths.insert(PathSet::one(path)); } diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index 371631154f72d..f44483095500b 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -91,7 +91,7 @@ impl Step for Std { // Explicitly pass -p for all dependencies krates -- this will force cargo // to also check the tests/benches/examples for these crates, rather // than just the leaf crate. - for krate in builder.in_tree_crates("test") { + for krate in builder.in_tree_crates("test", Some(target)) { cargo.arg("-p").arg(krate.name); } @@ -155,7 +155,7 @@ impl Step for Rustc { // Explicitly pass -p for all compiler krates -- this will force cargo // to also check the tests/benches/examples for these crates, rather // than just the leaf crate. - for krate in builder.in_tree_crates("rustc-main") { + for krate in builder.in_tree_crates("rustc-main", Some(target)) { cargo.arg("-p").arg(krate.name); } diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 5215ab3dd4f9b..b6654e8868b50 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -143,7 +143,7 @@ fn copy_third_party_objects( } } - if builder.config.sanitizers && compiler.stage != 0 { + if builder.config.sanitizers_enabled(target) && compiler.stage != 0 { // The sanitizers are only copied in stage1 or above, // to avoid creating dependency on LLVM. target_deps.extend( @@ -251,7 +251,7 @@ pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, stage: u32, car .arg("--features") .arg(features); } else { - let mut features = builder.std_features(); + let mut features = builder.std_features(target); features.push_str(compiler_builtins_c_feature); cargo diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index 3c1249f8de434..034b6c9bf1960 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -251,6 +251,8 @@ pub struct Target { pub ranlib: Option, pub linker: Option, pub ndk: Option, + pub sanitizers: bool, + pub profiler: bool, pub crt_static: Option, pub musl_root: Option, pub musl_libdir: Option, @@ -474,6 +476,8 @@ struct TomlTarget { llvm_config: Option, llvm_filecheck: Option, android_ndk: Option, + sanitizers: Option, + profiler: Option, crt_static: Option, musl_root: Option, musl_libdir: Option, @@ -857,6 +861,8 @@ impl Config { target.musl_libdir = cfg.musl_libdir.map(PathBuf::from); target.wasi_root = cfg.wasi_root.map(PathBuf::from); target.qemu_rootfs = cfg.qemu_rootfs.map(PathBuf::from); + target.sanitizers = cfg.sanitizers.unwrap_or(build.sanitizers.unwrap_or_default()); + target.profiler = cfg.profiler.unwrap_or(build.profiler.unwrap_or_default()); config.target_config.insert(TargetSelection::from_user(&triple), target); } @@ -959,6 +965,22 @@ impl Config { self.verbose > 1 } + pub fn sanitizers_enabled(&self, target: TargetSelection) -> bool { + self.target_config.get(&target).map(|t| t.sanitizers).unwrap_or(self.sanitizers) + } + + pub fn any_sanitizers_enabled(&self) -> bool { + self.target_config.values().any(|t| t.sanitizers) || self.sanitizers + } + + pub fn profiler_enabled(&self, target: TargetSelection) -> bool { + self.target_config.get(&target).map(|t| t.profiler).unwrap_or(self.profiler) + } + + pub fn any_profiler_enabled(&self) -> bool { + self.target_config.values().any(|t| t.profiler) || self.profiler + } + pub fn llvm_enabled(&self) -> bool { self.rust_codegen_backends.contains(&INTERNER.intern_str("llvm")) } diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index aa670bd9a2e0c..af7f7eff89418 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -535,8 +535,12 @@ impl Step for Rustc { // Find dependencies for top level crates. let mut compiler_crates = HashSet::new(); for root_crate in &["rustc_driver", "rustc_codegen_llvm", "rustc_codegen_ssa"] { - compiler_crates - .extend(builder.in_tree_crates(root_crate).into_iter().map(|krate| krate.name)); + compiler_crates.extend( + builder + .in_tree_crates(root_crate, Some(target)) + .into_iter() + .map(|krate| krate.name), + ); } for krate in &compiler_crates { diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 22a8e828862f7..5593f72dfe387 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -534,7 +534,7 @@ impl Build { /// Gets the space-separated set of activated features for the standard /// library. - fn std_features(&self) -> String { + fn std_features(&self, target: TargetSelection) -> String { let mut features = "panic-unwind".to_string(); if self.config.llvm_libunwind { @@ -543,7 +543,7 @@ impl Build { if self.config.backtrace { features.push_str(" backtrace"); } - if self.config.profiler { + if self.config.profiler_enabled(target) { features.push_str(" profiler"); } features @@ -1105,7 +1105,7 @@ impl Build { /// Returns a Vec of all the dependencies of the given root crate, /// including transitive dependencies and the root itself. Only includes /// "local" crates (those in the local source tree, not from a registry). - fn in_tree_crates(&self, root: &str) -> Vec<&Crate> { + fn in_tree_crates(&self, root: &str, target: Option) -> Vec<&Crate> { let mut ret = Vec::new(); let mut list = vec![INTERNER.intern_str(root)]; let mut visited = HashSet::new(); @@ -1122,7 +1122,10 @@ impl Build { // metadata::build. if visited.insert(dep) && dep != "build_helper" - && (dep != "profiler_builtins" || self.config.profiler) + && (dep != "profiler_builtins" + || target + .map(|t| self.config.profiler_enabled(t)) + .unwrap_or(self.config.any_profiler_enabled())) && (dep != "rustc_codegen_llvm" || self.config.llvm_enabled()) { list.push(*dep); diff --git a/src/bootstrap/sanity.rs b/src/bootstrap/sanity.rs index 6826d177a4ade..4cfcf6ca407b5 100644 --- a/src/bootstrap/sanity.rs +++ b/src/bootstrap/sanity.rs @@ -91,7 +91,7 @@ pub fn check(build: &mut Build) { .unwrap_or(true) }) .any(|build_llvm_ourselves| build_llvm_ourselves); - if building_llvm || build.config.sanitizers { + if building_llvm || build.config.any_sanitizers_enabled() { cmd_finder.must_have("cmake"); } diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 6c2c05ac7197e..81b55b2564cef 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -1265,11 +1265,11 @@ note: if you're sure you want to do this, please open an issue as to why. In the cmd.env("RUSTC_BOOTSTRAP", "1"); builder.add_rust_test_threads(&mut cmd); - if builder.config.sanitizers { + if builder.config.sanitizers_enabled(target) { cmd.env("RUSTC_SANITIZER_SUPPORT", "1"); } - if builder.config.profiler { + if builder.config.profiler_enabled(target) { cmd.env("RUSTC_PROFILER_SUPPORT", "1"); } @@ -1585,7 +1585,7 @@ impl Step for CrateLibrustc { let builder = run.builder; let compiler = builder.compiler(builder.top_stage, run.build_triple()); - for krate in builder.in_tree_crates("rustc-main") { + for krate in builder.in_tree_crates("rustc-main", Some(run.target)) { if krate.path.ends_with(&run.path) { let test_kind = builder.kind.into(); @@ -1692,7 +1692,7 @@ impl Step for Crate { }); }; - for krate in builder.in_tree_crates("test") { + for krate in builder.in_tree_crates("test", Some(run.target)) { if krate.path.ends_with(&run.path) { make(Mode::Std, krate); } From af4d1786e70e441e0998a80d5db0f2f197ea2cf1 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 27 Oct 2020 13:43:21 -0700 Subject: [PATCH 04/42] Fixup tests: Duration::MIN -> ::ZERO --- library/core/tests/time.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/core/tests/time.rs b/library/core/tests/time.rs index dffbdd0372c15..f14639e0d589f 100644 --- a/library/core/tests/time.rs +++ b/library/core/tests/time.rs @@ -124,8 +124,8 @@ fn saturating_sub() { Duration::SECOND.saturating_sub(Duration::NANOSECOND), Duration::new(0, 999_999_999) ); - assert_eq!(Duration::ZERO.saturating_sub(Duration::NANOSECOND), Duration::MIN); - assert_eq!(Duration::ZERO.saturating_sub(Duration::SECOND), Duration::MIN); + assert_eq!(Duration::ZERO.saturating_sub(Duration::NANOSECOND), Duration::ZERO); + assert_eq!(Duration::ZERO.saturating_sub(Duration::SECOND), Duration::ZERO); } #[test] @@ -381,7 +381,7 @@ fn duration_const() { const CHECKED_ADD: Option = MAX.checked_add(Duration::SECOND); assert_eq!(CHECKED_ADD, None); - const CHECKED_SUB: Option = (Duration::ZERO).checked_sub(Duration::SECOND); + const CHECKED_SUB: Option = Duration::ZERO.checked_sub(Duration::SECOND); assert_eq!(CHECKED_SUB, None); const CHECKED_MUL: Option = Duration::SECOND.checked_mul(1); @@ -411,7 +411,7 @@ fn duration_const() { const SATURATING_ADD: Duration = MAX.saturating_add(Duration::SECOND); assert_eq!(SATURATING_ADD, MAX); - const SATURATING_SUB: Duration = (Duration::ZERO).saturating_sub(Duration::SECOND); + const SATURATING_SUB: Duration = Duration::ZERO.saturating_sub(Duration::SECOND); assert_eq!(SATURATING_SUB, Duration::ZERO); const SATURATING_MUL: Duration = MAX.saturating_mul(2); From 82f3a236cdb80f65cd5b89a1cb015f95184bf66a Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 27 Oct 2020 14:06:55 -0700 Subject: [PATCH 05/42] Remove Duration::MIN entirely Duration::ZERO supercedes it in effect. --- library/core/src/time.rs | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/library/core/src/time.rs b/library/core/src/time.rs index cc9afaff888ae..88b4e2a2436e7 100644 --- a/library/core/src/time.rs +++ b/library/core/src/time.rs @@ -123,19 +123,6 @@ impl Duration { #[unstable(feature = "duration_zero", issue = "73544")] pub const ZERO: Duration = Duration::from_nanos(0); - /// The minimum duration. - /// - /// # Examples - /// - /// ``` - /// #![feature(duration_constants)] - /// use std::time::Duration; - /// - /// assert_eq!(Duration::MIN, Duration::new(0, 0)); - /// ``` - #[unstable(feature = "duration_constants", issue = "57391")] - pub const MIN: Duration = Duration::from_nanos(0); - /// The maximum duration. /// /// It is roughly equal to a duration of 584,942,417,355 years. @@ -533,18 +520,18 @@ impl Duration { } } - /// Saturating `Duration` subtraction. Computes `self - other`, returning [`Duration::MIN`] + /// Saturating `Duration` subtraction. Computes `self - other`, returning [`Duration::ZERO`] /// if the result would be negative or if overflow occurred. /// /// # Examples /// /// ``` /// #![feature(duration_saturating_ops)] - /// #![feature(duration_constants)] + /// #![feature(duration_zero)] /// use std::time::Duration; /// /// assert_eq!(Duration::new(0, 1).saturating_sub(Duration::new(0, 0)), Duration::new(0, 1)); - /// assert_eq!(Duration::new(0, 0).saturating_sub(Duration::new(0, 1)), Duration::MIN); + /// assert_eq!(Duration::new(0, 0).saturating_sub(Duration::new(0, 1)), Duration::ZERO); /// ``` #[unstable(feature = "duration_saturating_ops", issue = "76416")] #[inline] @@ -552,7 +539,7 @@ impl Duration { pub const fn saturating_sub(self, rhs: Duration) -> Duration { match self.checked_sub(rhs) { Some(res) => res, - None => Duration::MIN, + None => Duration::ZERO, } } From 458a1435f581db1eb5ca63a1051bccf927cd920f Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 18 Oct 2020 22:24:15 +0200 Subject: [PATCH 06/42] Add lint to warn about braces in a panic message. --- compiler/rustc_lint/src/lib.rs | 3 ++ compiler/rustc_lint/src/panic_fmt.rs | 57 ++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 compiler/rustc_lint/src/panic_fmt.rs diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index c8990842d32e6..46d74b6a6d540 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -52,6 +52,7 @@ mod levels; mod methods; mod non_ascii_idents; mod nonstandard_style; +mod panic_fmt; mod passes; mod redundant_semicolon; mod traits; @@ -77,6 +78,7 @@ use internal::*; use methods::*; use non_ascii_idents::*; use nonstandard_style::*; +use panic_fmt::PanicFmt; use redundant_semicolon::*; use traits::*; use types::*; @@ -163,6 +165,7 @@ macro_rules! late_lint_passes { ClashingExternDeclarations: ClashingExternDeclarations::new(), DropTraitConstraints: DropTraitConstraints, TemporaryCStringAsPtr: TemporaryCStringAsPtr, + PanicFmt: PanicFmt, ] ); }; diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs new file mode 100644 index 0000000000000..0d3649ec543b9 --- /dev/null +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -0,0 +1,57 @@ +use crate::{LateContext, LateLintPass, LintContext}; +use rustc_ast as ast; +use rustc_hir as hir; +use rustc_middle::ty; + +declare_lint! { + /// The `panic_fmt` lint detects `panic!("..")` with `{` or `}` in the string literal. + /// + /// ### Example + /// + /// ```rust,no_run + /// panic!("{}"); + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// `panic!("{}")` panics with the message `"{}"`, as a `panic!()` invocation + /// with a single argument does not use `format_args!()`. + /// A future version of Rust will interpret this string as format string, + /// which would break this. + PANIC_FMT, + Warn, + "detect braces in single-argument panic!() invocations", + report_in_external_macro +} + +declare_lint_pass!(PanicFmt => [PANIC_FMT]); + +impl<'tcx> LateLintPass<'tcx> for PanicFmt { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) { + if let hir::ExprKind::Call(f, [arg]) = &expr.kind { + if let &ty::FnDef(def_id, _) = cx.typeck_results().expr_ty(f).kind() { + if Some(def_id) == cx.tcx.lang_items().begin_panic_fn() + || Some(def_id) == cx.tcx.lang_items().panic_fn() + { + check_panic(cx, f, arg); + } + } + } + } +} + +fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'tcx>) { + if let hir::ExprKind::Lit(lit) = &arg.kind { + if let ast::LitKind::Str(sym, _) = lit.node { + if sym.as_str().contains(&['{', '}'][..]) { + cx.struct_span_lint(PANIC_FMT, f.span, |lint| { + lint.build("Panic message contains a brace") + .note("This message is not used as a format string, but will be in a future Rust version") + .emit(); + }); + } + } + } +} From d75046fcb659056f38aecb5952ec2138247c73d2 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 18 Oct 2020 22:19:13 +0200 Subject: [PATCH 07/42] Allow #[rustc_diagnostic_item] on macros. --- compiler/rustc_passes/src/diagnostic_items.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/rustc_passes/src/diagnostic_items.rs b/compiler/rustc_passes/src/diagnostic_items.rs index 94592935c7f91..ee9beb94191b6 100644 --- a/compiler/rustc_passes/src/diagnostic_items.rs +++ b/compiler/rustc_passes/src/diagnostic_items.rs @@ -113,6 +113,10 @@ fn collect<'tcx>(tcx: TyCtxt<'tcx>) -> FxHashMap { } } + for m in tcx.hir().krate().exported_macros { + collector.observe_item(m.attrs, m.hir_id); + } + collector.items } From 561b8722e0198efe6468725a1ab50255fd6f63ad Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 18 Oct 2020 22:20:19 +0200 Subject: [PATCH 08/42] Mark the panic macros as diagnostic items. --- library/core/src/macros/mod.rs | 1 + library/std/src/macros.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index bce8a70e92f31..9c7c6ff33c589 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -2,6 +2,7 @@ #[macro_export] #[allow_internal_unstable(core_panic, const_caller_location)] #[stable(feature = "core", since = "1.6.0")] +#[rustc_diagnostic_item = "core_panic_macro"] macro_rules! panic { () => ( $crate::panic!("explicit panic") diff --git a/library/std/src/macros.rs b/library/std/src/macros.rs index 57649d6f8f252..d003dd9fe84b8 100644 --- a/library/std/src/macros.rs +++ b/library/std/src/macros.rs @@ -8,6 +8,7 @@ #[macro_export] #[stable(feature = "rust1", since = "1.0.0")] #[allow_internal_unstable(libstd_sys_internals)] +#[rustc_diagnostic_item = "std_panic_macro"] macro_rules! panic { () => ({ $crate::panic!("explicit panic") }); ($msg:expr $(,)?) => ({ $crate::rt::begin_panic($msg) }); From ee99096e197451add85a368a0d3ed4cfb4aa500d Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 18 Oct 2020 22:26:36 +0200 Subject: [PATCH 09/42] Specialize panic_fmt lint for the {core,std}::panic!() macros. It now only reacts to expansion of those macros, and suggests inserting `"{}", ` in the right place. --- compiler/rustc_lint/src/panic_fmt.rs | 27 ++++++++++++++++++++++----- compiler/rustc_span/src/symbol.rs | 2 ++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs index 0d3649ec543b9..e0752515377fe 100644 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -1,7 +1,9 @@ use crate::{LateContext, LateLintPass, LintContext}; use rustc_ast as ast; +use rustc_errors::Applicability; use rustc_hir as hir; use rustc_middle::ty; +use rustc_span::sym; declare_lint! { /// The `panic_fmt` lint detects `panic!("..")` with `{` or `}` in the string literal. @@ -46,11 +48,26 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc if let hir::ExprKind::Lit(lit) = &arg.kind { if let ast::LitKind::Str(sym, _) = lit.node { if sym.as_str().contains(&['{', '}'][..]) { - cx.struct_span_lint(PANIC_FMT, f.span, |lint| { - lint.build("Panic message contains a brace") - .note("This message is not used as a format string, but will be in a future Rust version") - .emit(); - }); + let expn = f.span.ctxt().outer_expn_data(); + if let Some(id) = expn.macro_def_id { + if cx.tcx.is_diagnostic_item(sym::std_panic_macro, id) + || cx.tcx.is_diagnostic_item(sym::core_panic_macro, id) + { + cx.struct_span_lint(PANIC_FMT, expn.call_site, |lint| { + let mut l = lint.build("Panic message contains a brace"); + l.note("This message is not used as a format string, but will be in a future Rust version"); + if expn.call_site.contains(arg.span) { + l.span_suggestion( + arg.span.shrink_to_lo(), + "add a \"{}\" format string to use the message literally", + "\"{}\", ".into(), + Applicability::MachineApplicable, + ); + } + l.emit(); + }); + } + } } } } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 1a6c45b6c80d2..601adc1f43328 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -391,6 +391,7 @@ symbols! { copysignf64, core, core_intrinsics, + core_panic_macro, cosf32, cosf64, crate_id, @@ -1060,6 +1061,7 @@ symbols! { staticlib, std, std_inject, + std_panic_macro, stmt, stmt_expr_attributes, stop_after_dataflow, From 00dd6632cce40f49042f10208046233515601a3b Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 18 Oct 2020 22:29:40 +0200 Subject: [PATCH 10/42] Make panic_fmt lint work properly for assert!(expr, msg) too. --- compiler/rustc_lint/src/panic_fmt.rs | 12 ++++++++++++ compiler/rustc_span/src/symbol.rs | 1 + library/core/src/macros/mod.rs | 1 + 3 files changed, 14 insertions(+) diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs index e0752515377fe..198797974ff0c 100644 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -53,6 +53,18 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc if cx.tcx.is_diagnostic_item(sym::std_panic_macro, id) || cx.tcx.is_diagnostic_item(sym::core_panic_macro, id) { + let expn = { + // Unwrap another level of macro expansion if this + // panic!() was expanded from assert!(). + let parent = expn.call_site.ctxt().outer_expn_data(); + if parent.macro_def_id.map_or(false, |id| { + cx.tcx.is_diagnostic_item(sym::assert_macro, id) + }) { + parent + } else { + expn + } + }; cx.struct_span_lint(PANIC_FMT, expn.call_site, |lint| { let mut l = lint.build("Panic message contains a brace"); l.note("This message is not used as a format string, but will be in a future Rust version"); diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 601adc1f43328..3e696a5c33362 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -267,6 +267,7 @@ symbols! { asm, assert, assert_inhabited, + assert_macro, assert_receiver_is_total_eq, assert_uninit_valid, assert_zero_valid, diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index 9c7c6ff33c589..23baf7d82befd 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -1216,6 +1216,7 @@ pub(crate) mod builtin { #[stable(feature = "rust1", since = "1.0.0")] #[rustc_builtin_macro] #[macro_export] + #[rustc_diagnostic_item = "assert_macro"] macro_rules! assert { ($cond:expr $(,)?) => {{ /* compiler built-in */ }}; ($cond:expr, $($arg:tt)+) => {{ /* compiler built-in */ }}; From 5c5232afe716f49183f22ccb8afac25292932f5f Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 18 Oct 2020 22:30:16 +0200 Subject: [PATCH 11/42] Expand assert!(expr) to panic() function instead of panic!() macro. The panic message might contain braces which should never be interpreted as format placeholders, which panic!() will do in a future edition. --- compiler/rustc_builtin_macros/src/assert.rs | 62 ++++++++++++--------- compiler/rustc_span/src/symbol.rs | 1 + library/core/src/macros/mod.rs | 1 + 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/assert.rs b/compiler/rustc_builtin_macros/src/assert.rs index 5bfd8a2bf561c..bc3276538da8f 100644 --- a/compiler/rustc_builtin_macros/src/assert.rs +++ b/compiler/rustc_builtin_macros/src/assert.rs @@ -1,8 +1,8 @@ use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_ast::ptr::P; -use rustc_ast::token::{self, TokenKind}; -use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree}; +use rustc_ast::token; +use rustc_ast::tokenstream::{DelimSpan, TokenStream}; use rustc_ast::{self as ast, *}; use rustc_ast_pretty::pprust; use rustc_expand::base::*; @@ -26,31 +26,41 @@ pub fn expand_assert<'cx>( // `core::panic` and `std::panic` are different macros, so we use call-site // context to pick up whichever is currently in scope. let sp = cx.with_call_site_ctxt(sp); - let tokens = custom_message.unwrap_or_else(|| { - TokenStream::from(TokenTree::token( - TokenKind::lit( - token::Str, - Symbol::intern(&format!( - "assertion failed: {}", - pprust::expr_to_string(&cond_expr).escape_debug() - )), - None, - ), - DUMMY_SP, - )) - }); - let args = P(MacArgs::Delimited(DelimSpan::from_single(sp), MacDelimiter::Parenthesis, tokens)); - let panic_call = MacCall { - path: Path::from_ident(Ident::new(sym::panic, sp)), - args, - prior_type_ascription: None, + + let panic_call = { + if let Some(tokens) = custom_message { + // Pass the custom message to panic!(). + cx.expr( + sp, + ExprKind::MacCall(MacCall { + path: Path::from_ident(Ident::new(sym::panic, sp)), + args: P(MacArgs::Delimited( + DelimSpan::from_single(sp), + MacDelimiter::Parenthesis, + tokens, + )), + prior_type_ascription: None, + }), + ) + } else { + // Pass our own message directly to $crate::panicking::panic(), + // because it might contain `{` and `}` that should always be + // passed literally. + cx.expr_call_global( + sp, + cx.std_path(&[sym::panicking, sym::panic]), + vec![cx.expr_str( + DUMMY_SP, + Symbol::intern(&format!( + "assertion failed: {}", + pprust::expr_to_string(&cond_expr).escape_debug() + )), + )], + ) + } }; - let if_expr = cx.expr_if( - sp, - cx.expr(sp, ExprKind::Unary(UnOp::Not, cond_expr)), - cx.expr(sp, ExprKind::MacCall(panic_call)), - None, - ); + let if_expr = + cx.expr_if(sp, cx.expr(sp, ExprKind::Unary(UnOp::Not, cond_expr)), panic_call, None); MacEager::expr(if_expr) } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 3e696a5c33362..d128491da2126 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -788,6 +788,7 @@ symbols! { panic_runtime, panic_str, panic_unwind, + panicking, param_attrs, parent_trait, partial_cmp, diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index 23baf7d82befd..b84e86e9073ec 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -1217,6 +1217,7 @@ pub(crate) mod builtin { #[rustc_builtin_macro] #[macro_export] #[rustc_diagnostic_item = "assert_macro"] + #[allow_internal_unstable(core_panic)] macro_rules! assert { ($cond:expr $(,)?) => {{ /* compiler built-in */ }}; ($cond:expr, $($arg:tt)+) => {{ /* compiler built-in */ }}; From 3e8350e1e292edbc84bbb135fb98b2a21c480146 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 18 Oct 2020 22:52:36 +0200 Subject: [PATCH 12/42] Add test for the panic_fmt lint. --- src/test/ui/panic-brace.rs | 9 ++++++ src/test/ui/panic-brace.stderr | 51 ++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 src/test/ui/panic-brace.rs create mode 100644 src/test/ui/panic-brace.stderr diff --git a/src/test/ui/panic-brace.rs b/src/test/ui/panic-brace.rs new file mode 100644 index 0000000000000..744ecb30037cd --- /dev/null +++ b/src/test/ui/panic-brace.rs @@ -0,0 +1,9 @@ +// build-pass (FIXME(62277): should be check-pass) + +#[allow(unreachable_code)] +fn main() { + panic!("here's a brace: {"); //~ WARN Panic message contains a brace + std::panic!("another one: }"); //~ WARN Panic message contains a brace + core::panic!("Hello { { {"); //~ WARN Panic message contains a brace + assert!(false, "} } }..."); //~ WARN Panic message contains a brace +} diff --git a/src/test/ui/panic-brace.stderr b/src/test/ui/panic-brace.stderr new file mode 100644 index 0000000000000..c54044cb6c81f --- /dev/null +++ b/src/test/ui/panic-brace.stderr @@ -0,0 +1,51 @@ +warning: Panic message contains a brace + --> $DIR/panic-brace.rs:5:5 + | +LL | panic!("here's a brace: {"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(panic_fmt)]` on by default + = note: This message is not used as a format string, but will be in a future Rust version +help: add a "{}" format string to use the message literally + | +LL | panic!("{}", "here's a brace: {"); + | ^^^^^ + +warning: Panic message contains a brace + --> $DIR/panic-brace.rs:6:5 + | +LL | std::panic!("another one: }"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: This message is not used as a format string, but will be in a future Rust version +help: add a "{}" format string to use the message literally + | +LL | std::panic!("{}", "another one: }"); + | ^^^^^ + +warning: Panic message contains a brace + --> $DIR/panic-brace.rs:7:5 + | +LL | core::panic!("Hello { { {"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: This message is not used as a format string, but will be in a future Rust version +help: add a "{}" format string to use the message literally + | +LL | core::panic!("{}", "Hello { { {"); + | ^^^^^ + +warning: Panic message contains a brace + --> $DIR/panic-brace.rs:8:5 + | +LL | assert!(false, "} } }..."); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: This message is not used as a format string, but will be in a future Rust version +help: add a "{}" format string to use the message literally + | +LL | assert!(false, "{}", "} } }..."); + | ^^^^^ + +warning: 4 warnings emitted + From 098aaf9debaae25f6fb7d8b187401888f0f3eae7 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 18 Oct 2020 23:25:06 +0200 Subject: [PATCH 13/42] Improve panic_fmt message for panic!("{}") with a fmt placeholder. --- compiler/rustc_lint/src/panic_fmt.rs | 62 ++++++++++++++++++++-------- 1 file changed, 45 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs index 198797974ff0c..6879244721bb2 100644 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -47,24 +47,52 @@ impl<'tcx> LateLintPass<'tcx> for PanicFmt { fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'tcx>) { if let hir::ExprKind::Lit(lit) = &arg.kind { if let ast::LitKind::Str(sym, _) = lit.node { - if sym.as_str().contains(&['{', '}'][..]) { - let expn = f.span.ctxt().outer_expn_data(); - if let Some(id) = expn.macro_def_id { - if cx.tcx.is_diagnostic_item(sym::std_panic_macro, id) - || cx.tcx.is_diagnostic_item(sym::core_panic_macro, id) - { - let expn = { - // Unwrap another level of macro expansion if this - // panic!() was expanded from assert!(). - let parent = expn.call_site.ctxt().outer_expn_data(); - if parent.macro_def_id.map_or(false, |id| { - cx.tcx.is_diagnostic_item(sym::assert_macro, id) - }) { - parent - } else { - expn + let s = sym.as_str(); + let open = s.find('{'); + let close = s[open.unwrap_or(0)..].find('}'); + let looks_like_placeholder = match (open, close) { + (Some(_), Some(_)) => true, + (Some(_), None) | (None, Some(_)) => false, + (None, None) => return // OK, no braces. + }; + let expn = f.span.ctxt().outer_expn_data(); + if let Some(id) = expn.macro_def_id { + if cx.tcx.is_diagnostic_item(sym::std_panic_macro, id) + || cx.tcx.is_diagnostic_item(sym::core_panic_macro, id) + { + let expn = { + // Unwrap another level of macro expansion if this + // panic!() was expanded from assert!(). + let parent = expn.call_site.ctxt().outer_expn_data(); + if parent.macro_def_id.map_or(false, |id| { + cx.tcx.is_diagnostic_item(sym::assert_macro, id) + }) { + parent + } else { + expn + } + }; + if looks_like_placeholder { + cx.struct_span_lint(PANIC_FMT, arg.span.source_callsite(), |lint| { + let mut l = lint.build("Panic message contains an unused formatting placeholder"); + l.note("This message is not used as a format string when given without arguments, but will be in a future Rust version"); + if expn.call_site.contains(arg.span) { + l.span_suggestion( + arg.span.shrink_to_hi(), + "add the missing argument(s)", + ", argument".into(), + Applicability::HasPlaceholders, + ); + l.span_suggestion( + arg.span.shrink_to_lo(), + "or add a \"{}\" format string to use the message literally", + "\"{}\", ".into(), + Applicability::MaybeIncorrect, + ); } - }; + l.emit(); + }); + } else { cx.struct_span_lint(PANIC_FMT, expn.call_site, |lint| { let mut l = lint.build("Panic message contains a brace"); l.note("This message is not used as a format string, but will be in a future Rust version"); From d17364e73202fe4bbb5f35c045fa9ff2c54f8bf3 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 18 Oct 2020 23:25:57 +0200 Subject: [PATCH 14/42] Formatting. --- compiler/rustc_lint/src/panic_fmt.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs index 6879244721bb2..a615d57dfa705 100644 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -53,7 +53,7 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc let looks_like_placeholder = match (open, close) { (Some(_), Some(_)) => true, (Some(_), None) | (None, Some(_)) => false, - (None, None) => return // OK, no braces. + (None, None) => return, // OK, no braces. }; let expn = f.span.ctxt().outer_expn_data(); if let Some(id) = expn.macro_def_id { @@ -64,9 +64,10 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc // Unwrap another level of macro expansion if this // panic!() was expanded from assert!(). let parent = expn.call_site.ctxt().outer_expn_data(); - if parent.macro_def_id.map_or(false, |id| { - cx.tcx.is_diagnostic_item(sym::assert_macro, id) - }) { + if parent + .macro_def_id + .map_or(false, |id| cx.tcx.is_diagnostic_item(sym::assert_macro, id)) + { parent } else { expn From b754e2f88ebcbc4c68d4777d36c53aaecd5e2869 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 18 Oct 2020 23:36:00 +0200 Subject: [PATCH 15/42] Test for formating placeholders in panic_fmt lint test. --- src/test/ui/panic-brace.rs | 4 ++-- src/test/ui/panic-brace.stderr | 36 +++++++++++++++++++++------------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/test/ui/panic-brace.rs b/src/test/ui/panic-brace.rs index 744ecb30037cd..01ffb80076803 100644 --- a/src/test/ui/panic-brace.rs +++ b/src/test/ui/panic-brace.rs @@ -4,6 +4,6 @@ fn main() { panic!("here's a brace: {"); //~ WARN Panic message contains a brace std::panic!("another one: }"); //~ WARN Panic message contains a brace - core::panic!("Hello { { {"); //~ WARN Panic message contains a brace - assert!(false, "} } }..."); //~ WARN Panic message contains a brace + core::panic!("Hello {}"); //~ WARN Panic message contains an unused formatting placeholder + assert!(false, "{:03x} bla"); //~ WARN Panic message contains an unused formatting placeholder } diff --git a/src/test/ui/panic-brace.stderr b/src/test/ui/panic-brace.stderr index c54044cb6c81f..b91ce8f603a55 100644 --- a/src/test/ui/panic-brace.stderr +++ b/src/test/ui/panic-brace.stderr @@ -23,28 +23,36 @@ help: add a "{}" format string to use the message literally LL | std::panic!("{}", "another one: }"); | ^^^^^ -warning: Panic message contains a brace - --> $DIR/panic-brace.rs:7:5 +warning: Panic message contains an unused formatting placeholder + --> $DIR/panic-brace.rs:7:18 | -LL | core::panic!("Hello { { {"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | core::panic!("Hello {}"); + | ^^^^^^^^^^ | - = note: This message is not used as a format string, but will be in a future Rust version -help: add a "{}" format string to use the message literally + = note: This message is not used as a format string when given without arguments, but will be in a future Rust version +help: add the missing argument(s) + | +LL | core::panic!("Hello {}", argument); + | ^^^^^^^^^^ +help: or add a "{}" format string to use the message literally | -LL | core::panic!("{}", "Hello { { {"); +LL | core::panic!("{}", "Hello {}"); | ^^^^^ -warning: Panic message contains a brace - --> $DIR/panic-brace.rs:8:5 +warning: Panic message contains an unused formatting placeholder + --> $DIR/panic-brace.rs:8:20 | -LL | assert!(false, "} } }..."); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | assert!(false, "{:03x} bla"); + | ^^^^^^^^^^^^ | - = note: This message is not used as a format string, but will be in a future Rust version -help: add a "{}" format string to use the message literally + = note: This message is not used as a format string when given without arguments, but will be in a future Rust version +help: add the missing argument(s) + | +LL | assert!(false, "{:03x} bla", argument); + | ^^^^^^^^^^ +help: or add a "{}" format string to use the message literally | -LL | assert!(false, "{}", "} } }..."); +LL | assert!(false, "{}", "{:03x} bla"); | ^^^^^ warning: 4 warnings emitted From 44a03180748d7f33454b706b196c246c3687e4fa Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sun, 18 Oct 2020 23:45:20 +0200 Subject: [PATCH 16/42] Add cfg(not(bootstrap)) on the new rustc_diagnostic_item attributes. The beta compiler doesn't accept rustc_diagnostic_items on macros yet. --- library/core/src/macros/mod.rs | 4 ++-- library/std/src/macros.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index b84e86e9073ec..6358779f7b1c7 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -2,7 +2,7 @@ #[macro_export] #[allow_internal_unstable(core_panic, const_caller_location)] #[stable(feature = "core", since = "1.6.0")] -#[rustc_diagnostic_item = "core_panic_macro"] +#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "core_panic_macro")] macro_rules! panic { () => ( $crate::panic!("explicit panic") @@ -1216,7 +1216,7 @@ pub(crate) mod builtin { #[stable(feature = "rust1", since = "1.0.0")] #[rustc_builtin_macro] #[macro_export] - #[rustc_diagnostic_item = "assert_macro"] + #[cfg_attr(not(bootstrap), rustc_diagnostic_item = "assert_macro")] #[allow_internal_unstable(core_panic)] macro_rules! assert { ($cond:expr $(,)?) => {{ /* compiler built-in */ }}; diff --git a/library/std/src/macros.rs b/library/std/src/macros.rs index d003dd9fe84b8..f34eea1f8aca4 100644 --- a/library/std/src/macros.rs +++ b/library/std/src/macros.rs @@ -8,7 +8,7 @@ #[macro_export] #[stable(feature = "rust1", since = "1.0.0")] #[allow_internal_unstable(libstd_sys_internals)] -#[rustc_diagnostic_item = "std_panic_macro"] +#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "std_panic_macro")] macro_rules! panic { () => ({ $crate::panic!("explicit panic") }); ($msg:expr $(,)?) => ({ $crate::rt::begin_panic($msg) }); From f6e4459b56b3f8fa58437555b01cfc51544dd1b7 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 19 Oct 2020 00:05:19 +0200 Subject: [PATCH 17/42] Don't see `{{}}` as placeholder in panic_fmt lint. --- compiler/rustc_lint/src/panic_fmt.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs index a615d57dfa705..75ee0896510f0 100644 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -47,19 +47,18 @@ impl<'tcx> LateLintPass<'tcx> for PanicFmt { fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'tcx>) { if let hir::ExprKind::Lit(lit) = &arg.kind { if let ast::LitKind::Str(sym, _) = lit.node { - let s = sym.as_str(); - let open = s.find('{'); - let close = s[open.unwrap_or(0)..].find('}'); - let looks_like_placeholder = match (open, close) { - (Some(_), Some(_)) => true, - (Some(_), None) | (None, Some(_)) => false, - (None, None) => return, // OK, no braces. - }; let expn = f.span.ctxt().outer_expn_data(); if let Some(id) = expn.macro_def_id { if cx.tcx.is_diagnostic_item(sym::std_panic_macro, id) || cx.tcx.is_diagnostic_item(sym::core_panic_macro, id) { + let s = sym.as_str(); + if !s.contains(&['{', '}'][..]) { + return; + } + let s = s.replace("{{", "").replace("}}", ""); + let looks_like_placeholder = + s.find('{').map_or(false, |i| s[i + 1..].contains('}')); let expn = { // Unwrap another level of macro expansion if this // panic!() was expanded from assert!(). From 6dce5b427262e66193bc89bac72050aa9895da6f Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 19 Oct 2020 00:05:45 +0200 Subject: [PATCH 18/42] Fix brace problem in panic message in rustc_expand. --- compiler/rustc_expand/src/mbe/macro_rules.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 791d2686cb5eb..44565096200eb 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -1174,7 +1174,8 @@ fn quoted_tt_to_string(tt: &mbe::TokenTree) -> String { mbe::TokenTree::MetaVar(_, name) => format!("${}", name), mbe::TokenTree::MetaVarDecl(_, name, kind) => format!("${}:{}", name, kind), _ => panic!( - "unexpected mbe::TokenTree::{{Sequence or Delimited}} \ + "{}", + "unexpected mbe::TokenTree::{Sequence or Delimited} \ in follow set checker" ), } From 671d3e33a7a2b4d6a09da6b8823f3dd831fe7418 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 19 Oct 2020 00:25:17 +0200 Subject: [PATCH 19/42] Ignore panic_fmt lint in macro-comma-behavior-rpass ui test. --- src/test/ui/macros/macro-comma-behavior-rpass.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/ui/macros/macro-comma-behavior-rpass.rs b/src/test/ui/macros/macro-comma-behavior-rpass.rs index 32cf59294e760..e5e656de6fa7f 100644 --- a/src/test/ui/macros/macro-comma-behavior-rpass.rs +++ b/src/test/ui/macros/macro-comma-behavior-rpass.rs @@ -57,6 +57,7 @@ fn writeln_1arg() { // // (Example: Issue #48042) #[test] +#[allow(panic_fmt)] fn to_format_or_not_to_format() { // ("{}" is the easiest string to test because if this gets // sent to format_args!, it'll simply fail to compile. From 1481f0de876756b0bfdcce6b8a08a06e631eff06 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 19 Oct 2020 00:45:07 +0200 Subject: [PATCH 20/42] Also apply panic_fmt lint suggestions to debug_assert!(). --- compiler/rustc_lint/src/panic_fmt.rs | 16 +++++++--------- compiler/rustc_span/src/symbol.rs | 1 + library/core/src/macros/mod.rs | 1 + src/test/ui/panic-brace.rs | 1 + src/test/ui/panic-brace.stderr | 14 +++++++++++++- 5 files changed, 23 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs index 75ee0896510f0..05bc272d0e0a0 100644 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -47,7 +47,7 @@ impl<'tcx> LateLintPass<'tcx> for PanicFmt { fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'tcx>) { if let hir::ExprKind::Lit(lit) = &arg.kind { if let ast::LitKind::Str(sym, _) = lit.node { - let expn = f.span.ctxt().outer_expn_data(); + let mut expn = f.span.ctxt().outer_expn_data(); if let Some(id) = expn.macro_def_id { if cx.tcx.is_diagnostic_item(sym::std_panic_macro, id) || cx.tcx.is_diagnostic_item(sym::core_panic_macro, id) @@ -59,19 +59,17 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc let s = s.replace("{{", "").replace("}}", ""); let looks_like_placeholder = s.find('{').map_or(false, |i| s[i + 1..].contains('}')); - let expn = { - // Unwrap another level of macro expansion if this - // panic!() was expanded from assert!(). + // Unwrap another level of macro expansion if this panic!() + // was expanded from assert!() or debug_assert!(). + for &assert in &[sym::assert_macro, sym::debug_assert_macro] { let parent = expn.call_site.ctxt().outer_expn_data(); if parent .macro_def_id - .map_or(false, |id| cx.tcx.is_diagnostic_item(sym::assert_macro, id)) + .map_or(false, |id| cx.tcx.is_diagnostic_item(assert, id)) { - parent - } else { - expn + expn = parent; } - }; + } if looks_like_placeholder { cx.struct_span_lint(PANIC_FMT, arg.span.source_callsite(), |lint| { let mut l = lint.build("Panic message contains an unused formatting placeholder"); diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index d128491da2126..e42749278db87 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -416,6 +416,7 @@ symbols! { dead_code, dealloc, debug, + debug_assert_macro, debug_assertions, debug_struct, debug_trait, diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index 6358779f7b1c7..fc82a731ef6aa 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -163,6 +163,7 @@ macro_rules! assert_ne { /// ``` #[macro_export] #[stable(feature = "rust1", since = "1.0.0")] +#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "debug_assert_macro")] macro_rules! debug_assert { ($($arg:tt)*) => (if $crate::cfg!(debug_assertions) { $crate::assert!($($arg)*); }) } diff --git a/src/test/ui/panic-brace.rs b/src/test/ui/panic-brace.rs index 01ffb80076803..d6bb3222ac7e8 100644 --- a/src/test/ui/panic-brace.rs +++ b/src/test/ui/panic-brace.rs @@ -6,4 +6,5 @@ fn main() { std::panic!("another one: }"); //~ WARN Panic message contains a brace core::panic!("Hello {}"); //~ WARN Panic message contains an unused formatting placeholder assert!(false, "{:03x} bla"); //~ WARN Panic message contains an unused formatting placeholder + debug_assert!(false, "{{}} bla"); //~ WARN Panic message contains a brace } diff --git a/src/test/ui/panic-brace.stderr b/src/test/ui/panic-brace.stderr index b91ce8f603a55..578731896ba5f 100644 --- a/src/test/ui/panic-brace.stderr +++ b/src/test/ui/panic-brace.stderr @@ -55,5 +55,17 @@ help: or add a "{}" format string to use the message literally LL | assert!(false, "{}", "{:03x} bla"); | ^^^^^ -warning: 4 warnings emitted +warning: Panic message contains a brace + --> $DIR/panic-brace.rs:9:5 + | +LL | debug_assert!(false, "{{}} bla"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: This message is not used as a format string, but will be in a future Rust version +help: add a "{}" format string to use the message literally + | +LL | debug_assert!(false, "{}", "{{}} bla"); + | ^^^^^ + +warning: 5 warnings emitted From 7489ae2f0a7168d90c4a56a4c8832b2cc74bf37d Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 19 Oct 2020 00:45:42 +0200 Subject: [PATCH 21/42] Ignore panic_fmt lint in format-args-capture ui test. --- src/test/ui/fmt/format-args-capture.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/ui/fmt/format-args-capture.rs b/src/test/ui/fmt/format-args-capture.rs index 7490632110c85..b7d7ca4118103 100644 --- a/src/test/ui/fmt/format-args-capture.rs +++ b/src/test/ui/fmt/format-args-capture.rs @@ -27,6 +27,7 @@ fn panic_with_single_argument_does_not_get_formatted() { // RFC #2795 suggests that this may need to change so that captured arguments are formatted. // For stability reasons this will need to part of an edition change. + #[allow(panic_fmt)] let msg = std::panic::catch_unwind(|| { panic!("{foo}"); }).unwrap_err(); From e6a6120ad0e358a52d94e83906c476c1de055b1c Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 19 Oct 2020 09:44:57 +0200 Subject: [PATCH 22/42] Update mir-opt test output for new assert macro implementation. --- ...st_combine_deref.do_not_miscompile.InstCombine.diff | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/mir-opt/inst_combine_deref.do_not_miscompile.InstCombine.diff b/src/test/mir-opt/inst_combine_deref.do_not_miscompile.InstCombine.diff index 23c18bde2262b..dac9ec3b443d7 100644 --- a/src/test/mir-opt/inst_combine_deref.do_not_miscompile.InstCombine.diff +++ b/src/test/mir-opt/inst_combine_deref.do_not_miscompile.InstCombine.diff @@ -10,7 +10,7 @@ let mut _8: bool; // in scope 0 at $DIR/inst_combine_deref.rs:60:5: 60:23 let mut _9: bool; // in scope 0 at $DIR/inst_combine_deref.rs:60:13: 60:21 let mut _10: i32; // in scope 0 at $DIR/inst_combine_deref.rs:60:13: 60:15 - let mut _11: !; // in scope 0 at $SRC_DIR/std/src/macros.rs:LL:COL + let mut _11: !; // in scope 0 at $DIR/inst_combine_deref.rs:60:5: 60:23 scope 1 { debug x => _1; // in scope 1 at $DIR/inst_combine_deref.rs:55:9: 55:10 let _2: i32; // in scope 1 at $DIR/inst_combine_deref.rs:56:9: 56:10 @@ -69,11 +69,11 @@ } bb2: { - StorageLive(_11); // scope 4 at $SRC_DIR/std/src/macros.rs:LL:COL - begin_panic::<&str>(const "assertion failed: *y == 99"); // scope 4 at $SRC_DIR/std/src/macros.rs:LL:COL + StorageLive(_11); // scope 4 at $DIR/inst_combine_deref.rs:60:5: 60:23 + core::panicking::panic(const "assertion failed: *y == 99"); // scope 4 at $DIR/inst_combine_deref.rs:60:5: 60:23 // mir::Constant - // + span: $SRC_DIR/std/src/macros.rs:LL:COL - // + literal: Const { ty: fn(&str) -> ! {std::rt::begin_panic::<&str>}, val: Value(Scalar()) } + // + span: $DIR/inst_combine_deref.rs:60:5: 60:23 + // + literal: Const { ty: fn(&'static str) -> ! {core::panicking::panic}, val: Value(Scalar()) } // ty::Const // + ty: &str // + val: Value(Slice { data: Allocation { bytes: [97, 115, 115, 101, 114, 116, 105, 111, 110, 32, 102, 97, 105, 108, 101, 100, 58, 32, 42, 121, 32, 61, 61, 32, 57, 57], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [67108863], len: Size { raw: 26 } }, size: Size { raw: 26 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 26 }) From 33a3f06a9e0af444152269712c5171fc8d291c16 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 19 Oct 2020 10:07:30 +0200 Subject: [PATCH 23/42] Fix braces in panic message in test. --- library/core/tests/nonzero.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/tests/nonzero.rs b/library/core/tests/nonzero.rs index 825e5e63b59bc..a22b26ec250e3 100644 --- a/library/core/tests/nonzero.rs +++ b/library/core/tests/nonzero.rs @@ -82,7 +82,7 @@ fn test_match_option_string() { let five = "Five".to_string(); match Some(five) { Some(s) => assert_eq!(s, "Five"), - None => panic!("unexpected None while matching on Some(String { ... })"), + None => panic!("{}", "unexpected None while matching on Some(String { ... })"), } } From 1eb0a2c2052fcf939fff21d9c228690688bea746 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 19 Oct 2020 10:57:44 +0200 Subject: [PATCH 24/42] Add cfg(not(test)) to std_panic_macro rustc_diagnostic_item. --- library/std/src/macros.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/macros.rs b/library/std/src/macros.rs index f34eea1f8aca4..de072e83dfc41 100644 --- a/library/std/src/macros.rs +++ b/library/std/src/macros.rs @@ -8,7 +8,7 @@ #[macro_export] #[stable(feature = "rust1", since = "1.0.0")] #[allow_internal_unstable(libstd_sys_internals)] -#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "std_panic_macro")] +#[cfg_attr(not(any(bootstrap, test)), rustc_diagnostic_item = "std_panic_macro")] macro_rules! panic { () => ({ $crate::panic!("explicit panic") }); ($msg:expr $(,)?) => ({ $crate::rt::begin_panic($msg) }); From db162b6e6c944a70a2f67fcea5b81295a467f2a7 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Mon, 19 Oct 2020 21:14:05 +0200 Subject: [PATCH 25/42] Small cleanups in assert!() and panic_fmt lint. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (From the PR feedback.) Co-authored-by: Esteban Küber --- compiler/rustc_builtin_macros/src/assert.rs | 60 ++++++++++----------- compiler/rustc_lint/src/panic_fmt.rs | 8 +-- src/test/ui/panic-brace.rs | 10 ++-- src/test/ui/panic-brace.stderr | 20 +++---- 4 files changed, 48 insertions(+), 50 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/assert.rs b/compiler/rustc_builtin_macros/src/assert.rs index bc3276538da8f..bb6d3f6a0076c 100644 --- a/compiler/rustc_builtin_macros/src/assert.rs +++ b/compiler/rustc_builtin_macros/src/assert.rs @@ -27,37 +27,35 @@ pub fn expand_assert<'cx>( // context to pick up whichever is currently in scope. let sp = cx.with_call_site_ctxt(sp); - let panic_call = { - if let Some(tokens) = custom_message { - // Pass the custom message to panic!(). - cx.expr( - sp, - ExprKind::MacCall(MacCall { - path: Path::from_ident(Ident::new(sym::panic, sp)), - args: P(MacArgs::Delimited( - DelimSpan::from_single(sp), - MacDelimiter::Parenthesis, - tokens, - )), - prior_type_ascription: None, - }), - ) - } else { - // Pass our own message directly to $crate::panicking::panic(), - // because it might contain `{` and `}` that should always be - // passed literally. - cx.expr_call_global( - sp, - cx.std_path(&[sym::panicking, sym::panic]), - vec![cx.expr_str( - DUMMY_SP, - Symbol::intern(&format!( - "assertion failed: {}", - pprust::expr_to_string(&cond_expr).escape_debug() - )), - )], - ) - } + let panic_call = if let Some(tokens) = custom_message { + // Pass the custom message to panic!(). + cx.expr( + sp, + ExprKind::MacCall(MacCall { + path: Path::from_ident(Ident::new(sym::panic, sp)), + args: P(MacArgs::Delimited( + DelimSpan::from_single(sp), + MacDelimiter::Parenthesis, + tokens, + )), + prior_type_ascription: None, + }), + ) + } else { + // Pass our own message directly to $crate::panicking::panic(), + // because it might contain `{` and `}` that should always be + // passed literally. + cx.expr_call_global( + sp, + cx.std_path(&[sym::panicking, sym::panic]), + vec![cx.expr_str( + DUMMY_SP, + Symbol::intern(&format!( + "assertion failed: {}", + pprust::expr_to_string(&cond_expr).escape_debug() + )), + )], + ) }; let if_expr = cx.expr_if(sp, cx.expr(sp, ExprKind::Unary(UnOp::Not, cond_expr)), panic_call, None); diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs index 05bc272d0e0a0..288e1d61bbf49 100644 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -72,8 +72,8 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc } if looks_like_placeholder { cx.struct_span_lint(PANIC_FMT, arg.span.source_callsite(), |lint| { - let mut l = lint.build("Panic message contains an unused formatting placeholder"); - l.note("This message is not used as a format string when given without arguments, but will be in a future Rust version"); + let mut l = lint.build("panic message contains an unused formatting placeholder"); + l.note("this message is not used as a format string when given without arguments, but will be in a future Rust version"); if expn.call_site.contains(arg.span) { l.span_suggestion( arg.span.shrink_to_hi(), @@ -92,8 +92,8 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc }); } else { cx.struct_span_lint(PANIC_FMT, expn.call_site, |lint| { - let mut l = lint.build("Panic message contains a brace"); - l.note("This message is not used as a format string, but will be in a future Rust version"); + let mut l = lint.build("panic message contains a brace"); + l.note("this message is not used as a format string, but will be in a future Rust version"); if expn.call_site.contains(arg.span) { l.span_suggestion( arg.span.shrink_to_lo(), diff --git a/src/test/ui/panic-brace.rs b/src/test/ui/panic-brace.rs index d6bb3222ac7e8..6ab5fafee887c 100644 --- a/src/test/ui/panic-brace.rs +++ b/src/test/ui/panic-brace.rs @@ -2,9 +2,9 @@ #[allow(unreachable_code)] fn main() { - panic!("here's a brace: {"); //~ WARN Panic message contains a brace - std::panic!("another one: }"); //~ WARN Panic message contains a brace - core::panic!("Hello {}"); //~ WARN Panic message contains an unused formatting placeholder - assert!(false, "{:03x} bla"); //~ WARN Panic message contains an unused formatting placeholder - debug_assert!(false, "{{}} bla"); //~ WARN Panic message contains a brace + panic!("here's a brace: {"); //~ WARN panic message contains a brace + std::panic!("another one: }"); //~ WARN panic message contains a brace + core::panic!("Hello {}"); //~ WARN panic message contains an unused formatting placeholder + assert!(false, "{:03x} bla"); //~ WARN panic message contains an unused formatting placeholder + debug_assert!(false, "{{}} bla"); //~ WARN panic message contains a brace } diff --git a/src/test/ui/panic-brace.stderr b/src/test/ui/panic-brace.stderr index 578731896ba5f..00b005a59d8c6 100644 --- a/src/test/ui/panic-brace.stderr +++ b/src/test/ui/panic-brace.stderr @@ -1,35 +1,35 @@ -warning: Panic message contains a brace +warning: panic message contains a brace --> $DIR/panic-brace.rs:5:5 | LL | panic!("here's a brace: {"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(panic_fmt)]` on by default - = note: This message is not used as a format string, but will be in a future Rust version + = note: this message is not used as a format string, but will be in a future Rust version help: add a "{}" format string to use the message literally | LL | panic!("{}", "here's a brace: {"); | ^^^^^ -warning: Panic message contains a brace +warning: panic message contains a brace --> $DIR/panic-brace.rs:6:5 | LL | std::panic!("another one: }"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: This message is not used as a format string, but will be in a future Rust version + = note: this message is not used as a format string, but will be in a future Rust version help: add a "{}" format string to use the message literally | LL | std::panic!("{}", "another one: }"); | ^^^^^ -warning: Panic message contains an unused formatting placeholder +warning: panic message contains an unused formatting placeholder --> $DIR/panic-brace.rs:7:18 | LL | core::panic!("Hello {}"); | ^^^^^^^^^^ | - = note: This message is not used as a format string when given without arguments, but will be in a future Rust version + = note: this message is not used as a format string when given without arguments, but will be in a future Rust version help: add the missing argument(s) | LL | core::panic!("Hello {}", argument); @@ -39,13 +39,13 @@ help: or add a "{}" format string to use the message literally LL | core::panic!("{}", "Hello {}"); | ^^^^^ -warning: Panic message contains an unused formatting placeholder +warning: panic message contains an unused formatting placeholder --> $DIR/panic-brace.rs:8:20 | LL | assert!(false, "{:03x} bla"); | ^^^^^^^^^^^^ | - = note: This message is not used as a format string when given without arguments, but will be in a future Rust version + = note: this message is not used as a format string when given without arguments, but will be in a future Rust version help: add the missing argument(s) | LL | assert!(false, "{:03x} bla", argument); @@ -55,13 +55,13 @@ help: or add a "{}" format string to use the message literally LL | assert!(false, "{}", "{:03x} bla"); | ^^^^^ -warning: Panic message contains a brace +warning: panic message contains a brace --> $DIR/panic-brace.rs:9:5 | LL | debug_assert!(false, "{{}} bla"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: This message is not used as a format string, but will be in a future Rust version + = note: this message is not used as a format string, but will be in a future Rust version help: add a "{}" format string to use the message literally | LL | debug_assert!(false, "{}", "{{}} bla"); From 8212aaee97ce5b0f1256c5dc4205f5df8b138ebd Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 20 Oct 2020 22:25:42 +0200 Subject: [PATCH 26/42] Parse the format string for the panic_fmt lint for better warnings. --- Cargo.lock | 1 + compiler/rustc_lint/Cargo.toml | 1 + compiler/rustc_lint/src/panic_fmt.rs | 42 ++++++++++++++++++++++------ src/test/ui/panic-brace.rs | 2 +- src/test/ui/panic-brace.stderr | 22 +++++++-------- 5 files changed, 47 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 65d20190c0db5..a3e28dd089045 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3822,6 +3822,7 @@ dependencies = [ "rustc_hir", "rustc_index", "rustc_middle", + "rustc_parse_format", "rustc_session", "rustc_span", "rustc_target", diff --git a/compiler/rustc_lint/Cargo.toml b/compiler/rustc_lint/Cargo.toml index 760a8e385d680..c56eb09b63471 100644 --- a/compiler/rustc_lint/Cargo.toml +++ b/compiler/rustc_lint/Cargo.toml @@ -20,3 +20,4 @@ rustc_feature = { path = "../rustc_feature" } rustc_index = { path = "../rustc_index" } rustc_session = { path = "../rustc_session" } rustc_trait_selection = { path = "../rustc_trait_selection" } +rustc_parse_format = { path = "../rustc_parse_format" } diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs index 288e1d61bbf49..cff50ff9912af 100644 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -3,6 +3,7 @@ use rustc_ast as ast; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_middle::ty; +use rustc_parse_format::{ParseMode, Parser, Piece}; use rustc_span::sym; declare_lint! { @@ -52,13 +53,28 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc if cx.tcx.is_diagnostic_item(sym::std_panic_macro, id) || cx.tcx.is_diagnostic_item(sym::core_panic_macro, id) { - let s = sym.as_str(); - if !s.contains(&['{', '}'][..]) { + let fmt = sym.as_str(); + if !fmt.contains(&['{', '}'][..]) { return; } - let s = s.replace("{{", "").replace("}}", ""); - let looks_like_placeholder = - s.find('{').map_or(false, |i| s[i + 1..].contains('}')); + + let fmt_span = arg.span.source_callsite(); + + let (snippet, style) = + match cx.sess().parse_sess.source_map().span_to_snippet(fmt_span) { + Ok(snippet) => { + // Count the number of `#`s between the `r` and `"`. + let style = snippet.strip_prefix('r').and_then(|s| s.find('"')); + (Some(snippet), style) + } + Err(_) => (None, None), + }; + + let mut fmt_parser = + Parser::new(fmt.as_ref(), style, snippet, false, ParseMode::Format); + let n_arguments = + (&mut fmt_parser).filter(|a| matches!(a, Piece::NextArgument(_))).count(); + // Unwrap another level of macro expansion if this panic!() // was expanded from assert!() or debug_assert!(). for &assert in &[sym::assert_macro, sym::debug_assert_macro] { @@ -70,15 +86,23 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc expn = parent; } } - if looks_like_placeholder { - cx.struct_span_lint(PANIC_FMT, arg.span.source_callsite(), |lint| { - let mut l = lint.build("panic message contains an unused formatting placeholder"); + + if n_arguments > 0 && fmt_parser.errors.is_empty() { + let arg_spans: Vec<_> = match &fmt_parser.arg_places[..] { + [] => vec![fmt_span], + v => v.iter().map(|span| fmt_span.from_inner(*span)).collect(), + }; + cx.struct_span_lint(PANIC_FMT, arg_spans, |lint| { + let mut l = lint.build(match n_arguments { + 1 => "panic message contains an unused formatting placeholder", + _ => "panic message contains unused formatting placeholders", + }); l.note("this message is not used as a format string when given without arguments, but will be in a future Rust version"); if expn.call_site.contains(arg.span) { l.span_suggestion( arg.span.shrink_to_hi(), "add the missing argument(s)", - ", argument".into(), + ", ...".into(), Applicability::HasPlaceholders, ); l.span_suggestion( diff --git a/src/test/ui/panic-brace.rs b/src/test/ui/panic-brace.rs index 6ab5fafee887c..e74b6ad96c2c0 100644 --- a/src/test/ui/panic-brace.rs +++ b/src/test/ui/panic-brace.rs @@ -5,6 +5,6 @@ fn main() { panic!("here's a brace: {"); //~ WARN panic message contains a brace std::panic!("another one: }"); //~ WARN panic message contains a brace core::panic!("Hello {}"); //~ WARN panic message contains an unused formatting placeholder - assert!(false, "{:03x} bla"); //~ WARN panic message contains an unused formatting placeholder + assert!(false, "{:03x} {test} bla"); //~ WARN panic message contains unused formatting placeholders debug_assert!(false, "{{}} bla"); //~ WARN panic message contains a brace } diff --git a/src/test/ui/panic-brace.stderr b/src/test/ui/panic-brace.stderr index 00b005a59d8c6..23ae31d00ebd2 100644 --- a/src/test/ui/panic-brace.stderr +++ b/src/test/ui/panic-brace.stderr @@ -24,35 +24,35 @@ LL | std::panic!("{}", "another one: }"); | ^^^^^ warning: panic message contains an unused formatting placeholder - --> $DIR/panic-brace.rs:7:18 + --> $DIR/panic-brace.rs:7:25 | LL | core::panic!("Hello {}"); - | ^^^^^^^^^^ + | ^^ | = note: this message is not used as a format string when given without arguments, but will be in a future Rust version help: add the missing argument(s) | -LL | core::panic!("Hello {}", argument); - | ^^^^^^^^^^ +LL | core::panic!("Hello {}", ...); + | ^^^^^ help: or add a "{}" format string to use the message literally | LL | core::panic!("{}", "Hello {}"); | ^^^^^ -warning: panic message contains an unused formatting placeholder - --> $DIR/panic-brace.rs:8:20 +warning: panic message contains unused formatting placeholders + --> $DIR/panic-brace.rs:8:21 | -LL | assert!(false, "{:03x} bla"); - | ^^^^^^^^^^^^ +LL | assert!(false, "{:03x} {test} bla"); + | ^^^^^^ ^^^^^^ | = note: this message is not used as a format string when given without arguments, but will be in a future Rust version help: add the missing argument(s) | -LL | assert!(false, "{:03x} bla", argument); - | ^^^^^^^^^^ +LL | assert!(false, "{:03x} {test} bla", ...); + | ^^^^^ help: or add a "{}" format string to use the message literally | -LL | assert!(false, "{}", "{:03x} bla"); +LL | assert!(false, "{}", "{:03x} {test} bla"); | ^^^^^ warning: panic message contains a brace From 1e4bc20ee181e2f21116e8b5f21d380f84a7322f Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 20 Oct 2020 22:59:53 +0200 Subject: [PATCH 27/42] Improve panic_fmt error messages for invalid format strings too. --- compiler/rustc_lint/src/panic_fmt.rs | 22 ++++++++++--- src/test/ui/panic-brace.rs | 12 +++++-- src/test/ui/panic-brace.stderr | 48 ++++++++++++++++++++++------ 3 files changed, 66 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs index cff50ff9912af..7428a9d13ff79 100644 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -4,7 +4,7 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_middle::ty; use rustc_parse_format::{ParseMode, Parser, Piece}; -use rustc_span::sym; +use rustc_span::{sym, InnerSpan}; declare_lint! { /// The `panic_fmt` lint detects `panic!("..")` with `{` or `}` in the string literal. @@ -71,7 +71,7 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc }; let mut fmt_parser = - Parser::new(fmt.as_ref(), style, snippet, false, ParseMode::Format); + Parser::new(fmt.as_ref(), style, snippet.clone(), false, ParseMode::Format); let n_arguments = (&mut fmt_parser).filter(|a| matches!(a, Piece::NextArgument(_))).count(); @@ -115,8 +115,22 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc l.emit(); }); } else { - cx.struct_span_lint(PANIC_FMT, expn.call_site, |lint| { - let mut l = lint.build("panic message contains a brace"); + let brace_spans: Option> = snippet + .filter(|s| s.starts_with('"') || s.starts_with("r#")) + .map(|s| { + s.char_indices() + .filter(|&(_, c)| c == '{' || c == '}') + .map(|(i, _)| { + fmt_span.from_inner(InnerSpan { start: i, end: i + 1 }) + }) + .collect() + }); + let msg = match &brace_spans { + Some(v) if v.len() == 1 => "panic message contains a brace", + _ => "panic message contains braces", + }; + cx.struct_span_lint(PANIC_FMT, brace_spans.unwrap_or(vec![expn.call_site]), |lint| { + let mut l = lint.build(msg); l.note("this message is not used as a format string, but will be in a future Rust version"); if expn.call_site.contains(arg.span) { l.span_suggestion( diff --git a/src/test/ui/panic-brace.rs b/src/test/ui/panic-brace.rs index e74b6ad96c2c0..d38d8ac4deb32 100644 --- a/src/test/ui/panic-brace.rs +++ b/src/test/ui/panic-brace.rs @@ -1,10 +1,18 @@ // build-pass (FIXME(62277): should be check-pass) +const C: &str = "abc {}"; +static S: &str = "{bla}"; + #[allow(unreachable_code)] fn main() { panic!("here's a brace: {"); //~ WARN panic message contains a brace std::panic!("another one: }"); //~ WARN panic message contains a brace core::panic!("Hello {}"); //~ WARN panic message contains an unused formatting placeholder - assert!(false, "{:03x} {test} bla"); //~ WARN panic message contains unused formatting placeholders - debug_assert!(false, "{{}} bla"); //~ WARN panic message contains a brace + assert!(false, "{:03x} {test} bla"); + //~^ WARN panic message contains unused formatting placeholders + debug_assert!(false, "{{}} bla"); //~ WARN panic message contains braces + panic!(C); // No warning (yet) + panic!(S); // No warning (yet) + panic!(concat!("{", "}")); //~ WARN panic message contains an unused formatting placeholder + panic!(concat!("{", "{")); //~ WARN panic message contains braces } diff --git a/src/test/ui/panic-brace.stderr b/src/test/ui/panic-brace.stderr index 23ae31d00ebd2..16795ed3d36e8 100644 --- a/src/test/ui/panic-brace.stderr +++ b/src/test/ui/panic-brace.stderr @@ -1,8 +1,8 @@ warning: panic message contains a brace - --> $DIR/panic-brace.rs:5:5 + --> $DIR/panic-brace.rs:8:29 | LL | panic!("here's a brace: {"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ | = note: `#[warn(panic_fmt)]` on by default = note: this message is not used as a format string, but will be in a future Rust version @@ -12,10 +12,10 @@ LL | panic!("{}", "here's a brace: {"); | ^^^^^ warning: panic message contains a brace - --> $DIR/panic-brace.rs:6:5 + --> $DIR/panic-brace.rs:9:31 | LL | std::panic!("another one: }"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ | = note: this message is not used as a format string, but will be in a future Rust version help: add a "{}" format string to use the message literally @@ -24,7 +24,7 @@ LL | std::panic!("{}", "another one: }"); | ^^^^^ warning: panic message contains an unused formatting placeholder - --> $DIR/panic-brace.rs:7:25 + --> $DIR/panic-brace.rs:10:25 | LL | core::panic!("Hello {}"); | ^^ @@ -40,7 +40,7 @@ LL | core::panic!("{}", "Hello {}"); | ^^^^^ warning: panic message contains unused formatting placeholders - --> $DIR/panic-brace.rs:8:21 + --> $DIR/panic-brace.rs:11:21 | LL | assert!(false, "{:03x} {test} bla"); | ^^^^^^ ^^^^^^ @@ -55,11 +55,11 @@ help: or add a "{}" format string to use the message literally LL | assert!(false, "{}", "{:03x} {test} bla"); | ^^^^^ -warning: panic message contains a brace - --> $DIR/panic-brace.rs:9:5 +warning: panic message contains braces + --> $DIR/panic-brace.rs:13:27 | LL | debug_assert!(false, "{{}} bla"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^ | = note: this message is not used as a format string, but will be in a future Rust version help: add a "{}" format string to use the message literally @@ -67,5 +67,33 @@ help: add a "{}" format string to use the message literally LL | debug_assert!(false, "{}", "{{}} bla"); | ^^^^^ -warning: 5 warnings emitted +warning: panic message contains an unused formatting placeholder + --> $DIR/panic-brace.rs:16:12 + | +LL | panic!(concat!("{", "}")); + | ^^^^^^^^^^^^^^^^^ + | + = note: this message is not used as a format string when given without arguments, but will be in a future Rust version +help: add the missing argument(s) + | +LL | panic!(concat!("{", "}"), ...); + | ^^^^^ +help: or add a "{}" format string to use the message literally + | +LL | panic!("{}", concat!("{", "}")); + | ^^^^^ + +warning: panic message contains braces + --> $DIR/panic-brace.rs:17:5 + | +LL | panic!(concat!("{", "{")); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this message is not used as a format string, but will be in a future Rust version +help: add a "{}" format string to use the message literally + | +LL | panic!("{}", concat!("{", "{")); + | ^^^^^ + +warning: 7 warnings emitted From 9af971eb89e69c95226dec7d4e804fa164be3cbd Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sat, 24 Oct 2020 21:13:04 +0200 Subject: [PATCH 28/42] Test that panic_fmt lint doesn't trigger for custom panic macro. --- src/test/ui/panic-brace.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/ui/panic-brace.rs b/src/test/ui/panic-brace.rs index d38d8ac4deb32..e3ea7a8bf0f75 100644 --- a/src/test/ui/panic-brace.rs +++ b/src/test/ui/panic-brace.rs @@ -15,4 +15,11 @@ fn main() { panic!(S); // No warning (yet) panic!(concat!("{", "}")); //~ WARN panic message contains an unused formatting placeholder panic!(concat!("{", "{")); //~ WARN panic message contains braces + + // Check that the lint only triggers for std::panic and core::panic, + // not any panic macro: + macro_rules! panic { + ($e:expr) => (); + } + panic!("{}"); // OK } From 2ab39e01cadd931e582f61219e4feabe8a3e2112 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Wed, 28 Oct 2020 11:00:28 +0100 Subject: [PATCH 29/42] Mark panic_fmt suggestion as machine applicable. Co-authored-by: bjorn3 --- compiler/rustc_lint/src/panic_fmt.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs index 7428a9d13ff79..a08fae1f34e22 100644 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -109,7 +109,7 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc arg.span.shrink_to_lo(), "or add a \"{}\" format string to use the message literally", "\"{}\", ".into(), - Applicability::MaybeIncorrect, + Applicability::MachineApplicable, ); } l.emit(); From d2581f6667bceaabfffdda8d2b725f3065a660f4 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 29 Oct 2020 19:44:06 +0100 Subject: [PATCH 30/42] Improve panic_fmt lint messages. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (From the PR feedback.) Co-authored-by: Esteban Küber --- compiler/rustc_lint/src/panic_fmt.rs | 10 +++++----- src/test/ui/panic-brace.stderr | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_lint/src/panic_fmt.rs b/compiler/rustc_lint/src/panic_fmt.rs index a08fae1f34e22..0d2b20989b0c3 100644 --- a/compiler/rustc_lint/src/panic_fmt.rs +++ b/compiler/rustc_lint/src/panic_fmt.rs @@ -1,6 +1,6 @@ use crate::{LateContext, LateLintPass, LintContext}; use rustc_ast as ast; -use rustc_errors::Applicability; +use rustc_errors::{pluralize, Applicability}; use rustc_hir as hir; use rustc_middle::ty; use rustc_parse_format::{ParseMode, Parser, Piece}; @@ -21,7 +21,7 @@ declare_lint! { /// /// `panic!("{}")` panics with the message `"{}"`, as a `panic!()` invocation /// with a single argument does not use `format_args!()`. - /// A future version of Rust will interpret this string as format string, + /// A future edition of Rust will interpret this string as format string, /// which would break this. PANIC_FMT, Warn, @@ -97,11 +97,11 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc 1 => "panic message contains an unused formatting placeholder", _ => "panic message contains unused formatting placeholders", }); - l.note("this message is not used as a format string when given without arguments, but will be in a future Rust version"); + l.note("this message is not used as a format string when given without arguments, but will be in a future Rust edition"); if expn.call_site.contains(arg.span) { l.span_suggestion( arg.span.shrink_to_hi(), - "add the missing argument(s)", + &format!("add the missing argument{}", pluralize!(n_arguments)), ", ...".into(), Applicability::HasPlaceholders, ); @@ -131,7 +131,7 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc }; cx.struct_span_lint(PANIC_FMT, brace_spans.unwrap_or(vec![expn.call_site]), |lint| { let mut l = lint.build(msg); - l.note("this message is not used as a format string, but will be in a future Rust version"); + l.note("this message is not used as a format string, but will be in a future Rust edition"); if expn.call_site.contains(arg.span) { l.span_suggestion( arg.span.shrink_to_lo(), diff --git a/src/test/ui/panic-brace.stderr b/src/test/ui/panic-brace.stderr index 16795ed3d36e8..0520ab2a38f31 100644 --- a/src/test/ui/panic-brace.stderr +++ b/src/test/ui/panic-brace.stderr @@ -5,7 +5,7 @@ LL | panic!("here's a brace: {"); | ^ | = note: `#[warn(panic_fmt)]` on by default - = note: this message is not used as a format string, but will be in a future Rust version + = note: this message is not used as a format string, but will be in a future Rust edition help: add a "{}" format string to use the message literally | LL | panic!("{}", "here's a brace: {"); @@ -17,7 +17,7 @@ warning: panic message contains a brace LL | std::panic!("another one: }"); | ^ | - = note: this message is not used as a format string, but will be in a future Rust version + = note: this message is not used as a format string, but will be in a future Rust edition help: add a "{}" format string to use the message literally | LL | std::panic!("{}", "another one: }"); @@ -29,8 +29,8 @@ warning: panic message contains an unused formatting placeholder LL | core::panic!("Hello {}"); | ^^ | - = note: this message is not used as a format string when given without arguments, but will be in a future Rust version -help: add the missing argument(s) + = note: this message is not used as a format string when given without arguments, but will be in a future Rust edition +help: add the missing argument | LL | core::panic!("Hello {}", ...); | ^^^^^ @@ -45,8 +45,8 @@ warning: panic message contains unused formatting placeholders LL | assert!(false, "{:03x} {test} bla"); | ^^^^^^ ^^^^^^ | - = note: this message is not used as a format string when given without arguments, but will be in a future Rust version -help: add the missing argument(s) + = note: this message is not used as a format string when given without arguments, but will be in a future Rust edition +help: add the missing arguments | LL | assert!(false, "{:03x} {test} bla", ...); | ^^^^^ @@ -61,7 +61,7 @@ warning: panic message contains braces LL | debug_assert!(false, "{{}} bla"); | ^^^^ | - = note: this message is not used as a format string, but will be in a future Rust version + = note: this message is not used as a format string, but will be in a future Rust edition help: add a "{}" format string to use the message literally | LL | debug_assert!(false, "{}", "{{}} bla"); @@ -73,8 +73,8 @@ warning: panic message contains an unused formatting placeholder LL | panic!(concat!("{", "}")); | ^^^^^^^^^^^^^^^^^ | - = note: this message is not used as a format string when given without arguments, but will be in a future Rust version -help: add the missing argument(s) + = note: this message is not used as a format string when given without arguments, but will be in a future Rust edition +help: add the missing argument | LL | panic!(concat!("{", "}"), ...); | ^^^^^ @@ -89,7 +89,7 @@ warning: panic message contains braces LL | panic!(concat!("{", "{")); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: this message is not used as a format string, but will be in a future Rust version + = note: this message is not used as a format string, but will be in a future Rust edition help: add a "{}" format string to use the message literally | LL | panic!("{}", concat!("{", "{")); From 1e234734542a6761b61fea498890d34f2a494195 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 29 Oct 2020 22:21:40 +0100 Subject: [PATCH 31/42] Add test for panic_fmt lint with external panic!()-calling macro. --- src/test/ui/auxiliary/fancy-panic.rs | 6 ++++++ src/test/ui/panic-brace.rs | 6 ++++++ src/test/ui/panic-brace.stderr | 24 ++++++++++++++++-------- 3 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/auxiliary/fancy-panic.rs diff --git a/src/test/ui/auxiliary/fancy-panic.rs b/src/test/ui/auxiliary/fancy-panic.rs new file mode 100644 index 0000000000000..e5a25a171fbe0 --- /dev/null +++ b/src/test/ui/auxiliary/fancy-panic.rs @@ -0,0 +1,6 @@ +#[macro_export] +macro_rules! fancy_panic { + ($msg:expr) => { + panic!($msg) + }; +} diff --git a/src/test/ui/panic-brace.rs b/src/test/ui/panic-brace.rs index e3ea7a8bf0f75..754dcc287d0f9 100644 --- a/src/test/ui/panic-brace.rs +++ b/src/test/ui/panic-brace.rs @@ -1,4 +1,7 @@ // build-pass (FIXME(62277): should be check-pass) +// aux-build:fancy-panic.rs + +extern crate fancy_panic; const C: &str = "abc {}"; static S: &str = "{bla}"; @@ -16,6 +19,9 @@ fn main() { panic!(concat!("{", "}")); //~ WARN panic message contains an unused formatting placeholder panic!(concat!("{", "{")); //~ WARN panic message contains braces + fancy_panic::fancy_panic!("test {} 123"); + //~^ WARN panic message contains an unused formatting placeholder + // Check that the lint only triggers for std::panic and core::panic, // not any panic macro: macro_rules! panic { diff --git a/src/test/ui/panic-brace.stderr b/src/test/ui/panic-brace.stderr index 0520ab2a38f31..93808891c3c37 100644 --- a/src/test/ui/panic-brace.stderr +++ b/src/test/ui/panic-brace.stderr @@ -1,5 +1,5 @@ warning: panic message contains a brace - --> $DIR/panic-brace.rs:8:29 + --> $DIR/panic-brace.rs:11:29 | LL | panic!("here's a brace: {"); | ^ @@ -12,7 +12,7 @@ LL | panic!("{}", "here's a brace: {"); | ^^^^^ warning: panic message contains a brace - --> $DIR/panic-brace.rs:9:31 + --> $DIR/panic-brace.rs:12:31 | LL | std::panic!("another one: }"); | ^ @@ -24,7 +24,7 @@ LL | std::panic!("{}", "another one: }"); | ^^^^^ warning: panic message contains an unused formatting placeholder - --> $DIR/panic-brace.rs:10:25 + --> $DIR/panic-brace.rs:13:25 | LL | core::panic!("Hello {}"); | ^^ @@ -40,7 +40,7 @@ LL | core::panic!("{}", "Hello {}"); | ^^^^^ warning: panic message contains unused formatting placeholders - --> $DIR/panic-brace.rs:11:21 + --> $DIR/panic-brace.rs:14:21 | LL | assert!(false, "{:03x} {test} bla"); | ^^^^^^ ^^^^^^ @@ -56,7 +56,7 @@ LL | assert!(false, "{}", "{:03x} {test} bla"); | ^^^^^ warning: panic message contains braces - --> $DIR/panic-brace.rs:13:27 + --> $DIR/panic-brace.rs:16:27 | LL | debug_assert!(false, "{{}} bla"); | ^^^^ @@ -68,7 +68,7 @@ LL | debug_assert!(false, "{}", "{{}} bla"); | ^^^^^ warning: panic message contains an unused formatting placeholder - --> $DIR/panic-brace.rs:16:12 + --> $DIR/panic-brace.rs:19:12 | LL | panic!(concat!("{", "}")); | ^^^^^^^^^^^^^^^^^ @@ -84,7 +84,7 @@ LL | panic!("{}", concat!("{", "}")); | ^^^^^ warning: panic message contains braces - --> $DIR/panic-brace.rs:17:5 + --> $DIR/panic-brace.rs:20:5 | LL | panic!(concat!("{", "{")); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -95,5 +95,13 @@ help: add a "{}" format string to use the message literally LL | panic!("{}", concat!("{", "{")); | ^^^^^ -warning: 7 warnings emitted +warning: panic message contains an unused formatting placeholder + --> $DIR/panic-brace.rs:22:37 + | +LL | fancy_panic::fancy_panic!("test {} 123"); + | ^^ + | + = note: this message is not used as a format string when given without arguments, but will be in a future Rust edition + +warning: 8 warnings emitted From 439171e094e00e7d3ac0b2d8f65c23cac87836f2 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Sat, 7 Nov 2020 12:37:28 +0100 Subject: [PATCH 32/42] look at assoc ct, check the type of nodes --- .../src/traits/const_evaluatable.rs | 17 ++++++++++ .../associated-consts.rs | 31 +++++++++++++++++++ .../const_evaluatable_checked/different-fn.rs | 16 ++++++++++ .../different-fn.stderr | 14 +++++++++ 4 files changed, 78 insertions(+) create mode 100644 src/test/ui/const-generics/const_evaluatable_checked/associated-consts.rs create mode 100644 src/test/ui/const-generics/const_evaluatable_checked/different-fn.rs create mode 100644 src/test/ui/const-generics/const_evaluatable_checked/different-fn.stderr diff --git a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs index 638a8253e7eb9..e1721a5a88a76 100644 --- a/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs +++ b/compiler/rustc_trait_selection/src/traits/const_evaluatable.rs @@ -512,6 +512,13 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> { block = &self.body.basic_blocks()[next]; } else { assert_eq!(self.locals[mir::RETURN_PLACE], self.nodes.last().unwrap()); + // `AbstractConst`s should not contain any promoteds as they require references which + // are not allowed. + assert!(!self.nodes.iter().any(|n| matches!( + n.node, + Node::Leaf(ty::Const { val: ty::ConstKind::Unevaluated(_, _, Some(_)), ty: _ }) + ))); + self.nodes[self.locals[mir::RETURN_PLACE]].used = true; if let Some(&unused) = self.nodes.iter().find(|n| !n.used) { self.error(Some(unused.span), "dead code")?; @@ -609,6 +616,10 @@ pub(super) fn try_unify<'tcx>( (Node::Leaf(a_ct), Node::Leaf(b_ct)) => { let a_ct = a_ct.subst(tcx, a.substs); let b_ct = b_ct.subst(tcx, b.substs); + if a_ct.ty != b_ct.ty { + return false; + } + match (a_ct.val, b_ct.val) { // We can just unify errors with everything to reduce the amount of // emitted errors here. @@ -621,6 +632,12 @@ pub(super) fn try_unify<'tcx>( // we do not want to use `assert_eq!(a(), b())` to infer that `N` and `M` have to be `1`. This // means that we only allow inference variables if they are equal. (ty::ConstKind::Infer(a_val), ty::ConstKind::Infer(b_val)) => a_val == b_val, + // We may want to instead recurse into unevaluated constants here. That may require some + // care to prevent infinite recursion, so let's just ignore this for now. + ( + ty::ConstKind::Unevaluated(a_def, a_substs, None), + ty::ConstKind::Unevaluated(b_def, b_substs, None), + ) => a_def == b_def && a_substs == b_substs, // FIXME(const_evaluatable_checked): We may want to either actually try // to evaluate `a_ct` and `b_ct` if they are are fully concrete or something like // this, for now we just return false here. diff --git a/src/test/ui/const-generics/const_evaluatable_checked/associated-consts.rs b/src/test/ui/const-generics/const_evaluatable_checked/associated-consts.rs new file mode 100644 index 0000000000000..533fe55b45bee --- /dev/null +++ b/src/test/ui/const-generics/const_evaluatable_checked/associated-consts.rs @@ -0,0 +1,31 @@ +// run-pass +#![feature(const_generics, const_evaluatable_checked)] +#![allow(incomplete_features)] + +pub trait BlockCipher { + const BLOCK_SIZE: usize; +} + +struct FooCipher; +impl BlockCipher for FooCipher { + const BLOCK_SIZE: usize = 64; +} + +struct BarCipher; +impl BlockCipher for BarCipher { + const BLOCK_SIZE: usize = 32; +} + +pub struct Block(C); + +pub fn test() +where + [u8; M - C::BLOCK_SIZE]: Sized, +{ + let _ = [0; M - C::BLOCK_SIZE]; +} + +fn main() { + test::(); + test::(); +} diff --git a/src/test/ui/const-generics/const_evaluatable_checked/different-fn.rs b/src/test/ui/const-generics/const_evaluatable_checked/different-fn.rs new file mode 100644 index 0000000000000..05049d9c2a6ef --- /dev/null +++ b/src/test/ui/const-generics/const_evaluatable_checked/different-fn.rs @@ -0,0 +1,16 @@ +#![feature(const_generics, const_evaluatable_checked)] +#![allow(incomplete_features)] + +use std::mem::size_of; +use std::marker::PhantomData; + +struct Foo(PhantomData); + +fn test() -> [u8; size_of::()] { + [0; size_of::>()] + //~^ ERROR unconstrained generic constant +} + +fn main() { + test::(); +} diff --git a/src/test/ui/const-generics/const_evaluatable_checked/different-fn.stderr b/src/test/ui/const-generics/const_evaluatable_checked/different-fn.stderr new file mode 100644 index 0000000000000..1f6dddb04e56c --- /dev/null +++ b/src/test/ui/const-generics/const_evaluatable_checked/different-fn.stderr @@ -0,0 +1,14 @@ +error: unconstrained generic constant + --> $DIR/different-fn.rs:10:9 + | +LL | [0; size_of::>()] + | ^^^^^^^^^^^^^^^^^^^ + | +help: consider adding a `where` bound for this expression + --> $DIR/different-fn.rs:10:9 + | +LL | [0; size_of::>()] + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + From 685fd53ada562658e8ad1a3578efa4a2da4e83f4 Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Mon, 26 Oct 2020 10:59:31 +0100 Subject: [PATCH 33/42] BTreeMap: split off most code of append, slightly improve interfaces --- library/alloc/src/collections/btree/append.rs | 124 ++++++++++++++++++ library/alloc/src/collections/btree/map.rs | 96 +------------- .../alloc/src/collections/btree/map/tests.rs | 27 ++++ .../alloc/src/collections/btree/merge_iter.rs | 42 +++--- library/alloc/src/collections/btree/mod.rs | 1 + 5 files changed, 176 insertions(+), 114 deletions(-) create mode 100644 library/alloc/src/collections/btree/append.rs diff --git a/library/alloc/src/collections/btree/append.rs b/library/alloc/src/collections/btree/append.rs new file mode 100644 index 0000000000000..e0362b2f37df7 --- /dev/null +++ b/library/alloc/src/collections/btree/append.rs @@ -0,0 +1,124 @@ +use super::map::MIN_LEN; +use super::merge_iter::MergeIterInner; +use super::node::{self, ForceResult::*, Root}; +use core::iter::FusedIterator; + +impl Root { + /// Appends all key-value pairs from the union of two ascending iterators, + /// incrementing a `length` variable along the way. The latter makes it + /// easier for the caller to avoid a leak when a drop handler panicks. + /// + /// If both iterators produce the same key, this method drops the pair from + /// the left iterator and appends the pair from the right iterator. + /// + /// If you want the tree to end up in a strictly ascending order, like for + /// a `BTreeMap`, both iterators should produce keys in strictly ascending + /// order, each greater than all keys in the tree, including any keys + /// already in the tree upon entry. + pub fn append_from_sorted_iters(&mut self, left: I, right: I, length: &mut usize) + where + K: Ord, + I: Iterator + FusedIterator, + { + // We prepare to merge `left` and `right` into a sorted sequence in linear time. + let iter = MergeIter(MergeIterInner::new(left, right)); + + // Meanwhile, we build a tree from the sorted sequence in linear time. + self.bulk_push(iter, length) + } + + /// Pushes all key-value pairs to the end of the tree, incrementing a + /// `length` variable along the way. The latter makes it easier for the + /// caller to avoid a leak when the iterator panicks. + fn bulk_push(&mut self, iter: I, length: &mut usize) + where + I: Iterator, + { + let mut cur_node = self.node_as_mut().last_leaf_edge().into_node(); + // Iterate through all key-value pairs, pushing them into nodes at the right level. + for (key, value) in iter { + // Try to push key-value pair into the current leaf node. + if cur_node.len() < node::CAPACITY { + cur_node.push(key, value); + } else { + // No space left, go up and push there. + let mut open_node; + let mut test_node = cur_node.forget_type(); + loop { + match test_node.ascend() { + Ok(parent) => { + let parent = parent.into_node(); + if parent.len() < node::CAPACITY { + // Found a node with space left, push here. + open_node = parent; + break; + } else { + // Go up again. + test_node = parent.forget_type(); + } + } + Err(_) => { + // We are at the top, create a new root node and push there. + open_node = self.push_internal_level(); + break; + } + } + } + + // Push key-value pair and new right subtree. + let tree_height = open_node.height() - 1; + let mut right_tree = Root::new_leaf(); + for _ in 0..tree_height { + right_tree.push_internal_level(); + } + open_node.push(key, value, right_tree); + + // Go down to the right-most leaf again. + cur_node = open_node.forget_type().last_leaf_edge().into_node(); + } + + // Increment length every iteration, to make sure the map drops + // the appended elements even if advancing the iterator panicks. + *length += 1; + } + self.fix_right_edge(); + } + + fn fix_right_edge(&mut self) { + // Handle underfull nodes, start from the top. + let mut cur_node = self.node_as_mut(); + while let Internal(internal) = cur_node.force() { + // Check if right-most child is underfull. + let mut last_edge = internal.last_edge(); + let right_child_len = last_edge.reborrow().descend().len(); + if right_child_len < MIN_LEN { + // We need to steal. + let mut last_kv = match last_edge.left_kv() { + Ok(left) => left, + Err(_) => unreachable!(), + }; + last_kv.bulk_steal_left(MIN_LEN - right_child_len); + last_edge = last_kv.right_edge(); + } + + // Go further down. + cur_node = last_edge.descend(); + } + } +} + +// An iterator for merging two sorted sequences into one +struct MergeIter>(MergeIterInner); + +impl Iterator for MergeIter +where + I: Iterator + FusedIterator, +{ + type Item = (K, V); + + /// If two keys are equal, returns the key-value pair from the right source. + fn next(&mut self) -> Option<(K, V)> { + let (a_next, b_next) = self.0.nexts(|a: &(K, V), b: &(K, V)| K::cmp(&a.0, &b.0)); + b_next.or(a_next) + } +} diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index 07c23d29e20a6..49122f53d33ad 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -9,7 +9,6 @@ use core::ops::{Index, RangeBounds}; use core::ptr; use super::borrow::DormantMutRef; -use super::merge_iter::MergeIterInner; use super::node::{self, marker, ForceResult::*, Handle, NodeRef}; use super::search::{self, SearchResult::*}; use super::unwrap_unchecked; @@ -458,9 +457,6 @@ impl fmt::Debug for RangeMut<'_, K, V> { } } -// An iterator for merging two sorted sequences into one -struct MergeIter>(MergeIterInner); - impl BTreeMap { /// Makes a new empty BTreeMap. /// @@ -908,13 +904,10 @@ impl BTreeMap { return; } - // First, we merge `self` and `other` into a sorted sequence in linear time. let self_iter = mem::take(self).into_iter(); let other_iter = mem::take(other).into_iter(); - let iter = MergeIter(MergeIterInner::new(self_iter, other_iter)); - - // Second, we build a tree from the sorted sequence in linear time. - self.from_sorted_iter(iter); + let root = BTreeMap::ensure_is_owned(&mut self.root); + root.append_from_sorted_iters(self_iter, other_iter, &mut self.length) } /// Constructs a double-ended iterator over a sub-range of elements in the map. @@ -1039,78 +1032,6 @@ impl BTreeMap { } } - fn from_sorted_iter>(&mut self, iter: I) { - let root = Self::ensure_is_owned(&mut self.root); - let mut cur_node = root.node_as_mut().last_leaf_edge().into_node(); - // Iterate through all key-value pairs, pushing them into nodes at the right level. - for (key, value) in iter { - // Try to push key-value pair into the current leaf node. - if cur_node.len() < node::CAPACITY { - cur_node.push(key, value); - } else { - // No space left, go up and push there. - let mut open_node; - let mut test_node = cur_node.forget_type(); - loop { - match test_node.ascend() { - Ok(parent) => { - let parent = parent.into_node(); - if parent.len() < node::CAPACITY { - // Found a node with space left, push here. - open_node = parent; - break; - } else { - // Go up again. - test_node = parent.forget_type(); - } - } - Err(_) => { - // We are at the top, create a new root node and push there. - open_node = root.push_internal_level(); - break; - } - } - } - - // Push key-value pair and new right subtree. - let tree_height = open_node.height() - 1; - let mut right_tree = node::Root::new_leaf(); - for _ in 0..tree_height { - right_tree.push_internal_level(); - } - open_node.push(key, value, right_tree); - - // Go down to the right-most leaf again. - cur_node = open_node.forget_type().last_leaf_edge().into_node(); - } - - self.length += 1; - } - Self::fix_right_edge(root) - } - - fn fix_right_edge(root: &mut node::Root) { - // Handle underfull nodes, start from the top. - let mut cur_node = root.node_as_mut(); - while let Internal(internal) = cur_node.force() { - // Check if right-most child is underfull. - let mut last_edge = internal.last_edge(); - let right_child_len = last_edge.reborrow().descend().len(); - if right_child_len < MIN_LEN { - // We need to steal. - let mut last_kv = match last_edge.left_kv() { - Ok(left) => left, - Err(_) => unreachable!(), - }; - last_kv.bulk_steal_left(MIN_LEN - right_child_len); - last_edge = last_kv.right_edge(); - } - - // Go further down. - cur_node = last_edge.descend(); - } - } - /// Splits the collection into two at the given key. Returns everything after the given key, /// including the key. /// @@ -2220,18 +2141,5 @@ impl BTreeMap { } } -impl Iterator for MergeIter -where - I: Iterator + ExactSizeIterator + FusedIterator, -{ - type Item = (K, V); - - /// If two keys are equal, returns the key/value-pair from the right source. - fn next(&mut self) -> Option<(K, V)> { - let (a_next, b_next) = self.0.nexts(|a: &(K, V), b: &(K, V)| K::cmp(&a.0, &b.0)); - b_next.or(a_next) - } -} - #[cfg(test)] mod tests; diff --git a/library/alloc/src/collections/btree/map/tests.rs b/library/alloc/src/collections/btree/map/tests.rs index d5b1c600d93eb..89a6e327422bb 100644 --- a/library/alloc/src/collections/btree/map/tests.rs +++ b/library/alloc/src/collections/btree/map/tests.rs @@ -1667,6 +1667,33 @@ create_append_test!(test_append_239, 239); #[cfg(not(miri))] // Miri is too slow create_append_test!(test_append_1700, 1700); +#[test] +fn test_append_drop_leak() { + static DROPS: AtomicUsize = AtomicUsize::new(0); + + struct D; + + impl Drop for D { + fn drop(&mut self) { + if DROPS.fetch_add(1, Ordering::SeqCst) == 0 { + panic!("panic in `drop`"); + } + } + } + + let mut left = BTreeMap::new(); + let mut right = BTreeMap::new(); + left.insert(0, D); + left.insert(1, D); // first to be dropped during append + left.insert(2, D); + right.insert(1, D); + right.insert(2, D); + + catch_unwind(move || left.append(&mut right)).unwrap_err(); + + assert_eq!(DROPS.load(Ordering::SeqCst), 4); // Rust issue #47949 ate one little piggy +} + fn rand_data(len: usize) -> Vec<(u32, u32)> { assert!(len * 2 <= 70029); // from that point on numbers repeat let mut rng = DeterministicRng::new(); diff --git a/library/alloc/src/collections/btree/merge_iter.rs b/library/alloc/src/collections/btree/merge_iter.rs index 88e6f86c2c6f5..7f23d93b990f5 100644 --- a/library/alloc/src/collections/btree/merge_iter.rs +++ b/library/alloc/src/collections/btree/merge_iter.rs @@ -2,27 +2,25 @@ use core::cmp::Ordering; use core::fmt::{self, Debug}; use core::iter::FusedIterator; -/// Core of an iterator that merges the output of two ascending iterators, +/// Core of an iterator that merges the output of two strictly ascending iterators, /// for instance a union or a symmetric difference. -pub struct MergeIterInner -where - I: Iterator, -{ +pub struct MergeIterInner { a: I, b: I, peeked: Option>, } -/// Benchmarks faster than wrapping both iterators in a Peekable. +/// Benchmarks faster than wrapping both iterators in a Peekable, +/// probably because we can afford to impose a FusedIterator bound. #[derive(Clone, Debug)] enum Peeked { A(I::Item), B(I::Item), } -impl Clone for MergeIterInner +impl Clone for MergeIterInner where - I: Clone + Iterator, + I: Clone, I::Item: Clone, { fn clone(&self) -> Self { @@ -30,20 +28,17 @@ where } } -impl Debug for MergeIterInner +impl Debug for MergeIterInner where - I: Iterator + Debug, + I: Debug, I::Item: Debug, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_tuple("MergeIterInner").field(&self.a).field(&self.b).finish() + f.debug_tuple("MergeIterInner").field(&self.a).field(&self.b).field(&self.peeked).finish() } } -impl MergeIterInner -where - I: ExactSizeIterator + FusedIterator, -{ +impl MergeIterInner { /// Creates a new core for an iterator merging a pair of sources. pub fn new(a: I, b: I) -> Self { MergeIterInner { a, b, peeked: None } @@ -52,13 +47,17 @@ where /// Returns the next pair of items stemming from the pair of sources /// being merged. If both returned options contain a value, that value /// is equal and occurs in both sources. If one of the returned options - /// contains a value, that value doesn't occur in the other source. - /// If neither returned option contains a value, iteration has finished - /// and subsequent calls will return the same empty pair. + /// contains a value, that value doesn't occur in the other source (or + /// the sources are not strictly ascending). If neither returned option + /// contains a value, iteration has finished and subsequent calls will + /// return the same empty pair. pub fn nexts Ordering>( &mut self, cmp: Cmp, - ) -> (Option, Option) { + ) -> (Option, Option) + where + I: FusedIterator, + { let mut a_next; let mut b_next; match self.peeked.take() { @@ -86,7 +85,10 @@ where } /// Returns a pair of upper bounds for the `size_hint` of the final iterator. - pub fn lens(&self) -> (usize, usize) { + pub fn lens(&self) -> (usize, usize) + where + I: ExactSizeIterator, + { match self.peeked { Some(Peeked::A(_)) => (1 + self.a.len(), self.b.len()), Some(Peeked::B(_)) => (self.a.len(), 1 + self.b.len()), diff --git a/library/alloc/src/collections/btree/mod.rs b/library/alloc/src/collections/btree/mod.rs index 7bf1706dd6d57..ebcbb0e467c46 100644 --- a/library/alloc/src/collections/btree/mod.rs +++ b/library/alloc/src/collections/btree/mod.rs @@ -1,3 +1,4 @@ +mod append; mod borrow; pub mod map; mod mem; From ffa70d75c8f5925ea26697c7ca5d20fd4d85cbb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Mon, 9 Nov 2020 00:00:00 +0000 Subject: [PATCH 34/42] Support inlining diverging function calls Additionally introduce storage markers for all temporaries created by the inliner. The temporary introduced for destination rebrorrow, didn't use them previously. --- compiler/rustc_mir/src/transform/inline.rs | 152 +++++++++--------- src/test/mir-opt/inline/inline-diverging.rs | 40 +++++ .../inline/inline_diverging.f.Inline.diff | 26 +++ .../inline/inline_diverging.g.Inline.diff | 52 ++++++ .../inline/inline_diverging.h.Inline.diff | 58 +++++++ ...line_into_box_place.main.Inline.32bit.diff | 2 + ...line_into_box_place.main.Inline.64bit.diff | 2 + 7 files changed, 256 insertions(+), 76 deletions(-) create mode 100644 src/test/mir-opt/inline/inline-diverging.rs create mode 100644 src/test/mir-opt/inline/inline_diverging.f.Inline.diff create mode 100644 src/test/mir-opt/inline/inline_diverging.g.Inline.diff create mode 100644 src/test/mir-opt/inline/inline_diverging.h.Inline.diff diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs index 97b513445264a..f275920cf7b17 100644 --- a/compiler/rustc_mir/src/transform/inline.rs +++ b/compiler/rustc_mir/src/transform/inline.rs @@ -31,7 +31,8 @@ pub struct Inline; #[derive(Copy, Clone, Debug)] struct CallSite<'tcx> { callee: Instance<'tcx>, - bb: BasicBlock, + block: BasicBlock, + target: Option, source_info: SourceInfo, } @@ -175,8 +176,7 @@ impl Inliner<'tcx> { // Only consider direct calls to functions let terminator = bb_data.terminator(); - // FIXME: Handle inlining of diverging calls - if let TerminatorKind::Call { func: ref op, destination: Some(_), .. } = terminator.kind { + if let TerminatorKind::Call { func: ref op, ref destination, .. } = terminator.kind { if let ty::FnDef(callee_def_id, substs) = *op.ty(caller_body, self.tcx).kind() { // To resolve an instance its substs have to be fully normalized, so // we do this here. @@ -190,7 +190,12 @@ impl Inliner<'tcx> { return None; } - return Some(CallSite { callee, bb, source_info: terminator.source_info }); + return Some(CallSite { + callee, + block: bb, + target: destination.map(|(_, target)| target), + source_info: terminator.source_info, + }); } } @@ -398,9 +403,9 @@ impl Inliner<'tcx> { caller_body: &mut Body<'tcx>, mut callee_body: Body<'tcx>, ) { - let terminator = caller_body[callsite.bb].terminator.take().unwrap(); + let terminator = caller_body[callsite.block].terminator.take().unwrap(); match terminator.kind { - TerminatorKind::Call { args, destination: Some(destination), cleanup, .. } => { + TerminatorKind::Call { args, destination, cleanup, .. } => { // If the call is something like `a[*i] = f(i)`, where // `i : &mut usize`, then just duplicating the `a[*i]` // Place could result in two different locations if `f` @@ -417,35 +422,31 @@ impl Inliner<'tcx> { false } - let dest = if dest_needs_borrow(destination.0) { - trace!("creating temp for return destination"); - let dest = Rvalue::Ref( - self.tcx.lifetimes.re_erased, - BorrowKind::Mut { allow_two_phase_borrow: false }, - destination.0, - ); - - let ty = dest.ty(caller_body, self.tcx); - - let temp = LocalDecl::new(ty, callsite.source_info.span); - - let tmp = caller_body.local_decls.push(temp); - let tmp = Place::from(tmp); - - let stmt = Statement { - source_info: callsite.source_info, - kind: StatementKind::Assign(box (tmp, dest)), - }; - caller_body[callsite.bb].statements.push(stmt); - self.tcx.mk_place_deref(tmp) + let dest = if let Some((destination_place, _)) = destination { + if dest_needs_borrow(destination_place) { + trace!("creating temp for return destination"); + let dest = Rvalue::Ref( + self.tcx.lifetimes.re_erased, + BorrowKind::Mut { allow_two_phase_borrow: false }, + destination_place, + ); + let dest_ty = dest.ty(caller_body, self.tcx); + let temp = Place::from(self.new_call_temp(caller_body, &callsite, dest_ty)); + caller_body[callsite.block].statements.push(Statement { + source_info: callsite.source_info, + kind: StatementKind::Assign(box (temp, dest)), + }); + self.tcx.mk_place_deref(temp) + } else { + destination_place + } } else { - destination.0 + trace!("creating temp for return place"); + Place::from(self.new_call_temp(caller_body, &callsite, callee_body.return_ty())) }; - let return_block = destination.1; - // Copy the arguments if needed. - let args: Vec<_> = self.make_call_args(args, &callsite, caller_body, return_block); + let args: Vec<_> = self.make_call_args(args, &callsite, caller_body); let mut integrator = Integrator { args: &args, @@ -453,7 +454,7 @@ impl Inliner<'tcx> { new_scopes: SourceScope::new(caller_body.source_scopes.len()).., new_blocks: BasicBlock::new(caller_body.basic_blocks().len()).., destination: dest, - return_block, + return_block: callsite.target, cleanup_block: cleanup, in_cleanup_block: false, tcx: self.tcx, @@ -502,7 +503,7 @@ impl Inliner<'tcx> { caller_body.var_debug_info.extend(callee_body.var_debug_info.drain(..)); caller_body.basic_blocks_mut().extend(callee_body.basic_blocks_mut().drain(..)); - caller_body[callsite.bb].terminator = Some(Terminator { + caller_body[callsite.block].terminator = Some(Terminator { source_info: callsite.source_info, kind: TerminatorKind::Goto { target: integrator.map_block(START_BLOCK) }, }); @@ -526,7 +527,6 @@ impl Inliner<'tcx> { args: Vec>, callsite: &CallSite<'tcx>, caller_body: &mut Body<'tcx>, - return_block: BasicBlock, ) -> Vec { let tcx = self.tcx; @@ -557,18 +557,8 @@ impl Inliner<'tcx> { // `callee_body.spread_arg == None`, instead of special-casing closures. if tcx.is_closure(callsite.callee.def_id()) { let mut args = args.into_iter(); - let self_ = self.create_temp_if_necessary( - args.next().unwrap(), - callsite, - caller_body, - return_block, - ); - let tuple = self.create_temp_if_necessary( - args.next().unwrap(), - callsite, - caller_body, - return_block, - ); + let self_ = self.create_temp_if_necessary(args.next().unwrap(), callsite, caller_body); + let tuple = self.create_temp_if_necessary(args.next().unwrap(), callsite, caller_body); assert!(args.next().is_none()); let tuple = Place::from(tuple); @@ -588,13 +578,13 @@ impl Inliner<'tcx> { Operand::Move(tcx.mk_place_field(tuple, Field::new(i), ty.expect_ty())); // Spill to a local to make e.g., `tmp0`. - self.create_temp_if_necessary(tuple_field, callsite, caller_body, return_block) + self.create_temp_if_necessary(tuple_field, callsite, caller_body) }); closure_ref_arg.chain(tuple_tmp_args).collect() } else { args.into_iter() - .map(|a| self.create_temp_if_necessary(a, callsite, caller_body, return_block)) + .map(|a| self.create_temp_if_necessary(a, callsite, caller_body)) .collect() } } @@ -606,46 +596,52 @@ impl Inliner<'tcx> { arg: Operand<'tcx>, callsite: &CallSite<'tcx>, caller_body: &mut Body<'tcx>, - return_block: BasicBlock, ) -> Local { - // FIXME: Analysis of the usage of the arguments to avoid - // unnecessary temporaries. - + // Reuse the operand if it is a moved temporary. if let Operand::Move(place) = &arg { if let Some(local) = place.as_local() { if caller_body.local_kind(local) == LocalKind::Temp { - // Reuse the operand if it's a temporary already return local; } } } + // Otherwise, create a temporary for the argument. trace!("creating temp for argument {:?}", arg); - // Otherwise, create a temporary for the arg - let arg = Rvalue::Use(arg); - - let ty = arg.ty(caller_body, self.tcx); - - let arg_tmp = LocalDecl::new(ty, callsite.source_info.span); - let arg_tmp = caller_body.local_decls.push(arg_tmp); - - caller_body[callsite.bb].statements.push(Statement { + let arg_ty = arg.ty(caller_body, self.tcx); + let local = self.new_call_temp(caller_body, callsite, arg_ty); + caller_body[callsite.block].statements.push(Statement { source_info: callsite.source_info, - kind: StatementKind::StorageLive(arg_tmp), + kind: StatementKind::Assign(box (Place::from(local), Rvalue::Use(arg))), }); - caller_body[callsite.bb].statements.push(Statement { + local + } + + /// Introduces a new temporary into the caller body that is live for the duration of the call. + fn new_call_temp( + &self, + caller_body: &mut Body<'tcx>, + callsite: &CallSite<'tcx>, + ty: Ty<'tcx>, + ) -> Local { + let local = caller_body.local_decls.push(LocalDecl::new(ty, callsite.source_info.span)); + + caller_body[callsite.block].statements.push(Statement { source_info: callsite.source_info, - kind: StatementKind::Assign(box (Place::from(arg_tmp), arg)), + kind: StatementKind::StorageLive(local), }); - caller_body[return_block].statements.insert( - 0, - Statement { - source_info: callsite.source_info, - kind: StatementKind::StorageDead(arg_tmp), - }, - ); - - arg_tmp + + if let Some(block) = callsite.target { + caller_body[block].statements.insert( + 0, + Statement { + source_info: callsite.source_info, + kind: StatementKind::StorageDead(local), + }, + ); + } + + local } } @@ -670,7 +666,7 @@ struct Integrator<'a, 'tcx> { new_scopes: RangeFrom, new_blocks: RangeFrom, destination: Place<'tcx>, - return_block: BasicBlock, + return_block: Option, cleanup_block: Option, in_cleanup_block: bool, tcx: TyCtxt<'tcx>, @@ -810,7 +806,11 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Integrator<'a, 'tcx> { } } TerminatorKind::Return => { - terminator.kind = TerminatorKind::Goto { target: self.return_block }; + terminator.kind = if let Some(tgt) = self.return_block { + TerminatorKind::Goto { target: tgt } + } else { + TerminatorKind::Unreachable + } } TerminatorKind::Resume => { if let Some(tgt) = self.cleanup_block { diff --git a/src/test/mir-opt/inline/inline-diverging.rs b/src/test/mir-opt/inline/inline-diverging.rs new file mode 100644 index 0000000000000..ae6f814c290c8 --- /dev/null +++ b/src/test/mir-opt/inline/inline-diverging.rs @@ -0,0 +1,40 @@ +// Tests inlining of diverging calls. +// +// ignore-wasm32-bare compiled with panic=abort by default +#![crate_type = "lib"] + +// EMIT_MIR inline_diverging.f.Inline.diff +pub fn f() { + sleep(); +} + +// EMIT_MIR inline_diverging.g.Inline.diff +pub fn g(i: i32) -> u32 { + if i > 0 { + i as u32 + } else { + panic(); + } +} + +// EMIT_MIR inline_diverging.h.Inline.diff +pub fn h() { + call_twice(sleep); +} + +#[inline(always)] +pub fn call_twice R>(f: F) -> (R, R) { + let a = f(); + let b = f(); + (a, b) +} + +#[inline(always)] +fn panic() -> ! { + panic!(); +} + +#[inline(always)] +fn sleep() -> ! { + loop {} +} diff --git a/src/test/mir-opt/inline/inline_diverging.f.Inline.diff b/src/test/mir-opt/inline/inline_diverging.f.Inline.diff new file mode 100644 index 0000000000000..6e36dc06a201e --- /dev/null +++ b/src/test/mir-opt/inline/inline_diverging.f.Inline.diff @@ -0,0 +1,26 @@ +- // MIR for `f` before Inline ++ // MIR for `f` after Inline + + fn f() -> () { + let mut _0: (); // return place in scope 0 at $DIR/inline-diverging.rs:7:12: 7:12 + let mut _1: !; // in scope 0 at $DIR/inline-diverging.rs:7:12: 9:2 + let _2: !; // in scope 0 at $DIR/inline-diverging.rs:8:5: 8:12 ++ let mut _3: !; // in scope 0 at $DIR/inline-diverging.rs:8:5: 8:12 ++ scope 1 (inlined sleep) { // at $DIR/inline-diverging.rs:8:5: 8:12 ++ } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/inline-diverging.rs:8:5: 8:12 +- sleep(); // scope 0 at $DIR/inline-diverging.rs:8:5: 8:12 +- // mir::Constant +- // + span: $DIR/inline-diverging.rs:8:5: 8:10 +- // + literal: Const { ty: fn() -> ! {sleep}, val: Value(Scalar()) } ++ StorageLive(_3); // scope 0 at $DIR/inline-diverging.rs:8:5: 8:12 ++ goto -> bb1; // scope 0 at $DIR/inline-diverging.rs:8:5: 8:12 ++ } ++ ++ bb1: { ++ goto -> bb1; // scope 1 at $DIR/inline-diverging.rs:8:5: 8:12 + } + } + diff --git a/src/test/mir-opt/inline/inline_diverging.g.Inline.diff b/src/test/mir-opt/inline/inline_diverging.g.Inline.diff new file mode 100644 index 0000000000000..3dc33354a5a56 --- /dev/null +++ b/src/test/mir-opt/inline/inline_diverging.g.Inline.diff @@ -0,0 +1,52 @@ +- // MIR for `g` before Inline ++ // MIR for `g` after Inline + + fn g(_1: i32) -> u32 { + debug i => _1; // in scope 0 at $DIR/inline-diverging.rs:12:10: 12:11 + let mut _0: u32; // return place in scope 0 at $DIR/inline-diverging.rs:12:21: 12:24 + let mut _2: bool; // in scope 0 at $DIR/inline-diverging.rs:13:8: 13:13 + let mut _3: i32; // in scope 0 at $DIR/inline-diverging.rs:13:8: 13:9 + let mut _4: i32; // in scope 0 at $DIR/inline-diverging.rs:14:9: 14:10 + let mut _5: !; // in scope 0 at $DIR/inline-diverging.rs:15:12: 17:6 + let _6: !; // in scope 0 at $DIR/inline-diverging.rs:16:9: 16:16 ++ let mut _7: !; // in scope 0 at $DIR/inline-diverging.rs:16:9: 16:16 ++ scope 1 (inlined panic) { // at $DIR/inline-diverging.rs:16:9: 16:16 ++ } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/inline-diverging.rs:13:8: 13:13 + StorageLive(_3); // scope 0 at $DIR/inline-diverging.rs:13:8: 13:9 + _3 = _1; // scope 0 at $DIR/inline-diverging.rs:13:8: 13:9 + _2 = Gt(move _3, const 0_i32); // scope 0 at $DIR/inline-diverging.rs:13:8: 13:13 + StorageDead(_3); // scope 0 at $DIR/inline-diverging.rs:13:12: 13:13 + switchInt(_2) -> [false: bb1, otherwise: bb2]; // scope 0 at $DIR/inline-diverging.rs:13:5: 17:6 + } + + bb1: { + StorageLive(_6); // scope 0 at $DIR/inline-diverging.rs:16:9: 16:16 +- panic(); // scope 0 at $DIR/inline-diverging.rs:16:9: 16:16 ++ StorageLive(_7); // scope 0 at $DIR/inline-diverging.rs:16:9: 16:16 ++ begin_panic::<&str>(const "explicit panic"); // scope 1 at $DIR/inline-diverging.rs:16:9: 16:16 + // mir::Constant +- // + span: $DIR/inline-diverging.rs:16:9: 16:14 +- // + literal: Const { ty: fn() -> ! {panic}, val: Value(Scalar()) } ++ // + span: $DIR/inline-diverging.rs:16:9: 16:16 ++ // + literal: Const { ty: fn(&str) -> ! {std::rt::begin_panic::<&str>}, val: Value(Scalar()) } ++ // ty::Const ++ // + ty: &str ++ // + val: Value(Slice { data: Allocation { bytes: [101, 120, 112, 108, 105, 99, 105, 116, 32, 112, 97, 110, 105, 99], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [16383], len: Size { raw: 14 } }, size: Size { raw: 14 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 14 }) ++ // mir::Constant ++ // + span: $DIR/inline-diverging.rs:16:9: 16:16 ++ // + literal: Const { ty: &str, val: Value(Slice { data: Allocation { bytes: [101, 120, 112, 108, 105, 99, 105, 116, 32, 112, 97, 110, 105, 99], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [16383], len: Size { raw: 14 } }, size: Size { raw: 14 }, align: Align { pow2: 0 }, mutability: Not, extra: () }, start: 0, end: 14 }) } + } + + bb2: { + StorageLive(_4); // scope 0 at $DIR/inline-diverging.rs:14:9: 14:10 + _4 = _1; // scope 0 at $DIR/inline-diverging.rs:14:9: 14:10 + _0 = move _4 as u32 (Misc); // scope 0 at $DIR/inline-diverging.rs:14:9: 14:17 + StorageDead(_4); // scope 0 at $DIR/inline-diverging.rs:14:16: 14:17 + StorageDead(_2); // scope 0 at $DIR/inline-diverging.rs:18:1: 18:2 + return; // scope 0 at $DIR/inline-diverging.rs:18:2: 18:2 + } + } + diff --git a/src/test/mir-opt/inline/inline_diverging.h.Inline.diff b/src/test/mir-opt/inline/inline_diverging.h.Inline.diff new file mode 100644 index 0000000000000..b728ad4b42899 --- /dev/null +++ b/src/test/mir-opt/inline/inline_diverging.h.Inline.diff @@ -0,0 +1,58 @@ +- // MIR for `h` before Inline ++ // MIR for `h` after Inline + + fn h() -> () { + let mut _0: (); // return place in scope 0 at $DIR/inline-diverging.rs:21:12: 21:12 + let _1: (!, !); // in scope 0 at $DIR/inline-diverging.rs:22:5: 22:22 ++ let mut _2: fn() -> ! {sleep}; // in scope 0 at $DIR/inline-diverging.rs:22:5: 22:22 ++ let mut _7: (); // in scope 0 at $DIR/inline-diverging.rs:22:5: 22:22 ++ let mut _8: (); // in scope 0 at $DIR/inline-diverging.rs:22:5: 22:22 ++ scope 1 (inlined call_twice:: ! {sleep}>) { // at $DIR/inline-diverging.rs:22:5: 22:22 ++ debug f => _2; // in scope 1 at $DIR/inline-diverging.rs:22:5: 22:22 ++ let _3: !; // in scope 1 at $DIR/inline-diverging.rs:22:5: 22:22 ++ let mut _4: &fn() -> ! {sleep}; // in scope 1 at $DIR/inline-diverging.rs:22:5: 22:22 ++ let mut _5: &fn() -> ! {sleep}; // in scope 1 at $DIR/inline-diverging.rs:22:5: 22:22 ++ let mut _6: !; // in scope 1 at $DIR/inline-diverging.rs:22:5: 22:22 ++ scope 2 { ++ debug a => _3; // in scope 2 at $DIR/inline-diverging.rs:22:5: 22:22 ++ scope 3 { ++ debug b => _6; // in scope 3 at $DIR/inline-diverging.rs:22:5: 22:22 ++ } ++ scope 6 (inlined ! {sleep} as Fn<()>>::call - shim(fn() -> ! {sleep})) { // at $DIR/inline-diverging.rs:22:5: 22:22 ++ scope 7 (inlined sleep) { // at $DIR/inline-diverging.rs:22:5: 22:22 ++ } ++ } ++ } ++ scope 4 (inlined ! {sleep} as Fn<()>>::call - shim(fn() -> ! {sleep})) { // at $DIR/inline-diverging.rs:22:5: 22:22 ++ scope 5 (inlined sleep) { // at $DIR/inline-diverging.rs:22:5: 22:22 ++ } ++ } ++ } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/inline-diverging.rs:22:5: 22:22 +- _1 = call_twice:: ! {sleep}>(sleep) -> bb1; // scope 0 at $DIR/inline-diverging.rs:22:5: 22:22 ++ StorageLive(_2); // scope 0 at $DIR/inline-diverging.rs:22:5: 22:22 ++ _2 = sleep; // scope 0 at $DIR/inline-diverging.rs:22:5: 22:22 + // mir::Constant +- // + span: $DIR/inline-diverging.rs:22:5: 22:15 +- // + literal: Const { ty: fn(fn() -> ! {sleep}) -> (!, !) {call_twice:: ! {sleep}>}, val: Value(Scalar()) } +- // mir::Constant + // + span: $DIR/inline-diverging.rs:22:16: 22:21 + // + literal: Const { ty: fn() -> ! {sleep}, val: Value(Scalar()) } ++ StorageLive(_3); // scope 1 at $DIR/inline-diverging.rs:22:5: 22:22 ++ StorageLive(_4); // scope 1 at $DIR/inline-diverging.rs:22:5: 22:22 ++ _4 = &_2; // scope 1 at $DIR/inline-diverging.rs:22:5: 22:22 ++ StorageLive(_7); // scope 1 at $DIR/inline-diverging.rs:22:5: 22:22 ++ _7 = const (); // scope 1 at $DIR/inline-diverging.rs:22:5: 22:22 ++ goto -> bb1; // scope 5 at $DIR/inline-diverging.rs:22:5: 22:22 + } + + bb1: { +- StorageDead(_1); // scope 0 at $DIR/inline-diverging.rs:22:22: 22:23 +- _0 = const (); // scope 0 at $DIR/inline-diverging.rs:21:12: 23:2 +- return; // scope 0 at $DIR/inline-diverging.rs:23:2: 23:2 ++ goto -> bb1; // scope 5 at $DIR/inline-diverging.rs:22:5: 22:22 + } + } + diff --git a/src/test/mir-opt/inline/inline_into_box_place.main.Inline.32bit.diff b/src/test/mir-opt/inline/inline_into_box_place.main.Inline.32bit.diff index 0fbafd76e209b..3c0dfb4a77e16 100644 --- a/src/test/mir-opt/inline/inline_into_box_place.main.Inline.32bit.diff +++ b/src/test/mir-opt/inline/inline_into_box_place.main.Inline.32bit.diff @@ -18,6 +18,7 @@ StorageLive(_2); // scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43 _2 = Box(std::vec::Vec); // scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43 - (*_2) = Vec::::new() -> [return: bb1, unwind: bb4]; // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43 ++ StorageLive(_4); // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43 + _4 = &mut (*_2); // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43 + ((*_4).0: alloc::raw_vec::RawVec) = const alloc::raw_vec::RawVec:: { ptr: Unique:: { pointer: {0x4 as *const u32}, _marker: PhantomData:: }, cap: 0_usize, alloc: std::alloc::Global }; // scope 2 at $DIR/inline-into-box-place.rs:8:33: 8:43 + // ty::Const @@ -34,6 +35,7 @@ + // + user_ty: UserType(0) + // + literal: Const { ty: alloc::raw_vec::RawVec, val: Value(ByRef { alloc: Allocation { bytes: [4, 0, 0, 0, 0, 0, 0, 0], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [255], len: Size { raw: 8 } }, size: Size { raw: 8 }, align: Align { pow2: 2 }, mutability: Not, extra: () }, offset: Size { raw: 0 } }) } + ((*_4).1: usize) = const 0_usize; // scope 2 at $DIR/inline-into-box-place.rs:8:33: 8:43 ++ StorageDead(_4); // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43 _1 = move _2; // scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43 StorageDead(_2); // scope 0 at $DIR/inline-into-box-place.rs:8:42: 8:43 _0 = const (); // scope 0 at $DIR/inline-into-box-place.rs:7:11: 9:2 diff --git a/src/test/mir-opt/inline/inline_into_box_place.main.Inline.64bit.diff b/src/test/mir-opt/inline/inline_into_box_place.main.Inline.64bit.diff index 9277e57134eca..a72db9cf1dc8f 100644 --- a/src/test/mir-opt/inline/inline_into_box_place.main.Inline.64bit.diff +++ b/src/test/mir-opt/inline/inline_into_box_place.main.Inline.64bit.diff @@ -18,6 +18,7 @@ StorageLive(_2); // scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43 _2 = Box(std::vec::Vec); // scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43 - (*_2) = Vec::::new() -> [return: bb1, unwind: bb4]; // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43 ++ StorageLive(_4); // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43 + _4 = &mut (*_2); // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43 + ((*_4).0: alloc::raw_vec::RawVec) = const alloc::raw_vec::RawVec:: { ptr: Unique:: { pointer: {0x4 as *const u32}, _marker: PhantomData:: }, cap: 0_usize, alloc: std::alloc::Global }; // scope 2 at $DIR/inline-into-box-place.rs:8:33: 8:43 + // ty::Const @@ -34,6 +35,7 @@ + // + user_ty: UserType(0) + // + literal: Const { ty: alloc::raw_vec::RawVec, val: Value(ByRef { alloc: Allocation { bytes: [4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], relocations: Relocations(SortedMap { data: [] }), init_mask: InitMask { blocks: [65535], len: Size { raw: 16 } }, size: Size { raw: 16 }, align: Align { pow2: 3 }, mutability: Not, extra: () }, offset: Size { raw: 0 } }) } + ((*_4).1: usize) = const 0_usize; // scope 2 at $DIR/inline-into-box-place.rs:8:33: 8:43 ++ StorageDead(_4); // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43 _1 = move _2; // scope 0 at $DIR/inline-into-box-place.rs:8:29: 8:43 StorageDead(_2); // scope 0 at $DIR/inline-into-box-place.rs:8:42: 8:43 _0 = const (); // scope 0 at $DIR/inline-into-box-place.rs:7:11: 9:2 From 26f0f126d1228a32f7e8af3ac681c4521c6ac67a Mon Sep 17 00:00:00 2001 From: Raoul Strackx Date: Mon, 9 Nov 2020 09:20:49 -0800 Subject: [PATCH 35/42] Upgrading dlmalloc to 0.2.1 --- Cargo.lock | 4 +-- library/std/Cargo.toml | 2 +- library/std/src/sys/sgx/abi/mem.rs | 12 ++++++++ library/std/src/sys/sgx/alloc.rs | 46 +++++++++++++++++++++++++++++- 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9c48ff8676977..8358e0cac3e39 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -952,9 +952,9 @@ dependencies = [ [[package]] name = "dlmalloc" -version = "0.1.4" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "35055b1021724f4eb5262eb49130eebff23fc59fc5a14160e05faad8eeb36673" +checksum = "332570860c2edf2d57914987bf9e24835425f75825086b6ba7d1e6a3e4f1f254" dependencies = [ "compiler_builtins", "libc", diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index c08828bc0cde9..1baaaa602a630 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -36,7 +36,7 @@ features = ['read_core', 'elf', 'macho', 'pe'] rand = "0.7" [target.'cfg(any(all(target_arch = "wasm32", not(target_os = "emscripten")), all(target_vendor = "fortanix", target_env = "sgx")))'.dependencies] -dlmalloc = { version = "0.1", features = ['rustc-dep-of-std'] } +dlmalloc = { version = "0.2.1", features = ['rustc-dep-of-std'] } [target.x86_64-fortanix-unknown-sgx.dependencies] fortanix-sgx-abi = { version = "0.3.2", features = ['rustc-dep-of-std'] } diff --git a/library/std/src/sys/sgx/abi/mem.rs b/library/std/src/sys/sgx/abi/mem.rs index ffa234fccfea7..da899773dbb0d 100644 --- a/library/std/src/sys/sgx/abi/mem.rs +++ b/library/std/src/sys/sgx/abi/mem.rs @@ -12,6 +12,18 @@ pub(crate) unsafe fn rel_ptr_mut(offset: u64) -> *mut T { extern "C" { static ENCLAVE_SIZE: usize; + static HEAP_BASE: u64; + static HEAP_SIZE: usize; +} + +/// Returns the base memory address of the heap +pub(crate) fn heap_base() -> *const u8 { + unsafe { rel_ptr_mut(HEAP_BASE) } +} + +/// Returns the size of the heap +pub(crate) fn heap_size() -> usize { + unsafe { HEAP_SIZE } } // Do not remove inline: will result in relocation failure diff --git a/library/std/src/sys/sgx/alloc.rs b/library/std/src/sys/sgx/alloc.rs index 4559ea7cd2540..4aea28cb83e23 100644 --- a/library/std/src/sys/sgx/alloc.rs +++ b/library/std/src/sys/sgx/alloc.rs @@ -1,4 +1,7 @@ use crate::alloc::{GlobalAlloc, Layout, System}; +use crate::ptr; +use crate::sys::sgx::abi::mem as sgx_mem; +use core::sync::atomic::{AtomicBool, Ordering}; use super::waitqueue::SpinMutex; @@ -10,7 +13,48 @@ use super::waitqueue::SpinMutex; // dlmalloc.c from C to Rust. #[cfg_attr(test, linkage = "available_externally")] #[export_name = "_ZN16__rust_internals3std3sys3sgx5alloc8DLMALLOCE"] -static DLMALLOC: SpinMutex = SpinMutex::new(dlmalloc::DLMALLOC_INIT); +static DLMALLOC: SpinMutex> = + SpinMutex::new(dlmalloc::Dlmalloc::new_with_allocator(Sgx {})); + +struct Sgx; + +unsafe impl dlmalloc::Allocator for Sgx { + /// Allocs system resources + fn alloc(&self, _size: usize) -> (*mut u8, usize, u32) { + static INIT: AtomicBool = AtomicBool::new(false); + + // No ordering requirement since this function is protected by the global lock. + if !INIT.swap(true, Ordering::Relaxed) { + (sgx_mem::heap_base() as _, sgx_mem::heap_size(), 0) + } else { + (ptr::null_mut(), 0, 0) + } + } + + fn remap(&self, _ptr: *mut u8, _oldsize: usize, _newsize: usize, _can_move: bool) -> *mut u8 { + ptr::null_mut() + } + + fn free_part(&self, _ptr: *mut u8, _oldsize: usize, _newsize: usize) -> bool { + false + } + + fn free(&self, _ptr: *mut u8, _size: usize) -> bool { + return false; + } + + fn can_release_part(&self, _flags: u32) -> bool { + false + } + + fn allocates_zeros(&self) -> bool { + false + } + + fn page_size(&self) -> usize { + 0x1000 + } +} #[stable(feature = "alloc_system_type", since = "1.28.0")] unsafe impl GlobalAlloc for System { From c8943c62f7a139b466648249045d557259cf22fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Tue, 10 Nov 2020 00:00:00 +0000 Subject: [PATCH 36/42] Add flags customizing behaviour of MIR inlining * `-Zinline-mir-threshold` to change the default threshold. * `-Zinline-mir-hint-threshold` to change the threshold used by functions with inline hint. --- compiler/rustc_interface/src/tests.rs | 2 + compiler/rustc_mir/src/transform/inline.rs | 9 +-- compiler/rustc_session/src/options.rs | 4 ++ src/test/mir-opt/inline/inline-options.rs | 19 +++++++ .../inline_options.main.Inline.after.mir | 56 +++++++++++++++++++ 5 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 src/test/mir-opt/inline/inline-options.rs create mode 100644 src/test/mir-opt/inline/inline_options.main.Inline.after.mir diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index dfc6f69145753..1fc2d281e7935 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -554,6 +554,8 @@ fn test_debugging_options_tracking_hash() { tracked!(function_sections, Some(false)); tracked!(human_readable_cgu_names, true); tracked!(inline_in_all_cgus, Some(true)); + tracked!(inline_mir_threshold, 123); + tracked!(inline_mir_hint_threshold, 123); tracked!(insert_sideeffect, true); tracked!(instrument_coverage, true); tracked!(instrument_mcount, true); diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs index 97b513445264a..c742821e9bf0a 100644 --- a/compiler/rustc_mir/src/transform/inline.rs +++ b/compiler/rustc_mir/src/transform/inline.rs @@ -16,9 +16,6 @@ use crate::transform::MirPass; use std::iter; use std::ops::{Range, RangeFrom}; -const DEFAULT_THRESHOLD: usize = 50; -const HINT_THRESHOLD: usize = 100; - const INSTR_COST: usize = 5; const CALL_PENALTY: usize = 25; const LANDINGPAD_PENALTY: usize = 50; @@ -248,7 +245,11 @@ impl Inliner<'tcx> { } } - let mut threshold = if hinted { HINT_THRESHOLD } else { DEFAULT_THRESHOLD }; + let mut threshold = if hinted { + self.tcx.sess.opts.debugging_opts.inline_mir_hint_threshold + } else { + self.tcx.sess.opts.debugging_opts.inline_mir_threshold + }; // Significantly lower the threshold for inlining cold functions if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::COLD) { diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index ceed730e25b08..1cd3d11e32153 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -929,6 +929,10 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, (default: no)"), incremental_verify_ich: bool = (false, parse_bool, [UNTRACKED], "verify incr. comp. hashes of green query instances (default: no)"), + inline_mir_threshold: usize = (50, parse_uint, [TRACKED], + "a default MIR inlining threshold (default: 50)"), + inline_mir_hint_threshold: usize = (100, parse_uint, [TRACKED], + "inlining threshold for functions with inline hint (default: 100)"), inline_in_all_cgus: Option = (None, parse_opt_bool, [TRACKED], "control whether `#[inline]` functions are in all CGUs"), input_stats: bool = (false, parse_bool, [UNTRACKED], diff --git a/src/test/mir-opt/inline/inline-options.rs b/src/test/mir-opt/inline/inline-options.rs new file mode 100644 index 0000000000000..477f050b69e42 --- /dev/null +++ b/src/test/mir-opt/inline/inline-options.rs @@ -0,0 +1,19 @@ +// Checks that inlining threshold can be controlled with +// inline-mir-threshold and inline-hint-threshold options. +// +// compile-flags: -Zinline-mir-threshold=90 +// compile-flags: -Zinline-mir-hint-threshold=50 + +// EMIT_MIR inline_options.main.Inline.after.mir +fn main() { + not_inlined(); + inlined::(); +} + +// Cost is approximately 3 * 25 + 5 = 80. +#[inline] +pub fn not_inlined() { g(); g(); g(); } +pub fn inlined() { g(); g(); g(); } + +#[inline(never)] +fn g() {} diff --git a/src/test/mir-opt/inline/inline_options.main.Inline.after.mir b/src/test/mir-opt/inline/inline_options.main.Inline.after.mir new file mode 100644 index 0000000000000..4cbdde2ba07d7 --- /dev/null +++ b/src/test/mir-opt/inline/inline_options.main.Inline.after.mir @@ -0,0 +1,56 @@ +// MIR for `main` after Inline + +fn main() -> () { + let mut _0: (); // return place in scope 0 at $DIR/inline-options.rs:8:11: 8:11 + let _1: (); // in scope 0 at $DIR/inline-options.rs:9:5: 9:18 + let _2: (); // in scope 0 at $DIR/inline-options.rs:10:5: 10:21 + scope 1 (inlined inlined::) { // at $DIR/inline-options.rs:10:5: 10:21 + let _3: (); // in scope 1 at $DIR/inline-options.rs:10:5: 10:21 + let _4: (); // in scope 1 at $DIR/inline-options.rs:10:5: 10:21 + let _5: (); // in scope 1 at $DIR/inline-options.rs:10:5: 10:21 + } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/inline-options.rs:9:5: 9:18 + _1 = not_inlined() -> bb1; // scope 0 at $DIR/inline-options.rs:9:5: 9:18 + // mir::Constant + // + span: $DIR/inline-options.rs:9:5: 9:16 + // + literal: Const { ty: fn() {not_inlined}, val: Value(Scalar()) } + } + + bb1: { + StorageDead(_1); // scope 0 at $DIR/inline-options.rs:9:18: 9:19 + StorageLive(_2); // scope 0 at $DIR/inline-options.rs:10:5: 10:21 + StorageLive(_3); // scope 1 at $DIR/inline-options.rs:10:5: 10:21 + _3 = g() -> bb2; // scope 1 at $DIR/inline-options.rs:10:5: 10:21 + // mir::Constant + // + span: $DIR/inline-options.rs:10:5: 10:21 + // + literal: Const { ty: fn() {g}, val: Value(Scalar()) } + } + + bb2: { + StorageDead(_3); // scope 1 at $DIR/inline-options.rs:10:5: 10:21 + StorageLive(_4); // scope 1 at $DIR/inline-options.rs:10:5: 10:21 + _4 = g() -> bb3; // scope 1 at $DIR/inline-options.rs:10:5: 10:21 + // mir::Constant + // + span: $DIR/inline-options.rs:10:5: 10:21 + // + literal: Const { ty: fn() {g}, val: Value(Scalar()) } + } + + bb3: { + StorageDead(_4); // scope 1 at $DIR/inline-options.rs:10:5: 10:21 + StorageLive(_5); // scope 1 at $DIR/inline-options.rs:10:5: 10:21 + _5 = g() -> bb4; // scope 1 at $DIR/inline-options.rs:10:5: 10:21 + // mir::Constant + // + span: $DIR/inline-options.rs:10:5: 10:21 + // + literal: Const { ty: fn() {g}, val: Value(Scalar()) } + } + + bb4: { + StorageDead(_5); // scope 1 at $DIR/inline-options.rs:10:5: 10:21 + _2 = const (); // scope 1 at $DIR/inline-options.rs:10:5: 10:21 + StorageDead(_2); // scope 0 at $DIR/inline-options.rs:10:21: 10:22 + _0 = const (); // scope 0 at $DIR/inline-options.rs:8:11: 11:2 + return; // scope 0 at $DIR/inline-options.rs:11:2: 11:2 + } +} From 03eec5cc732ea298dde33f3188ac9ec926811459 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 10 Nov 2020 11:09:34 -0500 Subject: [PATCH 37/42] Cleanup and comment intra-doc link pass --- .../passes/collect_intra_doc_links.rs | 414 +++++++++++------- 1 file changed, 247 insertions(+), 167 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 8be9482acffde..c9d4f51cbf3a7 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1,3 +1,7 @@ +//! This module implements [RFC 1946]: Intra-rustdoc-links +//! +//! [RFC 1946]: https://github.com/rust-lang/rfcs/blob/master/text/1946-intra-rustdoc-links.md + use rustc_ast as ast; use rustc_data_structures::stable_set::FxHashSet; use rustc_errors::{Applicability, DiagnosticBuilder}; @@ -27,7 +31,7 @@ use std::cell::Cell; use std::mem; use std::ops::Range; -use crate::clean::*; +use crate::clean::{self, Crate, GetDefId, Import, Item, ItemLink, PrimitiveType}; use crate::core::DocContext; use crate::fold::DocFolder; use crate::html::markdown::markdown_links; @@ -42,10 +46,10 @@ pub const COLLECT_INTRA_DOC_LINKS: Pass = Pass { }; pub fn collect_intra_doc_links(krate: Crate, cx: &DocContext<'_>) -> Crate { - let mut coll = LinkCollector::new(cx); - coll.fold_crate(krate) + LinkCollector::new(cx).fold_crate(krate) } +/// Top-level errors emitted by this pass. enum ErrorKind<'a> { Resolve(Box>), AnchorFailure(AnchorFailure), @@ -58,18 +62,37 @@ impl<'a> From> for ErrorKind<'a> { } #[derive(Debug)] +/// A link failed to resolve. enum ResolutionFailure<'a> { /// This resolved, but with the wrong namespace. - /// `Namespace` is the expected namespace (as opposed to the actual). - WrongNamespace(Res, Namespace), + /// + /// `Namespace` is the namespace specified with a disambiguator + /// (as opposed to the actual namespace of the `Res`). + WrongNamespace(Res, /* disambiguated */ Namespace), /// The link failed to resolve. `resolution_failure` should look to see if there's /// a more helpful error that can be given. - NotResolved { module_id: DefId, partial_res: Option, unresolved: Cow<'a, str> }, - /// should not ever happen + NotResolved { + /// The scope the link was resolved in. + module_id: DefId, + /// If part of the link resolved, this has the `Res`. + /// + /// In `[std::io::Error::x]`, `std::io::Error` would be a partial resolution. + partial_res: Option, + /// The remaining unresolved path segments. + /// + /// In `[std::io::Error::x]`, `x` would be unresolved. + unresolved: Cow<'a, str>, + }, + /// This happens when rustdoc can't determine the parent scope for an item. + /// + /// It is always a bug in rustdoc. NoParentItem, /// This link has malformed generic parameters; e.g., the angle brackets are unbalanced. MalformedGenerics(MalformedGenerics), - /// used to communicate that this should be ignored, but shouldn't be reported to the user + /// Used to communicate that this should be ignored, but shouldn't be reported to the user + /// + /// This happens when there is no disambiguator and one of the namespaces + /// failed to resolve. Dummy, } @@ -115,7 +138,9 @@ enum MalformedGenerics { } impl ResolutionFailure<'a> { - // This resolved fully (not just partially) but is erroneous for some other reason + /// This resolved fully (not just partially) but is erroneous for some other reason + /// + /// Returns the full resolution of the link, if present. fn full_res(&self) -> Option { match self { Self::WrongNamespace(res, _) => Some(*res), @@ -125,13 +150,30 @@ impl ResolutionFailure<'a> { } enum AnchorFailure { + /// User error: `[std#x#y]` is not valid MultipleAnchors, + /// The anchor provided by the user conflicts with Rustdoc's generated anchor. + /// + /// This is an unfortunate state of affairs. Not every item that can be + /// linked to has its own page; sometimes it is a subheading within a page, + /// like for associated items. In those cases, rustdoc uses an anchor to + /// link to the subheading. Since you can't have two anchors for the same + /// link, Rustdoc disallows having a user-specified anchor. + /// + /// Most of the time this is fine, because you can just link to the page of + /// the item if you want to provide your own anchor. For primitives, though, + /// rustdoc uses the anchor as a side channel to know which page to link to; + /// it doesn't show up in the generated link. Ideally, rustdoc would remove + /// this limitation, allowing you to link to subheaders on primitives. RustdocAnchorConflict(Res), } struct LinkCollector<'a, 'tcx> { cx: &'a DocContext<'tcx>, - // NOTE: this may not necessarily be a module in the current crate + /// A stack of modules used to decide what scope to resolve in. + /// + /// The last module will be used if the parent scope of the current item is + /// unknown. mod_ids: Vec, /// This is used to store the kind of associated items, /// because `clean` and the disambiguator code expect them to be different. @@ -144,6 +186,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { LinkCollector { cx, mod_ids: Vec::new(), kind_side_channel: Cell::new(None) } } + /// Given a full link, parse it as an [enum struct variant]. + /// + /// In particular, this will return an error whenever there aren't three + /// full path segments left in the link. + /// + /// [enum struct variant]: hir::VariantData::Struct fn variant_field( &self, path_str: &'path str, @@ -235,6 +283,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } } + /// Given a primitive type, try to resolve an associated item. + /// + /// HACK(jynelson): `item_str` is passed in instead of derived from `item_name` so the + /// lifetimes on `&'path` will work. fn resolve_primitive_associated_item( &self, prim_ty: hir::PrimTy, @@ -286,7 +338,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } /// Resolves a string as a macro. - fn macro_resolve( + /// + /// FIXME(jynelson): Can this be unified with `resolve()`? + fn resolve_macro( &self, path_str: &'a str, module_id: DefId, @@ -294,6 +348,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { let cx = self.cx; let path = ast::Path::from_ident(Ident::from_str(path_str)); cx.enter_resolver(|resolver| { + // FIXME(jynelson): does this really need 3 separate lookups? if let Ok((Some(ext), res)) = resolver.resolve_macro_path( &path, None, @@ -326,6 +381,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) } + /// Convenience wrapper around `resolve_str_path_error`. + /// + /// This also handles resolving `true` and `false` as booleans. + /// NOTE: `resolve_str_path_error` knows only about paths, not about types. + /// Associated items will never be resolved by this function. fn resolve_path(&self, path_str: &str, ns: Namespace, module_id: DefId) -> Option { let result = self.cx.enter_resolver(|resolver| { resolver.resolve_str_path_error(DUMMY_SP, &path_str, ns, module_id) @@ -339,12 +399,13 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } } - /// Resolves a string as a path within a particular namespace. Also returns an optional - /// URL fragment in the case of variants and methods. + /// Resolves a string as a path within a particular namespace. Returns an + /// optional URL fragment in the case of variants and methods. fn resolve<'path>( &self, path_str: &'path str, ns: Namespace, + // FIXME(#76467): This is for `Self`, and it's wrong. current_item: &Option, module_id: DefId, extra_fragment: &Option, @@ -353,15 +414,13 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { if let Some(res) = self.resolve_path(path_str, ns, module_id) { match res { + // FIXME(#76467): make this fallthrough to lookup the associated + // item a separate function. Res::Def(DefKind::AssocFn | DefKind::AssocConst, _) => { assert_eq!(ns, ValueNS); - // Fall through: In case this is a trait item, skip the - // early return and try looking for the trait. } Res::Def(DefKind::AssocTy, _) => { assert_eq!(ns, TypeNS); - // Fall through: In case this is a trait item, skip the - // early return and try looking for the trait. } Res::Def(DefKind::Variant, _) => { return handle_variant(cx, res, extra_fragment); @@ -410,7 +469,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { })?; // FIXME: are these both necessary? - let ty_res = if let Some(ty_res) = is_primitive(&path_root, TypeNS) + let ty_res = if let Some(ty_res) = resolve_primitive(&path_root, TypeNS) .map(|(_, res)| res) .or_else(|| self.resolve_path(&path_root, TypeNS, module_id)) { @@ -452,8 +511,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // There should only ever be one associated item that matches from any inherent impl .next() // Check if item_name belongs to `impl SomeTrait for SomeItem` - // This gives precedence to `impl SomeItem`: - // Although having both would be ambiguous, use impl version for compat. sake. + // FIXME(#74563): This gives precedence to `impl SomeItem`: + // Although having both would be ambiguous, use impl version for compatibility's sake. // To handle that properly resolve() would have to support // something like [`ambi_fn`](::ambi_fn) .or_else(|| { @@ -480,6 +539,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) } else if ns == Namespace::ValueNS { debug!("looking for variants or fields named {} for {:?}", item_name, did); + // FIXME(jynelson): why is this different from + // `variant_field`? match cx.tcx.type_of(did).kind() { ty::Adt(def, _) => { let field = if def.is_enum() { @@ -577,7 +638,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ) -> Option { // resolve() can't be used for macro namespace let result = match ns { - Namespace::MacroNS => self.macro_resolve(path_str, module_id).map_err(ErrorKind::from), + Namespace::MacroNS => self.resolve_macro(path_str, module_id).map_err(ErrorKind::from), Namespace::TypeNS | Namespace::ValueNS => self .resolve(path_str, ns, current_item, module_id, extra_fragment) .map(|(res, _)| res), @@ -593,6 +654,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } } +/// Look to see if a resolved item has an associated item named `item_name`. +/// +/// Given `[std::io::Error::source]`, where `source` is unresolved, this would +/// find `std::error::Error::source` and return +/// `::source`. fn resolve_associated_trait_item( did: DefId, module: DefId, @@ -601,12 +667,12 @@ fn resolve_associated_trait_item( cx: &DocContext<'_>, ) -> Option<(ty::AssocKind, DefId)> { let ty = cx.tcx.type_of(did); - // First consider automatic impls: `impl From for T` + // First consider blanket impls: `impl From for T` let implicit_impls = crate::clean::get_auto_trait_and_blanket_impls(cx, ty, did); let mut candidates: Vec<_> = implicit_impls .flat_map(|impl_outer| { match impl_outer.inner { - ImplItem(impl_) => { + clean::ImplItem(impl_) => { debug!("considering auto or blanket impl for trait {:?}", impl_.trait_); // Give precedence to methods that were overridden if !impl_.provided_trait_methods.contains(&*item_name.as_str()) { @@ -669,7 +735,7 @@ fn resolve_associated_trait_item( .map(|assoc| (assoc.kind, assoc.def_id)) })); } - // FIXME: warn about ambiguity + // FIXME(#74563): warn about ambiguity debug!("the candidates were {:?}", candidates); candidates.pop() } @@ -719,20 +785,15 @@ fn traits_implemented_by(cx: &DocContext<'_>, type_: DefId, module: DefId) -> Fx iter.collect() } -/// Check for resolve collisions between a trait and its derive +/// Check for resolve collisions between a trait and its derive. /// -/// These are common and we should just resolve to the trait in that case +/// These are common and we should just resolve to the trait in that case. fn is_derive_trait_collision(ns: &PerNS>>) -> bool { - if let PerNS { + matches!(*ns, PerNS { type_ns: Ok((Res::Def(DefKind::Trait, _), _)), macro_ns: Ok((Res::Def(DefKind::Macro(MacroKind::Derive), _), _)), .. - } = *ns - { - true - } else { - false - } + }) } impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { @@ -772,29 +833,30 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } let current_item = match item.inner { - ModuleItem(..) => { + clean::ModuleItem(..) => { if item.attrs.inner_docs { if item.def_id.is_top_level_module() { item.name.clone() } else { None } } else { match parent_node.or(self.mod_ids.last().copied()) { Some(parent) if !parent.is_top_level_module() => { - // FIXME: can we pull the parent module's name from elsewhere? Some(self.cx.tcx.item_name(parent).to_string()) } _ => None, } } } - ImplItem(Impl { ref for_, .. }) => { + clean::ImplItem(clean::Impl { ref for_, .. }) => { for_.def_id().map(|did| self.cx.tcx.item_name(did).to_string()) } // we don't display docs on `extern crate` items anyway, so don't process them. - ExternCrateItem(..) => { + clean::ExternCrateItem(..) => { debug!("ignoring extern crate item {:?}", item.def_id); return self.fold_item_recur(item); } - ImportItem(Import { kind: ImportKind::Simple(ref name, ..), .. }) => Some(name.clone()), - MacroItem(..) => None, + clean::ImportItem(Import { kind: clean::ImportKind::Simple(ref name, ..), .. }) => { + Some(name.clone()) + } + clean::MacroItem(..) => None, _ => item.name.clone(), }; @@ -803,6 +865,8 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } // find item's parent to resolve `Self` in item's docs below + // FIXME(#76467, #75809): this is a mess and doesn't handle cross-crate + // re-exports let parent_name = self.cx.as_local_hir_id(item.def_id).and_then(|item_hir| { let parent_hir = self.cx.tcx.hir().get_parent_item(item_hir); let item_parent = self.cx.tcx.hir().find(parent_hir); @@ -870,7 +934,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { }; // NOTE: if there are links that start in one crate and end in another, this will not resolve them. // This is a degenerate case and it's not supported by rustdoc. - // FIXME: this will break links that start in `#[doc = ...]` and end as a sugared doc. Should this be supported? for (ori_link, link_range) in markdown_links(&combined_docs) { let link = self.resolve_link( &item, @@ -888,15 +951,13 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } - if item.is_mod() && !item.attrs.inner_docs { - self.mod_ids.push(item.def_id); - } - if item.is_mod() { - let ret = self.fold_item_recur(item); + if !item.attrs.inner_docs { + self.mod_ids.push(item.def_id); + } + let ret = self.fold_item_recur(item); self.mod_ids.pop(); - ret } else { self.fold_item_recur(item) @@ -905,6 +966,9 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } impl LinkCollector<'_, '_> { + /// This is the entry point for resolving an intra-doc link. + /// + /// FIXME(jynelson): this is way too many arguments fn resolve_link( &self, item: &Item, @@ -943,129 +1007,120 @@ impl LinkCollector<'_, '_> { } else { (parts[0], None) }; - let resolved_self; - let link_text; - let mut path_str; - let disambiguator; - let stripped_path_string; - let (mut res, mut fragment) = { - path_str = if let Ok((d, path)) = Disambiguator::from_str(&link) { - disambiguator = Some(d); - path - } else { - disambiguator = None; - &link - } - .trim(); - if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ":_<>, ".contains(ch))) { - return None; - } + // Parse and strip the disambiguator from the link, if present. + let (mut path_str, disambiguator) = if let Ok((d, path)) = Disambiguator::from_str(&link) { + (path.trim(), Some(d)) + } else { + (link.trim(), None) + }; - // We stripped `()` and `!` when parsing the disambiguator. - // Add them back to be displayed, but not prefix disambiguators. - link_text = disambiguator - .map(|d| d.display_for(path_str)) - .unwrap_or_else(|| path_str.to_owned()); - - // In order to correctly resolve intra-doc-links we need to - // pick a base AST node to work from. If the documentation for - // this module came from an inner comment (//!) then we anchor - // our name resolution *inside* the module. If, on the other - // hand it was an outer comment (///) then we anchor the name - // resolution in the parent module on the basis that the names - // used are more likely to be intended to be parent names. For - // this, we set base_node to None for inner comments since - // we've already pushed this node onto the resolution stack but - // for outer comments we explicitly try and resolve against the - // parent_node first. - let base_node = if item.is_mod() && item.attrs.inner_docs { - self.mod_ids.last().copied() - } else { - parent_node - }; + if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ":_<>, ".contains(ch))) { + return None; + } - let mut module_id = if let Some(id) = base_node { - id - } else { - debug!("attempting to resolve item without parent module: {}", path_str); - let err_kind = ResolutionFailure::NoParentItem.into(); - resolution_failure( - self, - &item, - path_str, - disambiguator, - dox, - link_range, - smallvec![err_kind], - ); - return None; - }; + // We stripped `()` and `!` when parsing the disambiguator. + // Add them back to be displayed, but not prefix disambiguators. + let link_text = + disambiguator.map(|d| d.display_for(path_str)).unwrap_or_else(|| path_str.to_owned()); + + // In order to correctly resolve intra-doc-links we need to + // pick a base AST node to work from. If the documentation for + // this module came from an inner comment (//!) then we anchor + // our name resolution *inside* the module. If, on the other + // hand it was an outer comment (///) then we anchor the name + // resolution in the parent module on the basis that the names + // used are more likely to be intended to be parent names. For + // this, we set base_node to None for inner comments since + // we've already pushed this node onto the resolution stack but + // for outer comments we explicitly try and resolve against the + // parent_node first. + let base_node = if item.is_mod() && item.attrs.inner_docs { + self.mod_ids.last().copied() + } else { + parent_node + }; - // replace `Self` with suitable item's parent name - if path_str.starts_with("Self::") { - if let Some(ref name) = parent_name { - resolved_self = format!("{}::{}", name, &path_str[6..]); - path_str = &resolved_self; - } - } else if path_str.starts_with("crate::") { - use rustc_span::def_id::CRATE_DEF_INDEX; - - // HACK(jynelson): rustc_resolve thinks that `crate` is the crate currently being documented. - // But rustdoc wants it to mean the crate this item was originally present in. - // To work around this, remove it and resolve relative to the crate root instead. - // HACK(jynelson)(2): If we just strip `crate::` then suddenly primitives become ambiguous - // (consider `crate::char`). Instead, change it to `self::`. This works because 'self' is now the crate root. - resolved_self = format!("self::{}", &path_str["crate::".len()..]); - path_str = &resolved_self; - module_id = DefId { krate, index: CRATE_DEF_INDEX }; - } + let mut module_id = if let Some(id) = base_node { + id + } else { + // This is a bug. + debug!("attempting to resolve item without parent module: {}", path_str); + let err_kind = ResolutionFailure::NoParentItem.into(); + resolution_failure( + self, + &item, + path_str, + disambiguator, + dox, + link_range, + smallvec![err_kind], + ); + return None; + }; - // Strip generics from the path. - if path_str.contains(['<', '>'].as_slice()) { - stripped_path_string = match strip_generics_from_path(path_str) { - Ok(path) => path, - Err(err_kind) => { - debug!("link has malformed generics: {}", path_str); - resolution_failure( - self, - &item, - path_str, - disambiguator, - dox, - link_range, - smallvec![err_kind], - ); - return None; - } - }; - path_str = &stripped_path_string; + let resolved_self; + // replace `Self` with suitable item's parent name + if path_str.starts_with("Self::") { + if let Some(ref name) = parent_name { + resolved_self = format!("{}::{}", name, &path_str[6..]); + path_str = &resolved_self; } + } else if path_str.starts_with("crate::") { + use rustc_span::def_id::CRATE_DEF_INDEX; + + // HACK(jynelson): rustc_resolve thinks that `crate` is the crate currently being documented. + // But rustdoc wants it to mean the crate this item was originally present in. + // To work around this, remove it and resolve relative to the crate root instead. + // HACK(jynelson)(2): If we just strip `crate::` then suddenly primitives become ambiguous + // (consider `crate::char`). Instead, change it to `self::`. This works because 'self' is now the crate root. + // FIXME(#78696): This doesn't always work. + resolved_self = format!("self::{}", &path_str["crate::".len()..]); + path_str = &resolved_self; + module_id = DefId { krate, index: CRATE_DEF_INDEX }; + } - // Sanity check to make sure we don't have any angle brackets after stripping generics. - assert!(!path_str.contains(['<', '>'].as_slice())); + // Strip generics from the path. + let stripped_path_string; + if path_str.contains(['<', '>'].as_slice()) { + stripped_path_string = match strip_generics_from_path(path_str) { + Ok(path) => path, + Err(err_kind) => { + debug!("link has malformed generics: {}", path_str); + resolution_failure( + self, + &item, + path_str, + disambiguator, + dox, + link_range, + smallvec![err_kind], + ); + return None; + } + }; + path_str = &stripped_path_string; + } + // Sanity check to make sure we don't have any angle brackets after stripping generics. + assert!(!path_str.contains(['<', '>'].as_slice())); - // The link is not an intra-doc link if it still contains commas or spaces after - // stripping generics. - if path_str.contains([',', ' '].as_slice()) { - return None; - } + // The link is not an intra-doc link if it still contains commas or spaces after + // stripping generics. + if path_str.contains([',', ' '].as_slice()) { + return None; + } - match self.resolve_with_disambiguator( - disambiguator, - item, - dox, - path_str, - current_item, - module_id, - extra_fragment, - &ori_link, - link_range.clone(), - ) { - Some(x) => x, - None => return None, - } - }; + let (mut res, mut fragment) = self.resolve_with_disambiguator( + disambiguator, + item, + dox, + path_str, + current_item, + module_id, + extra_fragment, + &ori_link, + link_range.clone(), + )?; // Check for a primitive which might conflict with a module // Report the ambiguity and require that the user specify which one they meant. @@ -1075,7 +1130,7 @@ impl LinkCollector<'_, '_> { None | Some(Disambiguator::Namespace(Namespace::TypeNS) | Disambiguator::Primitive) ) && !matches!(res, Res::PrimTy(_)) { - if let Some((path, prim)) = is_primitive(path_str, TypeNS) { + if let Some((path, prim)) = resolve_primitive(path_str, TypeNS) { // `prim@char` if matches!(disambiguator, Some(Disambiguator::Primitive)) { if fragment.is_some() { @@ -1168,11 +1223,13 @@ impl LinkCollector<'_, '_> { privacy_error(cx, &item, &path_str, dox, link_range); } } - let id = register_res(cx, res); + let id = clean::register_res(cx, res); Some(ItemLink { link: ori_link, link_text, did: Some(id), fragment }) } } + /// After parsing the disambiguator, resolve the main part of the link. + // FIXME(jynelson): wow this is just so much fn resolve_with_disambiguator( &self, disambiguator: Option, @@ -1232,7 +1289,7 @@ impl LinkCollector<'_, '_> { // Try everything! let mut candidates = PerNS { macro_ns: self - .macro_resolve(path_str, base_node) + .resolve_macro(path_str, base_node) .map(|res| (res, extra_fragment.clone())), type_ns: match self.resolve( path_str, @@ -1320,10 +1377,10 @@ impl LinkCollector<'_, '_> { } } Some(MacroNS) => { - match self.macro_resolve(path_str, base_node) { + match self.resolve_macro(path_str, base_node) { Ok(res) => Some((res, extra_fragment)), Err(mut kind) => { - // `macro_resolve` only looks in the macro namespace. Try to give a better error if possible. + // `resolve_macro` only looks in the macro namespace. Try to give a better error if possible. for &ns in &[TypeNS, ValueNS] { if let Some(res) = self.check_full_res( ns, @@ -1354,9 +1411,15 @@ impl LinkCollector<'_, '_> { } #[derive(Copy, Clone, Debug, PartialEq, Eq)] +/// Disambiguators for a link. enum Disambiguator { + /// `prim@` + /// + /// This is buggy, see Primitive, + /// `struct@` or `f()` Kind(DefKind), + /// `type@` Namespace(Namespace), } @@ -1373,7 +1436,7 @@ impl Disambiguator { } } - /// (disambiguator, path_str) + /// Given a link, parse and return `(disambiguator, path_str)` fn from_str(link: &str) -> Result<(Self, &str), ()> { use Disambiguator::{Kind, Namespace as NS, Primitive}; @@ -1424,6 +1487,7 @@ impl Disambiguator { } } + /// Used for error reporting. fn suggestion(self) -> Suggestion { let kind = match self { Disambiguator::Primitive => return Suggestion::Prefix("prim"), @@ -1490,9 +1554,13 @@ impl Disambiguator { } } +/// A suggestion to show in a diagnostic. enum Suggestion { + /// `struct@` Prefix(&'static str), + /// `f()` Function, + /// `m!` Macro, } @@ -1582,6 +1650,11 @@ fn report_diagnostic( }); } +/// Reports a link that failed to resolve. +/// +/// This also tries to resolve any intermediate path segments that weren't +/// handled earlier. For example, if passed `Item::Crate(std)` and `path_str` +/// `std::io::Error::x`, this will resolve `std::io::Error`. fn resolution_failure( collector: &LinkCollector<'_, '_>, item: &Item, @@ -1816,6 +1889,7 @@ fn resolution_failure( ); } +/// Report an anchor failure. fn anchor_failure( cx: &DocContext<'_>, item: &Item, @@ -1840,6 +1914,7 @@ fn anchor_failure( }); } +/// Report an ambiguity error, where there were multiple possible resolutions. fn ambiguity_error( cx: &DocContext<'_>, item: &Item, @@ -1886,6 +1961,8 @@ fn ambiguity_error( }); } +/// In case of an ambiguity or mismatched disambiguator, suggest the correct +/// disambiguator. fn suggest_disambiguator( disambiguator: Disambiguator, diag: &mut DiagnosticBuilder<'_>, @@ -1911,6 +1988,7 @@ fn suggest_disambiguator( } } +/// Report a link from a public item to a private one. fn privacy_error( cx: &DocContext<'_>, item: &Item, @@ -1978,7 +2056,8 @@ const PRIMITIVES: &[(Symbol, Res)] = &[ (sym::char, Res::PrimTy(hir::PrimTy::Char)), ]; -fn is_primitive(path_str: &str, ns: Namespace) -> Option<(Symbol, Res)> { +/// Resolve a primitive type or value. +fn resolve_primitive(path_str: &str, ns: Namespace) -> Option<(Symbol, Res)> { is_bool_value(path_str, ns).or_else(|| { if ns == TypeNS { // FIXME: this should be replaced by a lookup in PrimitiveTypeTable @@ -1990,6 +2069,7 @@ fn is_primitive(path_str: &str, ns: Namespace) -> Option<(Symbol, Res)> { }) } +/// Resolve a primitive value. fn is_bool_value(path_str: &str, ns: Namespace) -> Option<(Symbol, Res)> { if ns == TypeNS && (path_str == "true" || path_str == "false") { Some((sym::bool, Res::PrimTy(hir::PrimTy::Bool))) From ce91c689432714938a9bd10860e5c0aa874226e2 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 10 Nov 2020 23:51:34 +0300 Subject: [PATCH 38/42] rustc_taret: Remove `TargetOptions::is_like_android` --- compiler/rustc_target/src/spec/android_base.rs | 1 - compiler/rustc_target/src/spec/mod.rs | 6 ------ 2 files changed, 7 deletions(-) diff --git a/compiler/rustc_target/src/spec/android_base.rs b/compiler/rustc_target/src/spec/android_base.rs index 1bd5eb6988c26..6fefae5732587 100644 --- a/compiler/rustc_target/src/spec/android_base.rs +++ b/compiler/rustc_target/src/spec/android_base.rs @@ -9,7 +9,6 @@ pub fn opts() -> TargetOptions { .get_mut(&LinkerFlavor::Gcc) .unwrap() .push("-Wl,--allow-multiple-definition".to_string()); - base.is_like_android = true; base.dwarf_version = Some(2); base.position_independent_executables = true; base.has_elf_tls = false; diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 55d27fd8698a7..859889fa64468 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -819,9 +819,6 @@ pub struct TargetOptions { /// library naming convention. Defaults to false. pub is_like_windows: bool, pub is_like_msvc: bool, - /// Whether the target toolchain is like Android's. Only useful for compiling against Android. - /// Defaults to false. - pub is_like_android: bool, /// Whether the target toolchain is like Emscripten's. Only useful for compiling with /// Emscripten toolchain. /// Defaults to false. @@ -1029,7 +1026,6 @@ impl Default for TargetOptions { is_like_osx: false, is_like_solaris: false, is_like_windows: false, - is_like_android: false, is_like_emscripten: false, is_like_msvc: false, is_like_fuchsia: false, @@ -1459,7 +1455,6 @@ impl Target { key!(is_like_windows, bool); key!(is_like_msvc, bool); key!(is_like_emscripten, bool); - key!(is_like_android, bool); key!(is_like_fuchsia, bool); key!(dwarf_version, Option); key!(linker_is_gnu, bool); @@ -1697,7 +1692,6 @@ impl ToJson for Target { target_option_val!(is_like_windows); target_option_val!(is_like_msvc); target_option_val!(is_like_emscripten); - target_option_val!(is_like_android); target_option_val!(is_like_fuchsia); target_option_val!(dwarf_version); target_option_val!(linker_is_gnu); From ca17a91fb7e503da403ab470bd564d66d12ba114 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 10 Nov 2020 23:32:58 +0300 Subject: [PATCH 39/42] rustc_target: Move target env "gnu" from `linux_base` to `linux_gnu_base` --- compiler/rustc_target/src/spec/aarch64_unknown_linux_gnu.rs | 2 +- compiler/rustc_target/src/spec/android_base.rs | 2 +- compiler/rustc_target/src/spec/arm_unknown_linux_gnueabi.rs | 2 +- .../rustc_target/src/spec/arm_unknown_linux_gnueabihf.rs | 2 +- .../rustc_target/src/spec/armv4t_unknown_linux_gnueabi.rs | 2 +- .../rustc_target/src/spec/armv5te_unknown_linux_gnueabi.rs | 2 +- .../rustc_target/src/spec/armv7_unknown_linux_gnueabi.rs | 2 +- .../rustc_target/src/spec/armv7_unknown_linux_gnueabihf.rs | 2 +- compiler/rustc_target/src/spec/i686_unknown_linux_gnu.rs | 2 +- compiler/rustc_target/src/spec/linux_base.rs | 1 - compiler/rustc_target/src/spec/linux_gnu_base.rs | 5 +++++ .../rustc_target/src/spec/mips64_unknown_linux_gnuabi64.rs | 2 +- .../rustc_target/src/spec/mips64el_unknown_linux_gnuabi64.rs | 2 +- compiler/rustc_target/src/spec/mips_unknown_linux_gnu.rs | 2 +- compiler/rustc_target/src/spec/mipsel_unknown_linux_gnu.rs | 2 +- .../rustc_target/src/spec/mipsisa32r6_unknown_linux_gnu.rs | 2 +- .../rustc_target/src/spec/mipsisa32r6el_unknown_linux_gnu.rs | 2 +- .../src/spec/mipsisa64r6_unknown_linux_gnuabi64.rs | 2 +- .../src/spec/mipsisa64r6el_unknown_linux_gnuabi64.rs | 2 +- compiler/rustc_target/src/spec/mod.rs | 1 + .../rustc_target/src/spec/powerpc64_unknown_linux_gnu.rs | 2 +- .../rustc_target/src/spec/powerpc64le_unknown_linux_gnu.rs | 2 +- compiler/rustc_target/src/spec/powerpc_unknown_linux_gnu.rs | 2 +- .../rustc_target/src/spec/powerpc_unknown_linux_gnuspe.rs | 2 +- .../rustc_target/src/spec/riscv32gc_unknown_linux_gnu.rs | 2 +- .../rustc_target/src/spec/riscv64gc_unknown_linux_gnu.rs | 2 +- compiler/rustc_target/src/spec/s390x_unknown_linux_gnu.rs | 2 +- compiler/rustc_target/src/spec/sparc64_unknown_linux_gnu.rs | 2 +- compiler/rustc_target/src/spec/sparc_unknown_linux_gnu.rs | 2 +- .../src/spec/thumbv7neon_unknown_linux_gnueabihf.rs | 2 +- compiler/rustc_target/src/spec/x86_64_unknown_linux_gnu.rs | 2 +- .../rustc_target/src/spec/x86_64_unknown_linux_gnux32.rs | 2 +- 32 files changed, 35 insertions(+), 30 deletions(-) create mode 100644 compiler/rustc_target/src/spec/linux_gnu_base.rs diff --git a/compiler/rustc_target/src/spec/aarch64_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/aarch64_unknown_linux_gnu.rs index cc9338ff970e1..58c72af4e769c 100644 --- a/compiler/rustc_target/src/spec/aarch64_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/aarch64_unknown_linux_gnu.rs @@ -1,7 +1,7 @@ use crate::spec::{Target, TargetOptions}; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.max_atomic_width = Some(128); Target { diff --git a/compiler/rustc_target/src/spec/android_base.rs b/compiler/rustc_target/src/spec/android_base.rs index 7b9f546c25a5c..40688f057f5b0 100644 --- a/compiler/rustc_target/src/spec/android_base.rs +++ b/compiler/rustc_target/src/spec/android_base.rs @@ -1,7 +1,7 @@ use crate::spec::{LinkerFlavor, TargetOptions}; pub fn opts() -> TargetOptions { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.os = "android".to_string(); // Many of the symbols defined in compiler-rt are also defined in libgcc. // Android's linker doesn't like that by default. diff --git a/compiler/rustc_target/src/spec/arm_unknown_linux_gnueabi.rs b/compiler/rustc_target/src/spec/arm_unknown_linux_gnueabi.rs index 17b6fb21e09bf..c41cf6e521a1d 100644 --- a/compiler/rustc_target/src/spec/arm_unknown_linux_gnueabi.rs +++ b/compiler/rustc_target/src/spec/arm_unknown_linux_gnueabi.rs @@ -1,7 +1,7 @@ use crate::spec::{Target, TargetOptions}; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.max_atomic_width = Some(64); Target { llvm_target: "arm-unknown-linux-gnueabi".to_string(), diff --git a/compiler/rustc_target/src/spec/arm_unknown_linux_gnueabihf.rs b/compiler/rustc_target/src/spec/arm_unknown_linux_gnueabihf.rs index 227709f0b0b58..f2143966c1dcd 100644 --- a/compiler/rustc_target/src/spec/arm_unknown_linux_gnueabihf.rs +++ b/compiler/rustc_target/src/spec/arm_unknown_linux_gnueabihf.rs @@ -1,7 +1,7 @@ use crate::spec::{Target, TargetOptions}; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.max_atomic_width = Some(64); Target { llvm_target: "arm-unknown-linux-gnueabihf".to_string(), diff --git a/compiler/rustc_target/src/spec/armv4t_unknown_linux_gnueabi.rs b/compiler/rustc_target/src/spec/armv4t_unknown_linux_gnueabi.rs index 7808437453caf..e1ba72bf83b9c 100644 --- a/compiler/rustc_target/src/spec/armv4t_unknown_linux_gnueabi.rs +++ b/compiler/rustc_target/src/spec/armv4t_unknown_linux_gnueabi.rs @@ -1,7 +1,7 @@ use crate::spec::{Target, TargetOptions}; pub fn target() -> Target { - let base = super::linux_base::opts(); + let base = super::linux_gnu_base::opts(); Target { llvm_target: "armv4t-unknown-linux-gnueabi".to_string(), pointer_width: 32, diff --git a/compiler/rustc_target/src/spec/armv5te_unknown_linux_gnueabi.rs b/compiler/rustc_target/src/spec/armv5te_unknown_linux_gnueabi.rs index d958354f5843e..3ac8d53564d7d 100644 --- a/compiler/rustc_target/src/spec/armv5te_unknown_linux_gnueabi.rs +++ b/compiler/rustc_target/src/spec/armv5te_unknown_linux_gnueabi.rs @@ -1,7 +1,7 @@ use crate::spec::{Target, TargetOptions}; pub fn target() -> Target { - let base = super::linux_base::opts(); + let base = super::linux_gnu_base::opts(); Target { llvm_target: "armv5te-unknown-linux-gnueabi".to_string(), pointer_width: 32, diff --git a/compiler/rustc_target/src/spec/armv7_unknown_linux_gnueabi.rs b/compiler/rustc_target/src/spec/armv7_unknown_linux_gnueabi.rs index 13798e869b7f5..ae6b8286f085f 100644 --- a/compiler/rustc_target/src/spec/armv7_unknown_linux_gnueabi.rs +++ b/compiler/rustc_target/src/spec/armv7_unknown_linux_gnueabi.rs @@ -4,7 +4,7 @@ use crate::spec::{Target, TargetOptions}; // hardfloat. pub fn target() -> Target { - let base = super::linux_base::opts(); + let base = super::linux_gnu_base::opts(); Target { llvm_target: "armv7-unknown-linux-gnueabi".to_string(), pointer_width: 32, diff --git a/compiler/rustc_target/src/spec/armv7_unknown_linux_gnueabihf.rs b/compiler/rustc_target/src/spec/armv7_unknown_linux_gnueabihf.rs index f80f56ee3c580..48c16b620fd69 100644 --- a/compiler/rustc_target/src/spec/armv7_unknown_linux_gnueabihf.rs +++ b/compiler/rustc_target/src/spec/armv7_unknown_linux_gnueabihf.rs @@ -4,7 +4,7 @@ use crate::spec::{Target, TargetOptions}; // thumb-mode. See the thumbv7neon variant for enabling both. pub fn target() -> Target { - let base = super::linux_base::opts(); + let base = super::linux_gnu_base::opts(); Target { llvm_target: "armv7-unknown-linux-gnueabihf".to_string(), pointer_width: 32, diff --git a/compiler/rustc_target/src/spec/i686_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/i686_unknown_linux_gnu.rs index 62b02d841c21d..083c115d084b4 100644 --- a/compiler/rustc_target/src/spec/i686_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/i686_unknown_linux_gnu.rs @@ -1,7 +1,7 @@ use crate::spec::{LinkerFlavor, Target}; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.cpu = "pentium4".to_string(); base.max_atomic_width = Some(64); base.pre_link_args.get_mut(&LinkerFlavor::Gcc).unwrap().push("-m32".to_string()); diff --git a/compiler/rustc_target/src/spec/linux_base.rs b/compiler/rustc_target/src/spec/linux_base.rs index a83cceb24ee9f..0631644ad6355 100644 --- a/compiler/rustc_target/src/spec/linux_base.rs +++ b/compiler/rustc_target/src/spec/linux_base.rs @@ -20,7 +20,6 @@ pub fn opts() -> TargetOptions { TargetOptions { os: "linux".to_string(), - env: "gnu".to_string(), dynamic_linking: true, executables: true, os_family: Some("unix".to_string()), diff --git a/compiler/rustc_target/src/spec/linux_gnu_base.rs b/compiler/rustc_target/src/spec/linux_gnu_base.rs new file mode 100644 index 0000000000000..3d940ceaf0270 --- /dev/null +++ b/compiler/rustc_target/src/spec/linux_gnu_base.rs @@ -0,0 +1,5 @@ +use crate::spec::TargetOptions; + +pub fn opts() -> TargetOptions { + TargetOptions { env: "gnu".to_string(), ..super::linux_base::opts() } +} diff --git a/compiler/rustc_target/src/spec/mips64_unknown_linux_gnuabi64.rs b/compiler/rustc_target/src/spec/mips64_unknown_linux_gnuabi64.rs index 593be2549fd46..daa0d9da1725e 100644 --- a/compiler/rustc_target/src/spec/mips64_unknown_linux_gnuabi64.rs +++ b/compiler/rustc_target/src/spec/mips64_unknown_linux_gnuabi64.rs @@ -14,7 +14,7 @@ pub fn target() -> Target { max_atomic_width: Some(64), mcount: "_mcount".to_string(), - ..super::linux_base::opts() + ..super::linux_gnu_base::opts() }, } } diff --git a/compiler/rustc_target/src/spec/mips64el_unknown_linux_gnuabi64.rs b/compiler/rustc_target/src/spec/mips64el_unknown_linux_gnuabi64.rs index eed8a56d86a4c..d767705b0459b 100644 --- a/compiler/rustc_target/src/spec/mips64el_unknown_linux_gnuabi64.rs +++ b/compiler/rustc_target/src/spec/mips64el_unknown_linux_gnuabi64.rs @@ -13,7 +13,7 @@ pub fn target() -> Target { max_atomic_width: Some(64), mcount: "_mcount".to_string(), - ..super::linux_base::opts() + ..super::linux_gnu_base::opts() }, } } diff --git a/compiler/rustc_target/src/spec/mips_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/mips_unknown_linux_gnu.rs index b746ac351d711..a7ec1f19c9de7 100644 --- a/compiler/rustc_target/src/spec/mips_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/mips_unknown_linux_gnu.rs @@ -13,7 +13,7 @@ pub fn target() -> Target { max_atomic_width: Some(32), mcount: "_mcount".to_string(), - ..super::linux_base::opts() + ..super::linux_gnu_base::opts() }, } } diff --git a/compiler/rustc_target/src/spec/mipsel_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/mipsel_unknown_linux_gnu.rs index e0f8350ee88e3..9cb2a13c7d4b5 100644 --- a/compiler/rustc_target/src/spec/mipsel_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/mipsel_unknown_linux_gnu.rs @@ -13,7 +13,7 @@ pub fn target() -> Target { max_atomic_width: Some(32), mcount: "_mcount".to_string(), - ..super::linux_base::opts() + ..super::linux_gnu_base::opts() }, } } diff --git a/compiler/rustc_target/src/spec/mipsisa32r6_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/mipsisa32r6_unknown_linux_gnu.rs index 9a649ec52a276..11b3734a10507 100644 --- a/compiler/rustc_target/src/spec/mipsisa32r6_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/mipsisa32r6_unknown_linux_gnu.rs @@ -13,7 +13,7 @@ pub fn target() -> Target { max_atomic_width: Some(32), mcount: "_mcount".to_string(), - ..super::linux_base::opts() + ..super::linux_gnu_base::opts() }, } } diff --git a/compiler/rustc_target/src/spec/mipsisa32r6el_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/mipsisa32r6el_unknown_linux_gnu.rs index 20fbefe6f2e8d..06a5f40d69b3a 100644 --- a/compiler/rustc_target/src/spec/mipsisa32r6el_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/mipsisa32r6el_unknown_linux_gnu.rs @@ -13,7 +13,7 @@ pub fn target() -> Target { max_atomic_width: Some(32), mcount: "_mcount".to_string(), - ..super::linux_base::opts() + ..super::linux_gnu_base::opts() }, } } diff --git a/compiler/rustc_target/src/spec/mipsisa64r6_unknown_linux_gnuabi64.rs b/compiler/rustc_target/src/spec/mipsisa64r6_unknown_linux_gnuabi64.rs index a5da3e5d42c4b..6282c9e1d54b6 100644 --- a/compiler/rustc_target/src/spec/mipsisa64r6_unknown_linux_gnuabi64.rs +++ b/compiler/rustc_target/src/spec/mipsisa64r6_unknown_linux_gnuabi64.rs @@ -14,7 +14,7 @@ pub fn target() -> Target { max_atomic_width: Some(64), mcount: "_mcount".to_string(), - ..super::linux_base::opts() + ..super::linux_gnu_base::opts() }, } } diff --git a/compiler/rustc_target/src/spec/mipsisa64r6el_unknown_linux_gnuabi64.rs b/compiler/rustc_target/src/spec/mipsisa64r6el_unknown_linux_gnuabi64.rs index 73fbbaed4d5b9..589d7acba6804 100644 --- a/compiler/rustc_target/src/spec/mipsisa64r6el_unknown_linux_gnuabi64.rs +++ b/compiler/rustc_target/src/spec/mipsisa64r6el_unknown_linux_gnuabi64.rs @@ -13,7 +13,7 @@ pub fn target() -> Target { max_atomic_width: Some(64), mcount: "_mcount".to_string(), - ..super::linux_base::opts() + ..super::linux_gnu_base::opts() }, } } diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 2a4ae786fb752..271ceb2a03608 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -64,6 +64,7 @@ mod hermit_kernel_base; mod illumos_base; mod l4re_base; mod linux_base; +mod linux_gnu_base; mod linux_kernel_base; mod linux_musl_base; mod linux_uclibc_base; diff --git a/compiler/rustc_target/src/spec/powerpc64_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/powerpc64_unknown_linux_gnu.rs index 27515ac6e1f9f..03322818d33c3 100644 --- a/compiler/rustc_target/src/spec/powerpc64_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/powerpc64_unknown_linux_gnu.rs @@ -1,7 +1,7 @@ use crate::spec::{LinkerFlavor, RelroLevel, Target, TargetOptions}; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.cpu = "ppc64".to_string(); base.pre_link_args.get_mut(&LinkerFlavor::Gcc).unwrap().push("-m64".to_string()); base.max_atomic_width = Some(64); diff --git a/compiler/rustc_target/src/spec/powerpc64le_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/powerpc64le_unknown_linux_gnu.rs index 3c4389c5a7cca..07e0bf81bc72f 100644 --- a/compiler/rustc_target/src/spec/powerpc64le_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/powerpc64le_unknown_linux_gnu.rs @@ -1,7 +1,7 @@ use crate::spec::{LinkerFlavor, Target, TargetOptions}; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.cpu = "ppc64le".to_string(); base.pre_link_args.get_mut(&LinkerFlavor::Gcc).unwrap().push("-m64".to_string()); base.max_atomic_width = Some(64); diff --git a/compiler/rustc_target/src/spec/powerpc_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/powerpc_unknown_linux_gnu.rs index ece01705c4518..3a9271247b042 100644 --- a/compiler/rustc_target/src/spec/powerpc_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/powerpc_unknown_linux_gnu.rs @@ -1,7 +1,7 @@ use crate::spec::{LinkerFlavor, Target, TargetOptions}; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.pre_link_args.get_mut(&LinkerFlavor::Gcc).unwrap().push("-m32".to_string()); base.max_atomic_width = Some(32); diff --git a/compiler/rustc_target/src/spec/powerpc_unknown_linux_gnuspe.rs b/compiler/rustc_target/src/spec/powerpc_unknown_linux_gnuspe.rs index 35c2878747105..105a0b21aaf01 100644 --- a/compiler/rustc_target/src/spec/powerpc_unknown_linux_gnuspe.rs +++ b/compiler/rustc_target/src/spec/powerpc_unknown_linux_gnuspe.rs @@ -1,7 +1,7 @@ use crate::spec::{LinkerFlavor, Target, TargetOptions}; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.pre_link_args.get_mut(&LinkerFlavor::Gcc).unwrap().push("-mspe".to_string()); base.max_atomic_width = Some(32); diff --git a/compiler/rustc_target/src/spec/riscv32gc_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/riscv32gc_unknown_linux_gnu.rs index f9405d9dfb63a..cf5e0201d08bf 100644 --- a/compiler/rustc_target/src/spec/riscv32gc_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/riscv32gc_unknown_linux_gnu.rs @@ -13,7 +13,7 @@ pub fn target() -> Target { features: "+m,+a,+f,+d,+c".to_string(), llvm_abiname: "ilp32d".to_string(), max_atomic_width: Some(32), - ..super::linux_base::opts() + ..super::linux_gnu_base::opts() }, } } diff --git a/compiler/rustc_target/src/spec/riscv64gc_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/riscv64gc_unknown_linux_gnu.rs index 3b7ff47a54009..84f28413fcbce 100644 --- a/compiler/rustc_target/src/spec/riscv64gc_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/riscv64gc_unknown_linux_gnu.rs @@ -13,7 +13,7 @@ pub fn target() -> Target { features: "+m,+a,+f,+d,+c".to_string(), llvm_abiname: "lp64d".to_string(), max_atomic_width: Some(64), - ..super::linux_base::opts() + ..super::linux_gnu_base::opts() }, } } diff --git a/compiler/rustc_target/src/spec/s390x_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/s390x_unknown_linux_gnu.rs index 258b83a1c6e1b..d6e8e6ee220e6 100644 --- a/compiler/rustc_target/src/spec/s390x_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/s390x_unknown_linux_gnu.rs @@ -1,7 +1,7 @@ use crate::spec::Target; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.endian = "big".to_string(); // z10 is the oldest CPU supported by LLVM base.cpu = "z10".to_string(); diff --git a/compiler/rustc_target/src/spec/sparc64_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/sparc64_unknown_linux_gnu.rs index 4b5ee050d7265..e9b5520ac3d37 100644 --- a/compiler/rustc_target/src/spec/sparc64_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/sparc64_unknown_linux_gnu.rs @@ -1,7 +1,7 @@ use crate::spec::Target; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.endian = "big".to_string(); base.cpu = "v9".to_string(); base.max_atomic_width = Some(64); diff --git a/compiler/rustc_target/src/spec/sparc_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/sparc_unknown_linux_gnu.rs index 8d7b34fe2cbfd..aae186b2293c7 100644 --- a/compiler/rustc_target/src/spec/sparc_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/sparc_unknown_linux_gnu.rs @@ -1,7 +1,7 @@ use crate::spec::{LinkerFlavor, Target}; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.endian = "big".to_string(); base.cpu = "v9".to_string(); base.max_atomic_width = Some(64); diff --git a/compiler/rustc_target/src/spec/thumbv7neon_unknown_linux_gnueabihf.rs b/compiler/rustc_target/src/spec/thumbv7neon_unknown_linux_gnueabihf.rs index 561da4d15cd61..352d246874308 100644 --- a/compiler/rustc_target/src/spec/thumbv7neon_unknown_linux_gnueabihf.rs +++ b/compiler/rustc_target/src/spec/thumbv7neon_unknown_linux_gnueabihf.rs @@ -7,7 +7,7 @@ use crate::spec::{Target, TargetOptions}; // https://static.docs.arm.com/ddi0406/cd/DDI0406C_d_armv7ar_arm.pdf pub fn target() -> Target { - let base = super::linux_base::opts(); + let base = super::linux_gnu_base::opts(); Target { llvm_target: "armv7-unknown-linux-gnueabihf".to_string(), pointer_width: 32, diff --git a/compiler/rustc_target/src/spec/x86_64_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/x86_64_unknown_linux_gnu.rs index 1f368ff161191..f127dd49bc4b7 100644 --- a/compiler/rustc_target/src/spec/x86_64_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/x86_64_unknown_linux_gnu.rs @@ -1,7 +1,7 @@ use crate::spec::{LinkerFlavor, Target}; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.cpu = "x86-64".to_string(); base.max_atomic_width = Some(64); base.pre_link_args.get_mut(&LinkerFlavor::Gcc).unwrap().push("-m64".to_string()); diff --git a/compiler/rustc_target/src/spec/x86_64_unknown_linux_gnux32.rs b/compiler/rustc_target/src/spec/x86_64_unknown_linux_gnux32.rs index 375b22fd92b93..0cae57528483f 100644 --- a/compiler/rustc_target/src/spec/x86_64_unknown_linux_gnux32.rs +++ b/compiler/rustc_target/src/spec/x86_64_unknown_linux_gnux32.rs @@ -1,7 +1,7 @@ use crate::spec::{LinkerFlavor, Target}; pub fn target() -> Target { - let mut base = super::linux_base::opts(); + let mut base = super::linux_gnu_base::opts(); base.cpu = "x86-64".to_string(); base.max_atomic_width = Some(64); base.pre_link_args.get_mut(&LinkerFlavor::Gcc).unwrap().push("-mx32".to_string()); From 1854425758d6476a8940fba6c8a09c1a617becb1 Mon Sep 17 00:00:00 2001 From: Ikko Ashimine Date: Wed, 11 Nov 2020 20:23:08 +0900 Subject: [PATCH 40/42] Fix typo in comment occurences -> occurrences --- src/tools/clippy/clippy_lints/src/loops.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/clippy/clippy_lints/src/loops.rs b/src/tools/clippy/clippy_lints/src/loops.rs index 32c2562ee95b4..5a4264f9b5c4e 100644 --- a/src/tools/clippy/clippy_lints/src/loops.rs +++ b/src/tools/clippy/clippy_lints/src/loops.rs @@ -3089,7 +3089,7 @@ impl<'tcx> Visitor<'tcx> for IterFunctionVisitor { } } -/// Detect the occurences of calls to `iter` or `into_iter` for the +/// Detect the occurrences of calls to `iter` or `into_iter` for the /// given identifier fn detect_iter_and_into_iters<'tcx>(block: &'tcx Block<'tcx>, identifier: Ident) -> Option> { let mut visitor = IterFunctionVisitor { From de84ad95b4b097e504171c7c606bf3715803c13b Mon Sep 17 00:00:00 2001 From: Fabian Zaiser Date: Sat, 7 Nov 2020 14:28:55 +0000 Subject: [PATCH 41/42] Implement destructuring assignment for structs and slices Co-authored-by: varkor --- compiler/rustc_ast/src/ast.rs | 19 ++- compiler/rustc_ast/src/mut_visit.rs | 6 +- compiler/rustc_ast/src/visit.rs | 6 +- compiler/rustc_ast_lowering/src/expr.rs | 126 +++++++++++++++++- compiler/rustc_ast_passes/src/feature_gate.rs | 1 + compiler/rustc_ast_pretty/src/pprust/state.rs | 21 ++- compiler/rustc_expand/src/build.rs | 2 +- compiler/rustc_hir/src/def.rs | 5 + compiler/rustc_parse/src/parser/expr.rs | 10 +- compiler/rustc_resolve/src/late.rs | 4 +- .../rustc_save_analysis/src/dump_visitor.rs | 10 +- src/test/ui-fulldeps/pprust-expr-roundtrip.rs | 2 +- .../nested_destructure.rs | 17 +++ .../note-unsupported.rs | 7 +- .../note-unsupported.stderr | 69 ++++++---- .../slice_destructure.rs | 15 +++ .../slice_destructure_fail.rs | 7 + .../slice_destructure_fail.stderr | 17 +++ .../struct_destructure.rs | 19 +++ .../struct_destructure_fail.rs | 15 +++ .../struct_destructure_fail.stderr | 21 +++ .../tuple_struct_destructure.rs | 34 +++++ .../tuple_struct_destructure_fail.rs | 42 ++++++ .../tuple_struct_destructure_fail.stderr | 62 +++++++++ .../underscore-range-expr-gating.rs | 8 ++ .../underscore-range-expr-gating.stderr | 12 ++ src/test/ui/issues/issue-77218.rs | 10 +- src/test/ui/issues/issue-77218.stderr | 38 +++++- src/test/ui/suggestions/if-let-typo.rs | 5 + src/test/ui/suggestions/if-let-typo.stderr | 75 +++++++---- .../clippy_lints/src/utils/ast_utils.rs | 11 +- .../clippy/tests/ui/crashes/ice-6250.stderr | 29 +++- 32 files changed, 618 insertions(+), 107 deletions(-) create mode 100644 src/test/ui/destructuring-assignment/nested_destructure.rs create mode 100644 src/test/ui/destructuring-assignment/slice_destructure.rs create mode 100644 src/test/ui/destructuring-assignment/slice_destructure_fail.rs create mode 100644 src/test/ui/destructuring-assignment/slice_destructure_fail.stderr create mode 100644 src/test/ui/destructuring-assignment/struct_destructure.rs create mode 100644 src/test/ui/destructuring-assignment/struct_destructure_fail.rs create mode 100644 src/test/ui/destructuring-assignment/struct_destructure_fail.stderr create mode 100644 src/test/ui/destructuring-assignment/tuple_struct_destructure.rs create mode 100644 src/test/ui/destructuring-assignment/tuple_struct_destructure_fail.rs create mode 100644 src/test/ui/destructuring-assignment/tuple_struct_destructure_fail.stderr create mode 100644 src/test/ui/destructuring-assignment/underscore-range-expr-gating.rs create mode 100644 src/test/ui/destructuring-assignment/underscore-range-expr-gating.stderr diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index f13d67b9c1584..1886581df9a94 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1061,7 +1061,7 @@ pub struct Expr { // `Expr` is used a lot. Make sure it doesn't unintentionally get bigger. #[cfg(target_arch = "x86_64")] -rustc_data_structures::static_assert_size!(Expr, 112); +rustc_data_structures::static_assert_size!(Expr, 120); impl Expr { /// Returns `true` if this expression would be valid somewhere that expects a value; @@ -1218,6 +1218,16 @@ pub enum RangeLimits { Closed, } +#[derive(Clone, Encodable, Decodable, Debug)] +pub enum StructRest { + /// `..x`. + Base(P), + /// `..`. + Rest(Span), + /// No trailing `..` or expression. + None, +} + #[derive(Clone, Encodable, Decodable, Debug)] pub enum ExprKind { /// A `box x` expression. @@ -1312,7 +1322,7 @@ pub enum ExprKind { Field(P, Ident), /// An indexing operation (e.g., `foo[2]`). Index(P, P), - /// A range (e.g., `1..2`, `1..`, `..2`, `1..=2`, `..=2`). + /// A range (e.g., `1..2`, `1..`, `..2`, `1..=2`, `..=2`; and `..` in destructuring assingment). Range(Option>, Option>, RangeLimits), /// Variable reference, possibly containing `::` and/or type @@ -1340,9 +1350,8 @@ pub enum ExprKind { /// A struct literal expression. /// - /// E.g., `Foo {x: 1, y: 2}`, or `Foo {x: 1, .. base}`, - /// where `base` is the `Option`. - Struct(Path, Vec, Option>), + /// E.g., `Foo {x: 1, y: 2}`, or `Foo {x: 1, .. rest}`. + Struct(Path, Vec, StructRest), /// An array literal constructed from one repeated element. /// diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 7b85d28568bd3..a80a6adb4fae8 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1288,7 +1288,11 @@ pub fn noop_visit_expr( ExprKind::Struct(path, fields, expr) => { vis.visit_path(path); fields.flat_map_in_place(|field| vis.flat_map_field(field)); - visit_opt(expr, |expr| vis.visit_expr(expr)); + match expr { + StructRest::Base(expr) => vis.visit_expr(expr), + StructRest::Rest(_span) => {} + StructRest::None => {} + } } ExprKind::Paren(expr) => { vis.visit_expr(expr); diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index 8751f09cfcbbe..0666eb199e673 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -719,7 +719,11 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) { ExprKind::Struct(ref path, ref fields, ref optional_base) => { visitor.visit_path(path, expression.id); walk_list!(visitor, visit_field, fields); - walk_list!(visitor, visit_expr, optional_base); + match optional_base { + StructRest::Base(expr) => visitor.visit_expr(expr), + StructRest::Rest(_span) => {} + StructRest::None => {} + } } ExprKind::Tup(ref subexpressions) => { walk_list!(visitor, visit_expr, subexpressions); diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 1f2aba2b27e68..330776fc8c598 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -187,8 +187,18 @@ impl<'hir> LoweringContext<'_, 'hir> { } ExprKind::InlineAsm(ref asm) => self.lower_expr_asm(e.span, asm), ExprKind::LlvmInlineAsm(ref asm) => self.lower_expr_llvm_asm(asm), - ExprKind::Struct(ref path, ref fields, ref maybe_expr) => { - let maybe_expr = maybe_expr.as_ref().map(|x| self.lower_expr(x)); + ExprKind::Struct(ref path, ref fields, ref rest) => { + let rest = match rest { + StructRest::Base(e) => Some(self.lower_expr(e)), + StructRest::Rest(sp) => { + self.sess + .struct_span_err(*sp, "base expression required after `..`") + .span_label(*sp, "add a base expression here") + .emit(); + Some(&*self.arena.alloc(self.expr_err(*sp))) + } + StructRest::None => None, + }; hir::ExprKind::Struct( self.arena.alloc(self.lower_qpath( e.id, @@ -198,7 +208,7 @@ impl<'hir> LoweringContext<'_, 'hir> { ImplTraitContext::disallowed(), )), self.arena.alloc_from_iter(fields.iter().map(|x| self.lower_field(x))), - maybe_expr, + rest, ) } ExprKind::Yield(ref opt_expr) => self.lower_expr_yield(e.span, opt_expr.as_deref()), @@ -851,20 +861,22 @@ impl<'hir> LoweringContext<'_, 'hir> { whole_span: Span, ) -> hir::ExprKind<'hir> { // Return early in case of an ordinary assignment. - fn is_ordinary(lhs: &Expr) -> bool { + fn is_ordinary(lower_ctx: &mut LoweringContext<'_, '_>, lhs: &Expr) -> bool { match &lhs.kind { - ExprKind::Tup(..) => false, + ExprKind::Array(..) | ExprKind::Struct(..) | ExprKind::Tup(..) => false, + // Check for tuple struct constructor. + ExprKind::Call(callee, ..) => lower_ctx.extract_tuple_struct_path(callee).is_none(), ExprKind::Paren(e) => { match e.kind { // We special-case `(..)` for consistency with patterns. ExprKind::Range(None, None, RangeLimits::HalfOpen) => false, - _ => is_ordinary(e), + _ => is_ordinary(lower_ctx, e), } } _ => true, } } - if is_ordinary(lhs) { + if is_ordinary(self, lhs) { return hir::ExprKind::Assign(self.lower_expr(lhs), self.lower_expr(rhs), eq_sign_span); } if !self.sess.features_untracked().destructuring_assignment { @@ -902,6 +914,26 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ExprKind::Block(&self.block_all(whole_span, stmts, None), None) } + /// If the given expression is a path to a tuple struct, returns that path. + /// It is not a complete check, but just tries to reject most paths early + /// if they are not tuple structs. + /// Type checking will take care of the full validation later. + fn extract_tuple_struct_path<'a>(&mut self, expr: &'a Expr) -> Option<&'a Path> { + // For tuple struct destructuring, it must be a non-qualified path (like in patterns). + if let ExprKind::Path(None, path) = &expr.kind { + // Does the path resolves to something disallowed in a tuple struct/variant pattern? + if let Some(partial_res) = self.resolver.get_partial_res(expr.id) { + if partial_res.unresolved_segments() == 0 + && !partial_res.base_res().expected_in_tuple_struct_pat() + { + return None; + } + } + return Some(path); + } + None + } + /// Convert the LHS of a destructuring assignment to a pattern. /// Each sub-assignment is recorded in `assignments`. fn destructure_assign( @@ -911,6 +943,86 @@ impl<'hir> LoweringContext<'_, 'hir> { assignments: &mut Vec>, ) -> &'hir hir::Pat<'hir> { match &lhs.kind { + // Slice patterns. + ExprKind::Array(elements) => { + let (pats, rest) = + self.destructure_sequence(elements, "slice", eq_sign_span, assignments); + let slice_pat = if let Some((i, span)) = rest { + let (before, after) = pats.split_at(i); + hir::PatKind::Slice( + before, + Some(self.pat_without_dbm(span, hir::PatKind::Wild)), + after, + ) + } else { + hir::PatKind::Slice(pats, None, &[]) + }; + return self.pat_without_dbm(lhs.span, slice_pat); + } + // Tuple structs. + ExprKind::Call(callee, args) => { + if let Some(path) = self.extract_tuple_struct_path(callee) { + let (pats, rest) = self.destructure_sequence( + args, + "tuple struct or variant", + eq_sign_span, + assignments, + ); + let qpath = self.lower_qpath( + callee.id, + &None, + path, + ParamMode::Optional, + ImplTraitContext::disallowed(), + ); + // Destructure like a tuple struct. + let tuple_struct_pat = + hir::PatKind::TupleStruct(qpath, pats, rest.map(|r| r.0)); + return self.pat_without_dbm(lhs.span, tuple_struct_pat); + } + } + // Structs. + ExprKind::Struct(path, fields, rest) => { + let field_pats = self.arena.alloc_from_iter(fields.iter().map(|f| { + let pat = self.destructure_assign(&f.expr, eq_sign_span, assignments); + hir::FieldPat { + hir_id: self.next_id(), + ident: f.ident, + pat, + is_shorthand: f.is_shorthand, + span: f.span, + } + })); + let qpath = self.lower_qpath( + lhs.id, + &None, + path, + ParamMode::Optional, + ImplTraitContext::disallowed(), + ); + let fields_omitted = match rest { + StructRest::Base(e) => { + self.sess + .struct_span_err( + e.span, + "functional record updates are not allowed in destructuring \ + assignments", + ) + .span_suggestion( + e.span, + "consider removing the trailing pattern", + String::new(), + rustc_errors::Applicability::MachineApplicable, + ) + .emit(); + true + } + StructRest::Rest(_) => true, + StructRest::None => false, + }; + let struct_pat = hir::PatKind::Struct(qpath, field_pats, fields_omitted); + return self.pat_without_dbm(lhs.span, struct_pat); + } // Tuples. ExprKind::Tup(elements) => { let (pats, rest) = diff --git a/compiler/rustc_ast_passes/src/feature_gate.rs b/compiler/rustc_ast_passes/src/feature_gate.rs index f20084497671f..2831675cb3671 100644 --- a/compiler/rustc_ast_passes/src/feature_gate.rs +++ b/compiler/rustc_ast_passes/src/feature_gate.rs @@ -630,6 +630,7 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session) { gate_all!(const_trait_impl, "const trait impls are experimental"); gate_all!(half_open_range_patterns, "half-open range patterns are unstable"); gate_all!(inline_const, "inline-const is experimental"); + gate_all!(destructuring_assignment, "destructuring assignments are unstable"); // All uses of `gate_all!` below this point were added in #65742, // and subsequently disabled (with the non-early gating readded). diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index 9fcba90244330..17b3711b485ea 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -1729,7 +1729,7 @@ impl<'a> State<'a> { &mut self, path: &ast::Path, fields: &[ast::Field], - wth: &Option>, + rest: &ast::StructRest, attrs: &[ast::Attribute], ) { self.print_path(path, true, 0); @@ -1750,22 +1750,21 @@ impl<'a> State<'a> { }, |f| f.span, ); - match *wth { - Some(ref expr) => { + match rest { + ast::StructRest::Base(_) | ast::StructRest::Rest(_) => { self.ibox(INDENT_UNIT); if !fields.is_empty() { self.s.word(","); self.s.space(); } self.s.word(".."); - self.print_expr(expr); - self.end(); - } - _ => { - if !fields.is_empty() { - self.s.word(",") + if let ast::StructRest::Base(ref expr) = *rest { + self.print_expr(expr); } + self.end(); } + ast::StructRest::None if !fields.is_empty() => self.s.word(","), + _ => {} } self.s.word("}"); } @@ -1891,8 +1890,8 @@ impl<'a> State<'a> { ast::ExprKind::Repeat(ref element, ref count) => { self.print_expr_repeat(element, count, attrs); } - ast::ExprKind::Struct(ref path, ref fields, ref wth) => { - self.print_expr_struct(path, &fields[..], wth, attrs); + ast::ExprKind::Struct(ref path, ref fields, ref rest) => { + self.print_expr_struct(path, &fields[..], rest, attrs); } ast::ExprKind::Tup(ref exprs) => { self.print_expr_tup(&exprs[..], attrs); diff --git a/compiler/rustc_expand/src/build.rs b/compiler/rustc_expand/src/build.rs index 1c9bfb902d61a..30f0fc6cddfa2 100644 --- a/compiler/rustc_expand/src/build.rs +++ b/compiler/rustc_expand/src/build.rs @@ -298,7 +298,7 @@ impl<'a> ExtCtxt<'a> { path: ast::Path, fields: Vec, ) -> P { - self.expr(span, ast::ExprKind::Struct(path, fields, None)) + self.expr(span, ast::ExprKind::Struct(path, fields, ast::StructRest::None)) } pub fn expr_struct_ident( &self, diff --git a/compiler/rustc_hir/src/def.rs b/compiler/rustc_hir/src/def.rs index 193247af584bb..298cfcc254c86 100644 --- a/compiler/rustc_hir/src/def.rs +++ b/compiler/rustc_hir/src/def.rs @@ -484,4 +484,9 @@ impl Res { pub fn matches_ns(&self, ns: Namespace) -> bool { self.ns().map_or(true, |actual_ns| actual_ns == ns) } + + /// Returns whether such a resolved path can occur in a tuple struct/variant pattern + pub fn expected_in_tuple_struct_pat(&self) -> bool { + matches!(self, Res::Def(DefKind::Ctor(_, CtorKind::Fn), _) | Res::SelfCtor(..)) + } } diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index c2a13d4b0dec1..188bf227c4249 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -2087,7 +2087,7 @@ impl<'a> Parser<'a> { recover: bool, ) -> PResult<'a, P> { let mut fields = Vec::new(); - let mut base = None; + let mut base = ast::StructRest::None; let mut recover_async = false; attrs.extend(self.parse_inner_attributes()?); @@ -2102,8 +2102,14 @@ impl<'a> Parser<'a> { while self.token != token::CloseDelim(token::Brace) { if self.eat(&token::DotDot) { let exp_span = self.prev_token.span; + // We permit `.. }` on the left-hand side of a destructuring assignment. + if self.check(&token::CloseDelim(token::Brace)) { + self.sess.gated_spans.gate(sym::destructuring_assignment, self.prev_token.span); + base = ast::StructRest::Rest(self.prev_token.span.shrink_to_hi()); + break; + } match self.parse_expr() { - Ok(e) => base = Some(e), + Ok(e) => base = ast::StructRest::Base(e), Err(mut e) if recover => { e.emit(); self.recover_stmt(); diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 2337f0d09abb7..2149fd7e01184 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -298,9 +298,7 @@ impl<'a> PathSource<'a> { _, ) | Res::SelfCtor(..)), - PathSource::TupleStruct(..) => { - matches!(res, Res::Def(DefKind::Ctor(_, CtorKind::Fn), _) | Res::SelfCtor(..)) - } + PathSource::TupleStruct(..) => res.expected_in_tuple_struct_pat(), PathSource::Struct => matches!(res, Res::Def( DefKind::Struct | DefKind::Union diff --git a/compiler/rustc_save_analysis/src/dump_visitor.rs b/compiler/rustc_save_analysis/src/dump_visitor.rs index dbb5e3cc9f066..40d60a8394be3 100644 --- a/compiler/rustc_save_analysis/src/dump_visitor.rs +++ b/compiler/rustc_save_analysis/src/dump_visitor.rs @@ -816,7 +816,7 @@ impl<'tcx> DumpVisitor<'tcx> { path: &'tcx hir::QPath<'tcx>, fields: &'tcx [hir::Field<'tcx>], variant: &'tcx ty::VariantDef, - base: Option<&'tcx hir::Expr<'tcx>>, + rest: Option<&'tcx hir::Expr<'tcx>>, ) { if let Some(struct_lit_data) = self.save_ctxt.get_expr_data(ex) { if let hir::QPath::Resolved(_, path) = path { @@ -836,7 +836,9 @@ impl<'tcx> DumpVisitor<'tcx> { } } - walk_list!(self, visit_expr, base); + if let Some(base) = rest { + self.visit_expr(&base); + } } fn process_method_call( @@ -1399,7 +1401,7 @@ impl<'tcx> Visitor<'tcx> for DumpVisitor<'tcx> { debug!("visit_expr {:?}", ex.kind); self.process_macro_use(ex.span); match ex.kind { - hir::ExprKind::Struct(ref path, ref fields, ref base) => { + hir::ExprKind::Struct(ref path, ref fields, ref rest) => { let hir_expr = self.save_ctxt.tcx.hir().expect_expr(ex.hir_id); let adt = match self.save_ctxt.typeck_results().expr_ty_opt(&hir_expr) { Some(ty) if ty.ty_adt_def().is_some() => ty.ty_adt_def().unwrap(), @@ -1409,7 +1411,7 @@ impl<'tcx> Visitor<'tcx> for DumpVisitor<'tcx> { } }; let res = self.save_ctxt.get_path_res(hir_expr.hir_id); - self.process_struct_lit(ex, path, fields, adt.variant_of_res(res), *base) + self.process_struct_lit(ex, path, fields, adt.variant_of_res(res), *rest) } hir::ExprKind::MethodCall(ref seg, _, args, _) => { self.process_method_call(ex, seg, args) diff --git a/src/test/ui-fulldeps/pprust-expr-roundtrip.rs b/src/test/ui-fulldeps/pprust-expr-roundtrip.rs index caf55bec53ddd..bff92d8607ece 100644 --- a/src/test/ui-fulldeps/pprust-expr-roundtrip.rs +++ b/src/test/ui-fulldeps/pprust-expr-roundtrip.rs @@ -155,7 +155,7 @@ fn iter_exprs(depth: usize, f: &mut dyn FnMut(P)) { }, 17 => { let path = Path::from_ident(Ident::from_str("S")); - g(ExprKind::Struct(path, vec![], Some(make_x()))); + g(ExprKind::Struct(path, vec![], StructRest::Base(make_x()))); }, 18 => { iter_exprs(depth - 1, &mut |e| g(ExprKind::Try(e))); diff --git a/src/test/ui/destructuring-assignment/nested_destructure.rs b/src/test/ui/destructuring-assignment/nested_destructure.rs new file mode 100644 index 0000000000000..393dfc16c0a1c --- /dev/null +++ b/src/test/ui/destructuring-assignment/nested_destructure.rs @@ -0,0 +1,17 @@ +// run-pass + +#![feature(destructuring_assignment)] + +struct Struct { + a: S, + b: T, +} + +struct TupleStruct(S, T); + +fn main() { + let (a, b, c, d); + Struct { a: TupleStruct((a, b), c), b: [d] } = + Struct { a: TupleStruct((0, 1), 2), b: [3] }; + assert_eq!((a, b, c, d), (0, 1, 2, 3)); +} diff --git a/src/test/ui/destructuring-assignment/note-unsupported.rs b/src/test/ui/destructuring-assignment/note-unsupported.rs index e0cb9dc9158e2..249fba7f920bc 100644 --- a/src/test/ui/destructuring-assignment/note-unsupported.rs +++ b/src/test/ui/destructuring-assignment/note-unsupported.rs @@ -7,18 +7,19 @@ fn main() { (a, b) += (3, 4); //~ ERROR invalid left-hand side of assignment //~| ERROR binary assignment operation `+=` cannot be applied - [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 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 destructuring assignments are unstable S { x: a, y: b } += s; //~ ERROR invalid left-hand side of assignment //~| ERROR binary assignment operation `+=` cannot be applied S { x: a, ..s } = S { x: 3, y: 4 }; - //~^ ERROR invalid left-hand side of assignment + //~^ ERROR functional record updates are not allowed in destructuring assignments + //~| ERROR destructuring assignments are unstable let c = 3; diff --git a/src/test/ui/destructuring-assignment/note-unsupported.stderr b/src/test/ui/destructuring-assignment/note-unsupported.stderr index c5543fab825eb..a81324b99e586 100644 --- a/src/test/ui/destructuring-assignment/note-unsupported.stderr +++ b/src/test/ui/destructuring-assignment/note-unsupported.stderr @@ -10,7 +10,46 @@ LL | (a, b) = (3, 4); = help: add `#![feature(destructuring_assignment)]` to the crate attributes to enable error[E0658]: destructuring assignments are unstable - --> $DIR/note-unsupported.rs:25:17 + --> $DIR/note-unsupported.rs:10: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[E0658]: destructuring assignments are unstable + --> $DIR/note-unsupported.rs:16:22 + | +LL | S { x: a, y: b } = s; + | ---------------- ^ + | | + | 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/note-unsupported.rs:20:21 + | +LL | S { x: a, ..s } = S { x: 3, y: 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: functional record updates are not allowed in destructuring assignments + --> $DIR/note-unsupported.rs:20:17 + | +LL | S { x: a, ..s } = S { x: 3, y: 4 }; + | ^ help: consider removing the trailing pattern + +error[E0658]: destructuring assignments are unstable + --> $DIR/note-unsupported.rs:26:17 | LL | ((a, b), c) = ((3, 4), 5); | ----------- ^ @@ -36,14 +75,6 @@ LL | (a, b) += (3, 4); | | | cannot assign to this expression -error[E0070]: invalid left-hand side of assignment - --> $DIR/note-unsupported.rs:10:12 - | -LL | [a, b] = [3, 4]; - | ------ ^ - | | - | cannot assign to this expression - error[E0368]: binary assignment operation `+=` cannot be applied to type `[{integer}; 2]` --> $DIR/note-unsupported.rs:11:5 | @@ -60,14 +91,6 @@ LL | [a, b] += [3, 4]; | | | cannot assign to this expression -error[E0070]: invalid left-hand side of assignment - --> $DIR/note-unsupported.rs:16:22 - | -LL | S { x: a, y: b } = s; - | ---------------- ^ - | | - | cannot assign to this expression - error[E0368]: binary assignment operation `+=` cannot be applied to type `S` --> $DIR/note-unsupported.rs:17:5 | @@ -86,15 +109,7 @@ LL | S { x: a, y: b } += s; | | | cannot assign to this expression -error[E0070]: invalid left-hand side of assignment - --> $DIR/note-unsupported.rs:20:21 - | -LL | S { x: a, ..s } = S { x: 3, y: 4 }; - | --------------- ^ - | | - | cannot assign to this expression - -error: aborting due to 11 previous errors +error: aborting due to 12 previous errors -Some errors have detailed explanations: E0067, E0070, E0368, E0658. +Some errors have detailed explanations: E0067, E0368, E0658. For more information about an error, try `rustc --explain E0067`. diff --git a/src/test/ui/destructuring-assignment/slice_destructure.rs b/src/test/ui/destructuring-assignment/slice_destructure.rs new file mode 100644 index 0000000000000..3dd10aff19c72 --- /dev/null +++ b/src/test/ui/destructuring-assignment/slice_destructure.rs @@ -0,0 +1,15 @@ +// run-pass + +#![feature(destructuring_assignment)] + +fn main() { + let (mut a, mut b); + [a, b] = [0, 1]; + assert_eq!((a, b), (0, 1)); + let mut c; + [a, .., b, c] = [1, 2, 3, 4, 5]; + assert_eq!((a, b, c), (1, 4, 5)); + [..] = [1, 2, 3]; + [c, ..] = [5, 6, 6]; + assert_eq!(c, 5); +} diff --git a/src/test/ui/destructuring-assignment/slice_destructure_fail.rs b/src/test/ui/destructuring-assignment/slice_destructure_fail.rs new file mode 100644 index 0000000000000..f636ea3511c26 --- /dev/null +++ b/src/test/ui/destructuring-assignment/slice_destructure_fail.rs @@ -0,0 +1,7 @@ +#![feature(destructuring_assignment)] + +fn main() { + let (mut a, mut b); + [a, .., b, ..] = [0, 1]; //~ ERROR `..` can only be used once per slice pattern + [a, a, b] = [1, 2]; //~ ERROR pattern requires 3 elements but array has 2 +} diff --git a/src/test/ui/destructuring-assignment/slice_destructure_fail.stderr b/src/test/ui/destructuring-assignment/slice_destructure_fail.stderr new file mode 100644 index 0000000000000..728687deb8bbb --- /dev/null +++ b/src/test/ui/destructuring-assignment/slice_destructure_fail.stderr @@ -0,0 +1,17 @@ +error: `..` can only be used once per slice pattern + --> $DIR/slice_destructure_fail.rs:5:14 + | +LL | [a, .., b, ..] = [0, 1]; + | -- ^^ can only be used once per slice pattern + | | + | previously used here + +error[E0527]: pattern requires 3 elements but array has 2 + --> $DIR/slice_destructure_fail.rs:6:3 + | +LL | [a, a, b] = [1, 2]; + | ^^^^^^^^^ expected 2 elements + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0527`. diff --git a/src/test/ui/destructuring-assignment/struct_destructure.rs b/src/test/ui/destructuring-assignment/struct_destructure.rs new file mode 100644 index 0000000000000..b3a96ee157346 --- /dev/null +++ b/src/test/ui/destructuring-assignment/struct_destructure.rs @@ -0,0 +1,19 @@ +// run-pass + +#![feature(destructuring_assignment)] +struct Struct { + a: S, + b: T, +} + +fn main() { + let (mut a, mut b); + Struct { a, b } = Struct { a: 0, b: 1 }; + assert_eq!((a, b), (0, 1)); + Struct { a: b, b: a } = Struct { a: 1, b: 2 }; + assert_eq!((a,b), (2, 1)); + Struct { a, .. } = Struct { a: 1, b: 3 }; + assert_eq!((a, b), (1, 1)); + Struct { .. } = Struct { a: 1, b: 4 }; + assert_eq!((a, b), (1, 1)); +} diff --git a/src/test/ui/destructuring-assignment/struct_destructure_fail.rs b/src/test/ui/destructuring-assignment/struct_destructure_fail.rs new file mode 100644 index 0000000000000..c22695ed38849 --- /dev/null +++ b/src/test/ui/destructuring-assignment/struct_destructure_fail.rs @@ -0,0 +1,15 @@ +#![feature(destructuring_assignment)] +struct Struct { + a: S, + b: T, +} + +fn main() { + let (mut a, b); + let mut c; + let d = Struct { a: 0, b: 1 }; + Struct { a, b, c } = Struct { a: 0, b: 1 }; //~ ERROR does not have a field named `c` + Struct { a, ..d } = Struct { a: 1, b: 2 }; + //~^ ERROR functional record updates are not allowed in destructuring assignments + Struct { a, .. }; //~ ERROR base expression required after `..` +} diff --git a/src/test/ui/destructuring-assignment/struct_destructure_fail.stderr b/src/test/ui/destructuring-assignment/struct_destructure_fail.stderr new file mode 100644 index 0000000000000..4da4698804f1a --- /dev/null +++ b/src/test/ui/destructuring-assignment/struct_destructure_fail.stderr @@ -0,0 +1,21 @@ +error: functional record updates are not allowed in destructuring assignments + --> $DIR/struct_destructure_fail.rs:12:19 + | +LL | Struct { a, ..d } = Struct { a: 1, b: 2 }; + | ^ help: consider removing the trailing pattern + +error: base expression required after `..` + --> $DIR/struct_destructure_fail.rs:14:19 + | +LL | Struct { a, .. }; + | ^ add a base expression here + +error[E0026]: struct `Struct` does not have a field named `c` + --> $DIR/struct_destructure_fail.rs:11:20 + | +LL | Struct { a, b, c } = Struct { a: 0, b: 1 }; + | ^ struct `Struct` does not have this field + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0026`. diff --git a/src/test/ui/destructuring-assignment/tuple_struct_destructure.rs b/src/test/ui/destructuring-assignment/tuple_struct_destructure.rs new file mode 100644 index 0000000000000..106a9b16db459 --- /dev/null +++ b/src/test/ui/destructuring-assignment/tuple_struct_destructure.rs @@ -0,0 +1,34 @@ +// run-pass + +#![feature(destructuring_assignment)] + +struct TupleStruct(S, T); + +impl TupleStruct { + fn assign(self, first: &mut S, second: &mut T) { + // Test usage of `Self` instead of the struct name: + Self(*first, *second) = self + } +} + +enum Enum { + SingleVariant(S, T) +} + +type Alias = Enum; + +fn main() { + let (mut a, mut b); + TupleStruct(a, b) = TupleStruct(0, 1); + assert_eq!((a, b), (0, 1)); + TupleStruct(a, .., b) = TupleStruct(1, 2); + assert_eq!((a, b), (1, 2)); + TupleStruct(..) = TupleStruct(3, 4); + assert_eq!((a, b), (1, 2)); + TupleStruct(5,6).assign(&mut a, &mut b); + assert_eq!((a, b), (5, 6)); + Enum::SingleVariant(a, b) = Enum::SingleVariant(7, 8); + assert_eq!((a, b), (7, 8)); + Alias::SingleVariant(a, b) = Alias::SingleVariant(9, 10); + assert_eq!((a, b), (9, 10)); +} diff --git a/src/test/ui/destructuring-assignment/tuple_struct_destructure_fail.rs b/src/test/ui/destructuring-assignment/tuple_struct_destructure_fail.rs new file mode 100644 index 0000000000000..61ae42a51751f --- /dev/null +++ b/src/test/ui/destructuring-assignment/tuple_struct_destructure_fail.rs @@ -0,0 +1,42 @@ +#![feature(destructuring_assignment)] + +struct TupleStruct(S, T); + +enum Enum { + SingleVariant(S, T) +} + +type Alias = Enum; + +trait Test { + fn test() -> TupleStruct { + TupleStruct(0, 0) + } +} + +impl Test for Alias {} + +fn test() -> TupleStruct { + TupleStruct(0, 0) +} + +fn main() { + let (mut a, mut b); + TupleStruct(a, .., b, ..) = TupleStruct(0, 1); + //~^ ERROR `..` can only be used once per tuple struct or variant pattern + Enum::SingleVariant(a, .., b, ..) = Enum::SingleVariant(0, 1); + //~^ ERROR `..` can only be used once per tuple struct or variant pattern + + TupleStruct(a, a, b) = TupleStruct(1, 2); + //~^ ERROR this pattern has 3 fields, but the corresponding tuple struct has 2 fields + Enum::SingleVariant(a, a, b) = Enum::SingleVariant(1, 2); + //~^ ERROR this pattern has 3 fields, but the corresponding tuple variant has 2 fields + + // Check if `test` is recognized as not a tuple struct but a function call: + test() = TupleStruct(0, 0); + //~^ ERROR invalid left-hand side of assignment + (test)() = TupleStruct(0, 0); + //~^ ERROR invalid left-hand side of assignment + as Test>::test() = TupleStruct(0, 0); + //~^ ERROR invalid left-hand side of assignment +} diff --git a/src/test/ui/destructuring-assignment/tuple_struct_destructure_fail.stderr b/src/test/ui/destructuring-assignment/tuple_struct_destructure_fail.stderr new file mode 100644 index 0000000000000..863eedecf7697 --- /dev/null +++ b/src/test/ui/destructuring-assignment/tuple_struct_destructure_fail.stderr @@ -0,0 +1,62 @@ +error: `..` can only be used once per tuple struct or variant pattern + --> $DIR/tuple_struct_destructure_fail.rs:25:27 + | +LL | TupleStruct(a, .., b, ..) = TupleStruct(0, 1); + | -- ^^ can only be used once per tuple struct or variant pattern + | | + | previously used here + +error: `..` can only be used once per tuple struct or variant pattern + --> $DIR/tuple_struct_destructure_fail.rs:27:35 + | +LL | Enum::SingleVariant(a, .., b, ..) = Enum::SingleVariant(0, 1); + | -- ^^ can only be used once per tuple struct or variant pattern + | | + | previously used here + +error[E0023]: this pattern has 3 fields, but the corresponding tuple struct has 2 fields + --> $DIR/tuple_struct_destructure_fail.rs:30:5 + | +LL | struct TupleStruct(S, T); + | ------------------------------- tuple struct defined here +... +LL | TupleStruct(a, a, b) = TupleStruct(1, 2); + | ^^^^^^^^^^^^^^^^^^^^ expected 2 fields, found 3 + +error[E0023]: this pattern has 3 fields, but the corresponding tuple variant has 2 fields + --> $DIR/tuple_struct_destructure_fail.rs:32:5 + | +LL | SingleVariant(S, T) + | ------------------- tuple variant defined here +... +LL | Enum::SingleVariant(a, a, b) = Enum::SingleVariant(1, 2); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected 2 fields, found 3 + +error[E0070]: invalid left-hand side of assignment + --> $DIR/tuple_struct_destructure_fail.rs:36:12 + | +LL | test() = TupleStruct(0, 0); + | ------ ^ + | | + | cannot assign to this expression + +error[E0070]: invalid left-hand side of assignment + --> $DIR/tuple_struct_destructure_fail.rs:38:14 + | +LL | (test)() = TupleStruct(0, 0); + | -------- ^ + | | + | cannot assign to this expression + +error[E0070]: invalid left-hand side of assignment + --> $DIR/tuple_struct_destructure_fail.rs:40:38 + | +LL | as Test>::test() = TupleStruct(0, 0); + | -------------------------------- ^ + | | + | cannot assign to this expression + +error: aborting due to 7 previous errors + +Some errors have detailed explanations: E0023, E0070. +For more information about an error, try `rustc --explain E0023`. diff --git a/src/test/ui/destructuring-assignment/underscore-range-expr-gating.rs b/src/test/ui/destructuring-assignment/underscore-range-expr-gating.rs new file mode 100644 index 0000000000000..b41f2f52a3d6f --- /dev/null +++ b/src/test/ui/destructuring-assignment/underscore-range-expr-gating.rs @@ -0,0 +1,8 @@ +fn main() {} + +struct S { x : u32 } + +#[cfg(FALSE)] +fn foo() { + S { x: 5, .. }; //~ ERROR destructuring assignments are unstable +} diff --git a/src/test/ui/destructuring-assignment/underscore-range-expr-gating.stderr b/src/test/ui/destructuring-assignment/underscore-range-expr-gating.stderr new file mode 100644 index 0000000000000..442e36cd3065e --- /dev/null +++ b/src/test/ui/destructuring-assignment/underscore-range-expr-gating.stderr @@ -0,0 +1,12 @@ +error[E0658]: destructuring assignments are unstable + --> $DIR/underscore-range-expr-gating.rs:7:15 + | +LL | S { x: 5, .. }; + | ^^ + | + = 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`. diff --git a/src/test/ui/issues/issue-77218.rs b/src/test/ui/issues/issue-77218.rs index bc992c21dca5c..a6a2401795ff4 100644 --- a/src/test/ui/issues/issue-77218.rs +++ b/src/test/ui/issues/issue-77218.rs @@ -1,7 +1,11 @@ fn main() { let value = [7u8]; - while Some(0) = value.get(0) { //~ ERROR mismatched types - //~^ NOTE expected `bool`, found `()` - //~| HELP you might have meant to use pattern matching + while Some(0) = value.get(0) { //~ ERROR destructuring assignments are unstable + //~| ERROR invalid left-hand side of assignment + //~| ERROR mismatched types + //~| ERROR mismatched types + + // FIXME The following diagnostic should also be emitted + // HELP you might have meant to use pattern matching } } diff --git a/src/test/ui/issues/issue-77218.stderr b/src/test/ui/issues/issue-77218.stderr index eca44725eb258..4f6fbaa2265d5 100644 --- a/src/test/ui/issues/issue-77218.stderr +++ b/src/test/ui/issues/issue-77218.stderr @@ -1,14 +1,38 @@ +error[E0658]: destructuring assignments are unstable + --> $DIR/issue-77218.rs:3:19 + | +LL | while Some(0) = value.get(0) { + | ------- ^ + | | + | 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/issue-77218.rs:3:19 + | +LL | while Some(0) = value.get(0) { + | - ^ + | | + | cannot assign to this expression + +error[E0308]: mismatched types + --> $DIR/issue-77218.rs:3:16 + | +LL | while Some(0) = value.get(0) { + | ^ + | | + | expected integer, found `&u8` + | help: consider dereferencing the borrow: `*0` + error[E0308]: mismatched types --> $DIR/issue-77218.rs:3:11 | LL | while Some(0) = value.get(0) { | ^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `()` - | -help: you might have meant to use pattern matching - | -LL | while let Some(0) = value.get(0) { - | ^^^ -error: aborting due to previous error +error: aborting due to 4 previous errors -For more information about this error, try `rustc --explain E0308`. +Some errors have detailed explanations: E0070, E0308, E0658. +For more information about an error, try `rustc --explain E0070`. diff --git a/src/test/ui/suggestions/if-let-typo.rs b/src/test/ui/suggestions/if-let-typo.rs index 87def13c476c7..688b6e8265826 100644 --- a/src/test/ui/suggestions/if-let-typo.rs +++ b/src/test/ui/suggestions/if-let-typo.rs @@ -2,7 +2,12 @@ fn main() { let foo = Some(0); let bar = None; if Some(x) = foo {} //~ ERROR cannot find value `x` in this scope + //~^ ERROR mismatched types + //~^^ ERROR destructuring assignments are unstable if Some(foo) = bar {} //~ ERROR mismatched types + //~^ ERROR destructuring assignments are unstable if 3 = foo {} //~ ERROR mismatched types if Some(3) = foo {} //~ ERROR mismatched types + //~^ ERROR destructuring assignments are unstable + //~^^ ERROR invalid left-hand side of assignment } diff --git a/src/test/ui/suggestions/if-let-typo.stderr b/src/test/ui/suggestions/if-let-typo.stderr index d8e50cae55ad1..ce1ee0cd06d48 100644 --- a/src/test/ui/suggestions/if-let-typo.stderr +++ b/src/test/ui/suggestions/if-let-typo.stderr @@ -9,23 +9,53 @@ help: you might have meant to use pattern matching LL | if let Some(x) = foo {} | ^^^ -error[E0308]: mismatched types - --> $DIR/if-let-typo.rs:5:8 +error[E0658]: destructuring assignments are unstable + --> $DIR/if-let-typo.rs:4:16 + | +LL | if Some(x) = foo {} + | ------- ^ + | | + | 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/if-let-typo.rs:7:18 | LL | if Some(foo) = bar {} - | ^^^^^^^^^^^^^^^ expected `bool`, found `()` + | --------- ^ + | | + | cannot assign to this expression | -help: you might have meant to use pattern matching + = 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/if-let-typo.rs:10:16 | -LL | if let Some(foo) = bar {} - | ^^^ -help: you might have meant to compare for equality +LL | if Some(3) = foo {} + | ------- ^ + | | + | cannot assign to this expression + | + = note: see issue #71126 for more information + = help: add `#![feature(destructuring_assignment)]` to the crate attributes to enable + +error[E0308]: mismatched types + --> $DIR/if-let-typo.rs:4:8 + | +LL | if Some(x) = foo {} + | ^^^^^^^^^^^^^ expected `bool`, found `()` + +error[E0308]: mismatched types + --> $DIR/if-let-typo.rs:7:8 | -LL | if Some(foo) == bar {} - | ^^ +LL | if Some(foo) = bar {} + | ^^^^^^^^^^^^^^^ expected `bool`, found `()` error[E0308]: mismatched types - --> $DIR/if-let-typo.rs:6:8 + --> $DIR/if-let-typo.rs:9:8 | LL | if 3 = foo {} | ^^^^^^^ expected `bool`, found `()` @@ -35,22 +65,21 @@ help: you might have meant to use pattern matching LL | if let 3 = foo {} | ^^^ +error[E0070]: invalid left-hand side of assignment + --> $DIR/if-let-typo.rs:10:16 + | +LL | if Some(3) = foo {} + | - ^ + | | + | cannot assign to this expression + error[E0308]: mismatched types - --> $DIR/if-let-typo.rs:7:8 + --> $DIR/if-let-typo.rs:10:8 | LL | if Some(3) = foo {} | ^^^^^^^^^^^^^ expected `bool`, found `()` - | -help: you might have meant to use pattern matching - | -LL | if let Some(3) = foo {} - | ^^^ -help: you might have meant to compare for equality - | -LL | if Some(3) == foo {} - | ^^ -error: aborting due to 4 previous errors +error: aborting due to 9 previous errors -Some errors have detailed explanations: E0308, E0425. -For more information about an error, try `rustc --explain E0308`. +Some errors have detailed explanations: E0070, E0308, E0425, E0658. +For more information about an error, try `rustc --explain E0070`. diff --git a/src/tools/clippy/clippy_lints/src/utils/ast_utils.rs b/src/tools/clippy/clippy_lints/src/utils/ast_utils.rs index 0e9feef3746e7..7b65e664867ff 100644 --- a/src/tools/clippy/clippy_lints/src/utils/ast_utils.rs +++ b/src/tools/clippy/clippy_lints/src/utils/ast_utils.rs @@ -107,6 +107,15 @@ pub fn eq_expr_opt(l: &Option>, r: &Option>) -> bool { both(l, r, |l, r| eq_expr(l, r)) } +pub fn eq_struct_rest(l: &StructRest, r: &StructRest) -> bool { + match (l, r) { + (StructRest::Base(lb), StructRest::Base(rb)) => eq_expr(lb, rb), + (StructRest::Rest(_), StructRest::Rest(_)) => true, + (StructRest::None, StructRest::None) => true, + _ => false, + } +} + pub fn eq_expr(l: &Expr, r: &Expr) -> bool { use ExprKind::*; if !over(&l.attrs, &r.attrs, |l, r| eq_attr(l, r)) { @@ -150,7 +159,7 @@ pub fn eq_expr(l: &Expr, r: &Expr) -> bool { (Path(lq, lp), Path(rq, rp)) => both(lq, rq, |l, r| eq_qself(l, r)) && eq_path(lp, rp), (MacCall(l), MacCall(r)) => eq_mac_call(l, r), (Struct(lp, lfs, lb), Struct(rp, rfs, rb)) => { - eq_path(lp, rp) && eq_expr_opt(lb, rb) && unordered_over(lfs, rfs, |l, r| eq_field(l, r)) + eq_path(lp, rp) && eq_struct_rest(lb, rb) && unordered_over(lfs, rfs, |l, r| eq_field(l, r)) }, _ => false, } diff --git a/src/tools/clippy/tests/ui/crashes/ice-6250.stderr b/src/tools/clippy/tests/ui/crashes/ice-6250.stderr index 8241dcd8feb7b..c38727316cd4e 100644 --- a/src/tools/clippy/tests/ui/crashes/ice-6250.stderr +++ b/src/tools/clippy/tests/ui/crashes/ice-6250.stderr @@ -1,3 +1,14 @@ +error[E0658]: destructuring assignments are unstable + --> $DIR/ice-6250.rs:12:25 + | +LL | Some(reference) = cache.data.get(key) { + | --------------- ^ + | | + | cannot assign to this expression + | + = note: see issue #71126 for more information + = help: add `#![feature(destructuring_assignment)]` to the crate attributes to enable + error[E0601]: `main` function not found in crate `ice_6250` --> $DIR/ice-6250.rs:4:1 | @@ -10,18 +21,22 @@ LL | | } LL | | } | |_^ consider adding a `main` function to `$DIR/ice-6250.rs` +error[E0308]: mismatched types + --> $DIR/ice-6250.rs:12:14 + | +LL | Some(reference) = cache.data.get(key) { + | ^^^^^^^^^ + | | + | expected integer, found `&i32` + | help: consider dereferencing the borrow: `*reference` + error[E0308]: mismatched types --> $DIR/ice-6250.rs:12:9 | LL | Some(reference) = cache.data.get(key) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `()` - | -help: you might have meant to use pattern matching - | -LL | let Some(reference) = cache.data.get(key) { - | ^^^ -error: aborting due to 2 previous errors +error: aborting due to 4 previous errors -Some errors have detailed explanations: E0308, E0601. +Some errors have detailed explanations: E0308, E0601, E0658. For more information about an error, try `rustc --explain E0308`. From 2453ce717ae8a9cd2f2c47f31cff596a75e5b486 Mon Sep 17 00:00:00 2001 From: dalance Date: Wed, 11 Nov 2020 23:34:01 +0900 Subject: [PATCH 42/42] Ship llvm-cov through llvm-tools --- src/bootstrap/lib.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 3d111839dc725..d08143e907c4a 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -169,17 +169,18 @@ pub use crate::config::Config; pub use crate::flags::Subcommand; const LLVM_TOOLS: &[&str] = &[ - "llvm-nm", // used to inspect binaries; it shows symbol names, their sizes and visibility - "llvm-objcopy", // used to transform ELFs into binary format which flashing tools consume - "llvm-objdump", // used to disassemble programs + "llvm-cov", // used to generate coverage report + "llvm-nm", // used to inspect binaries; it shows symbol names, their sizes and visibility + "llvm-objcopy", // used to transform ELFs into binary format which flashing tools consume + "llvm-objdump", // used to disassemble programs "llvm-profdata", // used to inspect and merge files generated by profiles - "llvm-readobj", // used to get information from ELFs/objects that the other tools don't provide - "llvm-size", // used to prints the size of the linker sections of a program - "llvm-strip", // used to discard symbols from binary files to reduce their size - "llvm-ar", // used for creating and modifying archive files - "llvm-dis", // used to disassemble LLVM bitcode - "llc", // used to compile LLVM bytecode - "opt", // used to optimize LLVM bytecode + "llvm-readobj", // used to get information from ELFs/objects that the other tools don't provide + "llvm-size", // used to prints the size of the linker sections of a program + "llvm-strip", // used to discard symbols from binary files to reduce their size + "llvm-ar", // used for creating and modifying archive files + "llvm-dis", // used to disassemble LLVM bitcode + "llc", // used to compile LLVM bytecode + "opt", // used to optimize LLVM bytecode ]; pub const VERSION: usize = 2;