Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crater run for tail-expr-drop-order #134537

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
shallow = true
[submodule "src/tools/cargo"]
path = src/tools/cargo
url = https://github.com/rust-lang/cargo.git
url = https://github.com/ehuss/cargo.git
branch = "2024-crater"
shallow = true
[submodule "src/doc/reference"]
path = src/doc/reference
Expand Down
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
81 changes: 65 additions & 16 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,24 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
}
}

fn borrows_in_scope<'s>(
&self,
location: Location,
state: &'s BorrowckDomain,
) -> Cow<'s, BitSet<BorrowIndex>> {
if let Some(polonius) = &self.polonius_output {
// Use polonius output if it has been enabled.
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 @@ -1005,18 +1026,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
) -> bool {
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.borrows_in_scope(location, state);

each_borrow_involving_path(
self,
Expand Down Expand Up @@ -1136,6 +1146,45 @@ 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;

let borrows_in_scope = self.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
2 changes: 1 addition & 1 deletion src/tools/cargo
Submodule cargo updated 267 files
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

Loading