Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 4475508

Browse files
dingxiangfei2009cuviper
authored andcommittedJan 10, 2025
run borrowck tests on BIDs and emit tail-expr-drop-order lints for
potential violations (cherry picked from commit 045271c)
1 parent 9e57baf commit 4475508

File tree

6 files changed

+155
-17
lines changed

6 files changed

+155
-17
lines changed
 

‎compiler/rustc_borrowck/messages.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,9 @@ borrowck_suggest_create_fresh_reborrow =
213213
borrowck_suggest_iterate_over_slice =
214214
consider iterating over a slice of the `{$ty}`'s content to avoid moving into the `for` loop
215215
216+
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
217+
.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 {"{"} .. {"}"}`
218+
216219
borrowck_ty_no_impl_copy =
217220
{$is_partial_move ->
218221
[true] partial move

‎compiler/rustc_borrowck/src/lib.rs

+65-14
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#![warn(unreachable_pub)]
1616
// tidy-alphabetical-end
1717

18+
use std::borrow::Cow;
1819
use std::cell::RefCell;
1920
use std::marker::PhantomData;
2021
use std::ops::Deref;
@@ -23,6 +24,7 @@ use rustc_abi::FieldIdx;
2324
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
2425
use rustc_data_structures::graph::dominators::Dominators;
2526
use rustc_hir as hir;
27+
use rustc_hir::CRATE_HIR_ID;
2628
use rustc_hir::def_id::LocalDefId;
2729
use rustc_index::bit_set::{BitSet, MixedBitSet};
2830
use rustc_index::{IndexSlice, IndexVec};
@@ -42,7 +44,7 @@ use rustc_mir_dataflow::move_paths::{
4244
InitIndex, InitLocation, LookupResult, MoveData, MovePathIndex,
4345
};
4446
use rustc_mir_dataflow::{Analysis, EntryStates, Results, ResultsVisitor, visit_results};
45-
use rustc_session::lint::builtin::UNUSED_MUT;
47+
use rustc_session::lint::builtin::{TAIL_EXPR_DROP_ORDER, UNUSED_MUT};
4648
use rustc_span::{Span, Symbol};
4749
use smallvec::SmallVec;
4850
use tracing::{debug, instrument};
@@ -636,9 +638,11 @@ impl<'a, 'tcx> ResultsVisitor<'a, 'tcx, Borrowck<'a, 'tcx>> for MirBorrowckCtxt<
636638
| StatementKind::Coverage(..)
637639
// These do not actually affect borrowck
638640
| StatementKind::ConstEvalCounter
639-
// This do not affect borrowck
640-
| StatementKind::BackwardIncompatibleDropHint { .. }
641641
| StatementKind::StorageLive(..) => {}
642+
// This does not affect borrowck
643+
StatementKind::BackwardIncompatibleDropHint { place, reason: BackwardIncompatibleDropReason::Edition2024 } => {
644+
self.check_backward_incompatible_drop(location, (**place, span), state);
645+
}
642646
StatementKind::StorageDead(local) => {
643647
self.access_place(
644648
location,
@@ -1007,6 +1011,23 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
10071011
}
10081012
}
10091013

1014+
fn maybe_polonius_borrows_in_scope<'s>(
1015+
&self,
1016+
location: Location,
1017+
state: &'s BorrowckDomain,
1018+
) -> Cow<'s, BitSet<BorrowIndex>> {
1019+
if let Some(polonius) = &self.polonius_output {
1020+
let location = self.location_table.start_index(location);
1021+
let mut polonius_output = BitSet::new_empty(self.borrow_set.len());
1022+
for &idx in polonius.errors_at(location) {
1023+
polonius_output.insert(idx);
1024+
}
1025+
Cow::Owned(polonius_output)
1026+
} else {
1027+
Cow::Borrowed(&state.borrows)
1028+
}
1029+
}
1030+
10101031
#[instrument(level = "debug", skip(self, state))]
10111032
fn check_access_for_conflict(
10121033
&mut self,
@@ -1019,17 +1040,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
10191040
let mut error_reported = false;
10201041

10211042
// Use polonius output if it has been enabled.
1022-
let mut polonius_output;
1023-
let borrows_in_scope = if let Some(polonius) = &self.polonius_output {
1024-
let location = self.location_table.start_index(location);
1025-
polonius_output = BitSet::new_empty(self.borrow_set.len());
1026-
for &idx in polonius.errors_at(location) {
1027-
polonius_output.insert(idx);
1028-
}
1029-
&polonius_output
1030-
} else {
1031-
&state.borrows
1032-
};
1043+
let borrows_in_scope = self.maybe_polonius_borrows_in_scope(location, state);
10331044

10341045
each_borrow_involving_path(
10351046
self,
@@ -1149,6 +1160,46 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
11491160
error_reported
11501161
}
11511162

1163+
/// Through #123739, backward incompatible drops (BIDs) are introduced.
1164+
/// We would like to emit lints whether borrow checking fails at these future drop locations.
1165+
#[instrument(level = "debug", skip(self, state))]
1166+
fn check_backward_incompatible_drop(
1167+
&mut self,
1168+
location: Location,
1169+
place_span: (Place<'tcx>, Span),
1170+
state: &BorrowckDomain,
1171+
) {
1172+
let sd = AccessDepth::Drop;
1173+
1174+
// Use polonius output if it has been enabled.
1175+
let borrows_in_scope = self.maybe_polonius_borrows_in_scope(location, state);
1176+
1177+
// This is a very simplified version of `Self::check_access_for_conflict`.
1178+
// We are here checking on BIDs and specifically still-live borrows of data involving the BIDs.
1179+
each_borrow_involving_path(
1180+
self,
1181+
self.infcx.tcx,
1182+
self.body,
1183+
(sd, place_span.0),
1184+
self.borrow_set,
1185+
|borrow_index| borrows_in_scope.contains(borrow_index),
1186+
|this, _borrow_index, borrow| {
1187+
if matches!(borrow.kind, BorrowKind::Fake(_)) {
1188+
return Control::Continue;
1189+
}
1190+
let borrowed = this.retrieve_borrow_spans(borrow).var_or_use_path_span();
1191+
this.infcx.tcx.emit_node_span_lint(
1192+
TAIL_EXPR_DROP_ORDER,
1193+
CRATE_HIR_ID,
1194+
place_span.1,
1195+
session_diagnostics::TailExprDropOrder { borrowed },
1196+
);
1197+
// We may stop at the first case
1198+
Control::Break
1199+
},
1200+
);
1201+
}
1202+
11521203
fn mutate_place(
11531204
&mut self,
11541205
location: Location,

‎compiler/rustc_borrowck/src/session_diagnostics.rs

+7
Original file line numberDiff line numberDiff line change
@@ -480,3 +480,10 @@ pub(crate) struct SimdIntrinsicArgConst {
480480
pub arg: usize,
481481
pub intrinsic: String,
482482
}
483+
484+
#[derive(LintDiagnostic)]
485+
#[diag(borrowck_tail_expr_drop_order)]
486+
pub(crate) struct TailExprDropOrder {
487+
#[label]
488+
pub borrowed: Span,
489+
}

‎compiler/rustc_mir_build/src/builder/scope.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1131,15 +1131,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
11311131

11321132
/// Schedule emission of a backwards incompatible drop lint hint.
11331133
/// Applicable only to temporary values for now.
1134+
#[instrument(level = "debug", skip(self))]
11341135
pub(crate) fn schedule_backwards_incompatible_drop(
11351136
&mut self,
11361137
span: Span,
11371138
region_scope: region::Scope,
11381139
local: Local,
11391140
) {
1140-
if !self.local_decls[local].ty.has_significant_drop(self.tcx, self.typing_env()) {
1141-
return;
1142-
}
1141+
// Note that we are *not* gating BIDs here on whether they have significant destructor.
1142+
// We need to know all of them so that we can capture potential borrow-checking errors.
11431143
for scope in self.scopes.scopes.iter_mut().rev() {
11441144
// Since we are inserting linting MIR statement, we have to invalidate the caches
11451145
scope.invalidate_cache();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Edition 2024 lint for change in drop order at tail expression
2+
// This lint is to capture potential borrow-checking errors
3+
// due to implementation of RFC 3606 <https://github.com/rust-lang/rfcs/pull/3606>
4+
//@ edition: 2021
5+
6+
#![deny(tail_expr_drop_order)] //~ NOTE: the lint level is defined here
7+
8+
fn should_lint_with_potential_borrowck_err() {
9+
let _ = { String::new().as_str() }.len();
10+
//~^ ERROR: a temporary value will be dropped here
11+
//~| WARN: this changes meaning in Rust 2024
12+
//~| NOTE: consider using a `let` binding
13+
//~| NOTE: for more information, see
14+
}
15+
16+
fn should_lint_with_unsafe_block() {
17+
fn f(_: usize) {}
18+
f(unsafe { String::new().as_str() }.len());
19+
//~^ ERROR: a temporary value will be dropped here
20+
//~| WARN: this changes meaning in Rust 2024
21+
//~| NOTE: consider using a `let` binding
22+
//~| NOTE: for more information, see
23+
}
24+
25+
#[rustfmt::skip]
26+
fn should_lint_with_big_block() {
27+
fn f<T>(_: T) {}
28+
f({
29+
&mut || 0
30+
//~^ ERROR: a temporary value will be dropped here
31+
//~| WARN: this changes meaning in Rust 2024
32+
//~| NOTE: consider using a `let` binding
33+
//~| NOTE: for more information, see
34+
})
35+
}
36+
37+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
error: a temporary value will be dropped here before the execution exits the block in Edition 2024, which will raise borrow checking error
2+
--> $DIR/lint-tail-expr-drop-order-borrowck.rs:9:36
3+
|
4+
LL | let _ = { String::new().as_str() }.len();
5+
| ------------- ^
6+
| |
7+
| 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 { .. }`
8+
|
9+
= warning: this changes meaning in Rust 2024
10+
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/temporary-tail-expr-scope.html>
11+
note: the lint level is defined here
12+
--> $DIR/lint-tail-expr-drop-order-borrowck.rs:6:9
13+
|
14+
LL | #![deny(tail_expr_drop_order)]
15+
| ^^^^^^^^^^^^^^^^^^^^
16+
17+
error: a temporary value will be dropped here before the execution exits the block in Edition 2024, which will raise borrow checking error
18+
--> $DIR/lint-tail-expr-drop-order-borrowck.rs:18:37
19+
|
20+
LL | f(unsafe { String::new().as_str() }.len());
21+
| ------------- ^
22+
| |
23+
| 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 { .. }`
24+
|
25+
= warning: this changes meaning in Rust 2024
26+
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/temporary-tail-expr-scope.html>
27+
28+
error: a temporary value will be dropped here before the execution exits the block in Edition 2024, which will raise borrow checking error
29+
--> $DIR/lint-tail-expr-drop-order-borrowck.rs:29:17
30+
|
31+
LL | &mut || 0
32+
| --------^
33+
| |
34+
| 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 { .. }`
35+
|
36+
= warning: this changes meaning in Rust 2024
37+
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/temporary-tail-expr-scope.html>
38+
39+
error: aborting due to 3 previous errors
40+

0 commit comments

Comments
 (0)
Failed to load comments.