From 98fe2eb9aface6f97cae9612d55f6a32812165e4 Mon Sep 17 00:00:00 2001 From: pierwill Date: Fri, 4 Mar 2022 10:52:05 -0600 Subject: [PATCH 1/5] Remove ordering traits from `rustc_span::hygiene::LocalExpnId` --- compiler/rustc_span/src/hygiene.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/compiler/rustc_span/src/hygiene.rs b/compiler/rustc_span/src/hygiene.rs index 5fae46d5fd8..86adfa7a18c 100644 --- a/compiler/rustc_span/src/hygiene.rs +++ b/compiler/rustc_span/src/hygiene.rs @@ -85,10 +85,17 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { /// A unique ID associated with a macro invocation and expansion. pub struct LocalExpnId { ENCODABLE = custom + ORD_IMPL = custom DEBUG_FORMAT = "expn{}" } } +// To ensure correctness of incremental compilation, +// `LocalExpnId` must not implement `Ord` or `PartialOrd`. +// See https://github.com/rust-lang/rust/issues/90317. +impl !Ord for LocalExpnId {} +impl !PartialOrd for LocalExpnId {} + /// Assert that the provided `HashStableContext` is configured with the 'default' /// `HashingControls`. We should always have bailed out before getting to here /// with a non-default mode. With this check in place, we can avoid the need From 956659e5ced806360b726c103eb7ab1d960c163a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 6 Mar 2022 19:09:22 -0500 Subject: [PATCH 2/5] move saturating_add/sub into (pub) helper method --- .../src/interpret/intrinsics.rs | 87 ++++++++++--------- 1 file changed, 48 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index bf7e811c76f..6578db04c07 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -219,48 +219,11 @@ pub fn emulate_intrinsic( sym::saturating_add | sym::saturating_sub => { let l = self.read_immediate(&args[0])?; let r = self.read_immediate(&args[1])?; - let is_add = intrinsic_name == sym::saturating_add; - let (val, overflowed, _ty) = self.overflowing_binary_op( - if is_add { BinOp::Add } else { BinOp::Sub }, + let val = self.saturating_arith( + if intrinsic_name == sym::saturating_add { BinOp::Add } else { BinOp::Sub }, &l, &r, )?; - let val = if overflowed { - let size = l.layout.size; - let num_bits = size.bits(); - if l.layout.abi.is_signed() { - // For signed ints the saturated value depends on the sign of the first - // term since the sign of the second term can be inferred from this and - // the fact that the operation has overflowed (if either is 0 no - // overflow can occur) - let first_term: u128 = l.to_scalar()?.to_bits(l.layout.size)?; - let first_term_positive = first_term & (1 << (num_bits - 1)) == 0; - if first_term_positive { - // Negative overflow not possible since the positive first term - // can only increase an (in range) negative term for addition - // or corresponding negated positive term for subtraction - Scalar::from_uint( - (1u128 << (num_bits - 1)) - 1, // max positive - Size::from_bits(num_bits), - ) - } else { - // Positive overflow not possible for similar reason - // max negative - Scalar::from_uint(1u128 << (num_bits - 1), Size::from_bits(num_bits)) - } - } else { - // unsigned - if is_add { - // max unsigned - Scalar::from_uint(size.unsigned_int_max(), Size::from_bits(num_bits)) - } else { - // underflow to 0 - Scalar::from_uint(0u128, Size::from_bits(num_bits)) - } - } - } else { - val - }; self.write_scalar(val, dest)?; } sym::discriminant_value => { @@ -508,6 +471,52 @@ pub fn exact_div( self.binop_ignore_overflow(BinOp::Div, &a, &b, dest) } + pub fn saturating_arith( + &self, + mir_op: BinOp, + l: &ImmTy<'tcx, M::PointerTag>, + r: &ImmTy<'tcx, M::PointerTag>, + ) -> InterpResult<'tcx, Scalar> { + assert!(matches!(mir_op, BinOp::Add | BinOp::Sub)); + let (val, overflowed, _ty) = self.overflowing_binary_op(mir_op, l, r)?; + Ok(if overflowed { + let size = l.layout.size; + let num_bits = size.bits(); + if l.layout.abi.is_signed() { + // For signed ints the saturated value depends on the sign of the first + // term since the sign of the second term can be inferred from this and + // the fact that the operation has overflowed (if either is 0 no + // overflow can occur) + let first_term: u128 = l.to_scalar()?.to_bits(l.layout.size)?; + let first_term_positive = first_term & (1 << (num_bits - 1)) == 0; + if first_term_positive { + // Negative overflow not possible since the positive first term + // can only increase an (in range) negative term for addition + // or corresponding negated positive term for subtraction + Scalar::from_uint( + (1u128 << (num_bits - 1)) - 1, // max positive + Size::from_bits(num_bits), + ) + } else { + // Positive overflow not possible for similar reason + // max negative + Scalar::from_uint(1u128 << (num_bits - 1), Size::from_bits(num_bits)) + } + } else { + // unsigned + if matches!(mir_op, BinOp::Add) { + // max unsigned + Scalar::from_uint(size.unsigned_int_max(), Size::from_bits(num_bits)) + } else { + // underflow to 0 + Scalar::from_uint(0u128, Size::from_bits(num_bits)) + } + } + } else { + val + }) + } + /// Offsets a pointer by some multiple of its type, returning an error if the pointer leaves its /// allocation. For integer pointers, we consider each of them their own tiny allocation of size /// 0, so offset-by-0 (and only 0) is okay -- except that null cannot be offset by _any_ value. From ac844986d8f5646b686e2a5619e5a90372953b38 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 6 Mar 2022 19:11:31 -0500 Subject: [PATCH 3/5] use singed_int_max/min helper methods --- compiler/rustc_const_eval/src/interpret/intrinsics.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index 6578db04c07..63c0248993f 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -493,23 +493,20 @@ pub fn saturating_arith( // Negative overflow not possible since the positive first term // can only increase an (in range) negative term for addition // or corresponding negated positive term for subtraction - Scalar::from_uint( - (1u128 << (num_bits - 1)) - 1, // max positive - Size::from_bits(num_bits), - ) + Scalar::from_int(size.signed_int_max(), size) } else { // Positive overflow not possible for similar reason // max negative - Scalar::from_uint(1u128 << (num_bits - 1), Size::from_bits(num_bits)) + Scalar::from_int(size.signed_int_min(), size) } } else { // unsigned if matches!(mir_op, BinOp::Add) { // max unsigned - Scalar::from_uint(size.unsigned_int_max(), Size::from_bits(num_bits)) + Scalar::from_uint(size.unsigned_int_max(), size) } else { // underflow to 0 - Scalar::from_uint(0u128, Size::from_bits(num_bits)) + Scalar::from_uint(0u128, size) } } } else { From 5ddaa2d5e53344ff8b88d98e61e3dd36edb803e9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 6 Mar 2022 17:21:39 -0800 Subject: [PATCH 4/5] Erase regions when checking for missing Copy predicates --- .../src/diagnostics/conflict_errors.rs | 12 ++++++++++-- .../ui/borrowck/copy-suggestion-region-vid.rs | 17 +++++++++++++++++ .../borrowck/copy-suggestion-region-vid.stderr | 14 ++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/borrowck/copy-suggestion-region-vid.rs create mode 100644 src/test/ui/borrowck/copy-suggestion-region-vid.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 684a3ced5a0..2a74e1ce8b1 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -448,8 +448,16 @@ pub(crate) fn report_use_of_moved_or_uninitialized( self.mir_hir_id(), rustc_infer::traits::ObligationCauseCode::MiscObligation, ); - fulfill_cx.register_bound(&infcx, self.param_env, ty, copy_did, cause); - let errors = fulfill_cx.select_where_possible(&infcx); + fulfill_cx.register_bound( + &infcx, + self.param_env, + // Erase any region vids from the type, which may not be resolved + infcx.tcx.erase_regions(ty), + copy_did, + cause, + ); + // Select all, including ambiguous predicates + let errors = fulfill_cx.select_all_or_error(&infcx); // Only emit suggestion if all required predicates are on generic errors diff --git a/src/test/ui/borrowck/copy-suggestion-region-vid.rs b/src/test/ui/borrowck/copy-suggestion-region-vid.rs new file mode 100644 index 00000000000..dff95283459 --- /dev/null +++ b/src/test/ui/borrowck/copy-suggestion-region-vid.rs @@ -0,0 +1,17 @@ +pub struct DataStruct(); + +pub struct HelperStruct<'n> { + pub helpers: [Vec<&'n i64>; 2], + pub is_empty: bool, +} + +impl DataStruct { + pub fn f(&self) -> HelperStruct { + let helpers = [vec![], vec![]]; + + HelperStruct { helpers, is_empty: helpers[0].is_empty() } + //~^ ERROR borrow of moved value + } +} + +fn main() {} diff --git a/src/test/ui/borrowck/copy-suggestion-region-vid.stderr b/src/test/ui/borrowck/copy-suggestion-region-vid.stderr new file mode 100644 index 00000000000..f03cdd84d75 --- /dev/null +++ b/src/test/ui/borrowck/copy-suggestion-region-vid.stderr @@ -0,0 +1,14 @@ +error[E0382]: borrow of moved value: `helpers` + --> $DIR/copy-suggestion-region-vid.rs:12:43 + | +LL | let helpers = [vec![], vec![]]; + | ------- move occurs because `helpers` has type `[Vec<&i64>; 2]`, which does not implement the `Copy` trait +LL | +LL | HelperStruct { helpers, is_empty: helpers[0].is_empty() } + | ------- ^^^^^^^^^^^^^^^^^^^^^ value borrowed here after move + | | + | value moved here + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0382`. From dad81b65db1b67be7f24b2a418c66db10aecd4b5 Mon Sep 17 00:00:00 2001 From: lcnr Date: Thu, 3 Mar 2022 11:57:14 +0100 Subject: [PATCH 5/5] add tests for #94502 --- src/test/ui/nll/lint-no-err.rs | 28 ++++++++++++++++++++++++++++ src/test/ui/nll/lint-no-err.stderr | 17 +++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 src/test/ui/nll/lint-no-err.rs create mode 100644 src/test/ui/nll/lint-no-err.stderr diff --git a/src/test/ui/nll/lint-no-err.rs b/src/test/ui/nll/lint-no-err.rs new file mode 100644 index 00000000000..4b4169faadf --- /dev/null +++ b/src/test/ui/nll/lint-no-err.rs @@ -0,0 +1,28 @@ +// check-pass + +// mir borrowck previously incorrectly set `tainted_by_errors` +// when buffering lints, which resulted in ICE later on, +// see #94502. + +// Errors with `nll` which is already tested in enough other tests, +// so we ignore it here. +// +// ignore-compare-mode-nll + +struct Repro; +impl Repro { + fn get(&self) -> &i32 { + &3 + } + + fn insert(&mut self, _: i32) {} +} + +fn main() { + let x = &0; + let mut conflict = Repro; + let prev = conflict.get(); + conflict.insert(*prev + *x); + //~^ WARN cannot borrow `conflict` as mutable because it is also borrowed as immutable + //~| WARN this borrowing pattern was not meant to be accepted +} diff --git a/src/test/ui/nll/lint-no-err.stderr b/src/test/ui/nll/lint-no-err.stderr new file mode 100644 index 00000000000..1e7aecfaa64 --- /dev/null +++ b/src/test/ui/nll/lint-no-err.stderr @@ -0,0 +1,17 @@ +warning: cannot borrow `conflict` as mutable because it is also borrowed as immutable + --> $DIR/lint-no-err.rs:25:5 + | +LL | let prev = conflict.get(); + | -------------- immutable borrow occurs here +LL | conflict.insert(*prev + *x); + | ^^^^^^^^^^^^^^^^-----^^^^^^ + | | | + | | immutable borrow later used here + | mutable borrow occurs here + | + = note: `#[warn(mutable_borrow_reservation_conflict)]` on by default + = warning: this borrowing pattern was not meant to be accepted, and may become a hard error in the future + = note: for more information, see issue #59159 + +warning: 1 warning emitted +