Skip to content

Commit

Permalink
run borrowck tests on BIDs and emit tail-expr-drop-order lints for
Browse files Browse the repository at this point in the history
potential violations
  • Loading branch information
dingxiangfei2009 committed Dec 19, 2024
1 parent 3bf62cc commit 054769b
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 18 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_borrowck/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,9 @@ borrowck_suggest_create_fresh_reborrow =
borrowck_suggest_iterate_over_slice =
consider iterating over a slice of the `{$ty}`'s content to avoid moving into the `for` loop
borrowck_tail_expr_drop_order = a temporary value will be dropped here before the execution exits the block in Edition 2024, which will raise borrow checking error
.label = consider using a `let` binding to create a longer lived value; or replacing the `{"{"} .. {"}"}` block with curly brackets `( .. )`; or folding the rest of the expression into the surrounding `unsafe {"{"} .. {"}"}`
borrowck_ty_no_impl_copy =
{$is_partial_move ->
[true] partial move
Expand Down
80 changes: 65 additions & 15 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#![warn(unreachable_pub)]
// tidy-alphabetical-end

use std::borrow::Cow;
use std::cell::RefCell;
use std::collections::BTreeMap;
use std::marker::PhantomData;
Expand All @@ -24,8 +25,8 @@ use rustc_abi::FieldIdx;
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_data_structures::graph::dominators::Dominators;
use rustc_errors::Diag;
use rustc_hir as hir;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::{self as hir, CRATE_HIR_ID};
use rustc_index::bit_set::{BitSet, MixedBitSet};
use rustc_index::{IndexSlice, IndexVec};
use rustc_infer::infer::{
Expand All @@ -44,7 +45,7 @@ use rustc_mir_dataflow::move_paths::{
InitIndex, InitLocation, LookupResult, MoveData, MoveOutIndex, MovePathIndex,
};
use rustc_mir_dataflow::{Analysis, EntryStates, Results, ResultsVisitor, visit_results};
use rustc_session::lint::builtin::UNUSED_MUT;
use rustc_session::lint::builtin::{TAIL_EXPR_DROP_ORDER, UNUSED_MUT};
use rustc_span::{Span, Symbol};
use smallvec::SmallVec;
use tracing::{debug, instrument};
Expand Down Expand Up @@ -622,9 +623,11 @@ impl<'a, 'tcx> ResultsVisitor<'a, 'tcx, Borrowck<'a, 'tcx>> for MirBorrowckCtxt<
| StatementKind::Coverage(..)
// These do not actually affect borrowck
| StatementKind::ConstEvalCounter
// This do not affect borrowck
| StatementKind::BackwardIncompatibleDropHint { .. }
| StatementKind::StorageLive(..) => {}
// This does not affect borrowck
StatementKind::BackwardIncompatibleDropHint { place, reason: BackwardIncompatibleDropReason::Edition2024 } => {
self.check_backward_incompatible_drop(location, (**place, span), state);
}
StatementKind::StorageDead(local) => {
self.access_place(
location,
Expand Down Expand Up @@ -994,6 +997,23 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
}
}

fn maybe_polonius_borrows_in_scope<'s>(
&self,
location: Location,
state: &'s BorrowckDomain,
) -> Cow<'s, BitSet<BorrowIndex>> {
if let Some(polonius) = &self.polonius_output {
let location = self.location_table.start_index(location);
let mut polonius_output = BitSet::new_empty(self.borrow_set.len());
for &idx in polonius.errors_at(location) {
polonius_output.insert(idx);
}
Cow::Owned(polonius_output)
} else {
Cow::Borrowed(&state.borrows)
}
}

#[instrument(level = "debug", skip(self, state))]
fn check_access_for_conflict(
&mut self,
Expand All @@ -1006,17 +1026,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
let mut error_reported = false;

// Use polonius output if it has been enabled.
let mut polonius_output;
let borrows_in_scope = if let Some(polonius) = &self.polonius_output {
let location = self.location_table.start_index(location);
polonius_output = BitSet::new_empty(self.borrow_set.len());
for &idx in polonius.errors_at(location) {
polonius_output.insert(idx);
}
&polonius_output
} else {
&state.borrows
};
let borrows_in_scope = self.maybe_polonius_borrows_in_scope(location, state);

each_borrow_involving_path(
self,
Expand Down Expand Up @@ -1136,6 +1146,46 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
error_reported
}

/// Through #123739, backward incompatible drops (BIDs) are introduced.
/// We would like to emit lints whether borrow checking fails at these future drop locations.
#[instrument(level = "debug", skip(self, state))]
fn check_backward_incompatible_drop(
&mut self,
location: Location,
place_span: (Place<'tcx>, Span),
state: &BorrowckDomain,
) {
let sd = AccessDepth::Drop;

// Use polonius output if it has been enabled.
let borrows_in_scope = self.maybe_polonius_borrows_in_scope(location, state);

// This is a very simplified version of `Self::check_access_for_conflict`.
// We are here checking on BIDs and specifically still-live borrows of data involving the BIDs.
each_borrow_involving_path(
self,
self.infcx.tcx,
self.body,
(sd, place_span.0),
self.borrow_set,
|borrow_index| borrows_in_scope.contains(borrow_index),
|this, _borrow_index, borrow| {
if matches!(borrow.kind, BorrowKind::Fake(_)) {
return Control::Continue;
}
let borrowed = this.retrieve_borrow_spans(borrow).var_or_use_path_span();
this.infcx.tcx.emit_node_span_lint(
TAIL_EXPR_DROP_ORDER,
CRATE_HIR_ID,
place_span.1,
session_diagnostics::TailExprDropOrder { borrowed },
);
// We may stop at the first case
Control::Break
},
);
}

fn mutate_place(
&mut self,
location: Location,
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_borrowck/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,3 +480,10 @@ pub(crate) struct SimdIntrinsicArgConst {
pub arg: usize,
pub intrinsic: String,
}

#[derive(LintDiagnostic)]
#[diag(borrowck_tail_expr_drop_order)]
pub(crate) struct TailExprDropOrder {
#[label]
pub borrowed: Span,
}
6 changes: 3 additions & 3 deletions compiler/rustc_mir_build/src/builder/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1113,15 +1113,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

/// Schedule emission of a backwards incompatible drop lint hint.
/// Applicable only to temporary values for now.
#[instrument(level = "debug", skip(self))]
pub(crate) fn schedule_backwards_incompatible_drop(
&mut self,
span: Span,
region_scope: region::Scope,
local: Local,
) {
if !self.local_decls[local].ty.has_significant_drop(self.tcx, self.typing_env()) {
return;
}
// Note that we are *not* gating BIDs here on whether they have significant destructor.
// We need to know all of them so that we can capture potential borrow-checking errors.
for scope in self.scopes.scopes.iter_mut().rev() {
// Since we are inserting linting MIR statement, we have to invalidate the caches
scope.invalidate_cache();
Expand Down
37 changes: 37 additions & 0 deletions tests/ui/drop/lint-tail-expr-drop-order-borrowck.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Edition 2024 lint for change in drop order at tail expression
// This lint is to capture potential borrow-checking errors
// due to implementation of RFC 3606 <https://github.com/rust-lang/rfcs/pull/3606>
//@ edition: 2021

#![deny(tail_expr_drop_order)] //~ NOTE: the lint level is defined here

fn should_lint_with_potential_borrowck_err() {
let _ = { String::new().as_str() }.len();
//~^ ERROR: a temporary value will be dropped here
//~| WARN: this changes meaning in Rust 2024
//~| NOTE: consider using a `let` binding
//~| NOTE: for more information, see
}

fn should_lint_with_unsafe_block() {
fn f(_: usize) {}
f(unsafe { String::new().as_str() }.len());
//~^ ERROR: a temporary value will be dropped here
//~| WARN: this changes meaning in Rust 2024
//~| NOTE: consider using a `let` binding
//~| NOTE: for more information, see
}

#[rustfmt::skip]
fn should_lint_with_big_block() {
fn f<T>(_: T) {}
f({
&mut || 0
//~^ ERROR: a temporary value will be dropped here
//~| WARN: this changes meaning in Rust 2024
//~| NOTE: consider using a `let` binding
//~| NOTE: for more information, see
})
}

fn main() {}
40 changes: 40 additions & 0 deletions tests/ui/drop/lint-tail-expr-drop-order-borrowck.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
error: a temporary value will be dropped here before the execution exits the block in Edition 2024, which will raise borrow checking error
--> $DIR/lint-tail-expr-drop-order-borrowck.rs:9:36
|
LL | let _ = { String::new().as_str() }.len();
| ------------- ^
| |
| consider using a `let` binding to create a longer lived value; or replacing the `{ .. }` block with curly brackets `( .. )`; or folding the rest of the expression into the surrounding `unsafe { .. }`
|
= warning: this changes meaning in Rust 2024
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/temporary-tail-expr-scope.html>
note: the lint level is defined here
--> $DIR/lint-tail-expr-drop-order-borrowck.rs:6:9
|
LL | #![deny(tail_expr_drop_order)]
| ^^^^^^^^^^^^^^^^^^^^

error: a temporary value will be dropped here before the execution exits the block in Edition 2024, which will raise borrow checking error
--> $DIR/lint-tail-expr-drop-order-borrowck.rs:18:37
|
LL | f(unsafe { String::new().as_str() }.len());
| ------------- ^
| |
| consider using a `let` binding to create a longer lived value; or replacing the `{ .. }` block with curly brackets `( .. )`; or folding the rest of the expression into the surrounding `unsafe { .. }`
|
= warning: this changes meaning in Rust 2024
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/temporary-tail-expr-scope.html>

error: a temporary value will be dropped here before the execution exits the block in Edition 2024, which will raise borrow checking error
--> $DIR/lint-tail-expr-drop-order-borrowck.rs:29:17
|
LL | &mut || 0
| --------^
| |
| consider using a `let` binding to create a longer lived value; or replacing the `{ .. }` block with curly brackets `( .. )`; or folding the rest of the expression into the surrounding `unsafe { .. }`
|
= warning: this changes meaning in Rust 2024
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/temporary-tail-expr-scope.html>

error: aborting due to 3 previous errors

0 comments on commit 054769b

Please sign in to comment.