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

Reduce false positives of tail-expr-drop-order from consumed values (attempt #2) #131326

Merged
merged 1 commit into from
Nov 20, 2024
Merged
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
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/dataflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,7 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> {
| mir::StatementKind::Coverage(..)
| mir::StatementKind::Intrinsic(..)
| mir::StatementKind::ConstEvalCounter
| mir::StatementKind::BackwardIncompatibleDropHint { .. }
| mir::StatementKind::Nop => {}
}
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,8 @@ 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(..) => {}
StatementKind::StorageDead(local) => {
self.access_place(
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/polonius/loan_invalidations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LoanInvalidationsGenerator<'a, 'tcx> {
| StatementKind::Nop
| StatementKind::Retag { .. }
| StatementKind::Deinit(..)
| StatementKind::BackwardIncompatibleDropHint { .. }
| StatementKind::SetDiscriminant { .. } => {
bug!("Statement not allowed in this MIR phase")
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1252,6 +1252,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
| StatementKind::Coverage(..)
| StatementKind::ConstEvalCounter
| StatementKind::PlaceMention(..)
| StatementKind::BackwardIncompatibleDropHint { .. }
| StatementKind::Nop => {}
StatementKind::Deinit(..) | StatementKind::SetDiscriminant { .. } => {
bug!("Statement not allowed in this MIR phase")
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,7 @@ fn codegen_stmt<'tcx>(
| StatementKind::FakeRead(..)
| StatementKind::Retag { .. }
| StatementKind::PlaceMention(..)
| StatementKind::BackwardIncompatibleDropHint { .. }
| StatementKind::AscribeUserType(..) => {}

StatementKind::Coverage { .. } => unreachable!(),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,7 @@ pub(crate) fn mir_operand_get_const_val<'tcx>(
| StatementKind::PlaceMention(..)
| StatementKind::Coverage(_)
| StatementKind::ConstEvalCounter
| StatementKind::BackwardIncompatibleDropHint { .. }
| StatementKind::Nop => {}
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/mir/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
| mir::StatementKind::AscribeUserType(..)
| mir::StatementKind::ConstEvalCounter
| mir::StatementKind::PlaceMention(..)
| mir::StatementKind::BackwardIncompatibleDropHint { .. }
| mir::StatementKind::Nop => {}
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
| StatementKind::Coverage(..)
| StatementKind::Intrinsic(..)
| StatementKind::ConstEvalCounter
| StatementKind::BackwardIncompatibleDropHint { .. }
| StatementKind::Nop => {}
}
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// Defined to do nothing. These are added by optimization passes, to avoid changing the
// size of MIR constantly.
Nop => {}

// Only used for temporary lifetime lints
BackwardIncompatibleDropHint { .. } => {}
}

interp_ok(())
Expand Down
20 changes: 18 additions & 2 deletions compiler/rustc_hir_analysis/src/check/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use rustc_index::Idx;
use rustc_middle::bug;
use rustc_middle::middle::region::*;
use rustc_middle::ty::TyCtxt;
use rustc_session::lint;
use rustc_span::source_map;
use tracing::debug;

Expand Down Expand Up @@ -167,8 +168,23 @@ fn resolve_block<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, blk: &'tcx h
}
}
if let Some(tail_expr) = blk.expr {
if blk.span.edition().at_least_rust_2024() {
visitor.terminating_scopes.insert(tail_expr.hir_id.local_id);
let local_id = tail_expr.hir_id.local_id;
let edition = blk.span.edition();
if edition.at_least_rust_2024() {
visitor.terminating_scopes.insert(local_id);
} else if !visitor
.tcx
.lints_that_dont_need_to_run(())
.contains(&lint::LintId::of(lint::builtin::TAIL_EXPR_DROP_ORDER))
{
// If this temporary scope will be changing once the codebase adopts Rust 2024,
// and we are linting about possible semantic changes that would result,
// then record this node-id in the field `backwards_incompatible_scope`
// for future reference.
visitor
.scope_tree
.backwards_incompatible_scope
.insert(local_id, Scope { id: local_id, data: ScopeData::Node });
}
visitor.visit_expr(tail_expr);
}
Expand Down
140 changes: 136 additions & 4 deletions compiler/rustc_index/src/bit_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,10 @@ impl<T: Idx> ChunkedBitSet<T> {
self.chunks.iter().map(|chunk| chunk.count()).sum()
}

pub fn is_empty(&self) -> bool {
self.chunks.iter().all(|chunk| matches!(chunk, Chunk::Zeros(..) | Chunk::Ones(0)))
}

/// Returns `true` if `self` contains `elem`.
#[inline]
pub fn contains(&self, elem: T) -> bool {
Expand Down Expand Up @@ -668,12 +672,140 @@ impl<T: Idx> BitRelations<ChunkedBitSet<T>> for ChunkedBitSet<T> {
changed
}

fn subtract(&mut self, _other: &ChunkedBitSet<T>) -> bool {
unimplemented!("implement if/when necessary");
fn subtract(&mut self, other: &ChunkedBitSet<T>) -> bool {
assert_eq!(self.domain_size, other.domain_size);
debug_assert_eq!(self.chunks.len(), other.chunks.len());

let mut changed = false;
for (mut self_chunk, other_chunk) in self.chunks.iter_mut().zip(other.chunks.iter()) {
match (&mut self_chunk, &other_chunk) {
(Zeros(..), _) | (_, Zeros(..)) => {}
(
Ones(self_chunk_domain_size) | Mixed(self_chunk_domain_size, _, _),
Ones(other_chunk_domain_size),
) => {
debug_assert_eq!(self_chunk_domain_size, other_chunk_domain_size);
changed = true;
*self_chunk = Zeros(*self_chunk_domain_size);
}
(
Ones(self_chunk_domain_size),
Mixed(other_chunk_domain_size, other_chunk_count, other_chunk_words),
) => {
debug_assert_eq!(self_chunk_domain_size, other_chunk_domain_size);
changed = true;
let num_words = num_words(*self_chunk_domain_size as usize);
debug_assert!(num_words > 0 && num_words <= CHUNK_WORDS);
let mut tail_mask =
1 << (*other_chunk_domain_size - ((num_words - 1) * WORD_BITS) as u16) - 1;
let mut self_chunk_words = **other_chunk_words;
for word in self_chunk_words[0..num_words].iter_mut().rev() {
*word = !*word & tail_mask;
tail_mask = u64::MAX;
}
let self_chunk_count = *self_chunk_domain_size - *other_chunk_count;
debug_assert_eq!(
self_chunk_count,
self_chunk_words[0..num_words]
.iter()
.map(|w| w.count_ones() as ChunkSize)
.sum()
);
*self_chunk =
Mixed(*self_chunk_domain_size, self_chunk_count, Rc::new(self_chunk_words));
}
(
Mixed(
self_chunk_domain_size,
ref mut self_chunk_count,
ref mut self_chunk_words,
),
Mixed(_other_chunk_domain_size, _other_chunk_count, other_chunk_words),
) => {
// See [`<Self as BitRelations<ChunkedBitSet<T>>>::union`] for the explanation
let op = |a: u64, b: u64| a & !b;
let num_words = num_words(*self_chunk_domain_size as usize);
if bitwise_changes(
&self_chunk_words[0..num_words],
&other_chunk_words[0..num_words],
op,
) {
let self_chunk_words = Rc::make_mut(self_chunk_words);
let has_changed = bitwise(
&mut self_chunk_words[0..num_words],
&other_chunk_words[0..num_words],
op,
);
debug_assert!(has_changed);
*self_chunk_count = self_chunk_words[0..num_words]
.iter()
.map(|w| w.count_ones() as ChunkSize)
.sum();
if *self_chunk_count == 0 {
*self_chunk = Zeros(*self_chunk_domain_size);
}
changed = true;
}
}
}
}
changed
}

fn intersect(&mut self, _other: &ChunkedBitSet<T>) -> bool {
unimplemented!("implement if/when necessary");
fn intersect(&mut self, other: &ChunkedBitSet<T>) -> bool {
assert_eq!(self.domain_size, other.domain_size);
debug_assert_eq!(self.chunks.len(), other.chunks.len());

let mut changed = false;
for (mut self_chunk, other_chunk) in self.chunks.iter_mut().zip(other.chunks.iter()) {
match (&mut self_chunk, &other_chunk) {
(Zeros(..), _) | (_, Ones(..)) => {}
(
Ones(self_chunk_domain_size),
Zeros(other_chunk_domain_size) | Mixed(other_chunk_domain_size, ..),
)
| (Mixed(self_chunk_domain_size, ..), Zeros(other_chunk_domain_size)) => {
debug_assert_eq!(self_chunk_domain_size, other_chunk_domain_size);
changed = true;
*self_chunk = other_chunk.clone();
}
(
Mixed(
self_chunk_domain_size,
ref mut self_chunk_count,
ref mut self_chunk_words,
),
Mixed(_other_chunk_domain_size, _other_chunk_count, other_chunk_words),
) => {
// See [`<Self as BitRelations<ChunkedBitSet<T>>>::union`] for the explanation
let op = |a, b| a & b;
let num_words = num_words(*self_chunk_domain_size as usize);
if bitwise_changes(
&self_chunk_words[0..num_words],
&other_chunk_words[0..num_words],
op,
) {
let self_chunk_words = Rc::make_mut(self_chunk_words);
let has_changed = bitwise(
&mut self_chunk_words[0..num_words],
&other_chunk_words[0..num_words],
op,
);
debug_assert!(has_changed);
*self_chunk_count = self_chunk_words[0..num_words]
.iter()
.map(|w| w.count_ones() as ChunkSize)
.sum();
if *self_chunk_count == 0 {
*self_chunk = Zeros(*self_chunk_domain_size);
}
changed = true;
}
}
}
}

changed
}
}

Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -772,9 +772,6 @@ lint_suspicious_double_ref_clone =
lint_suspicious_double_ref_deref =
using `.deref()` on a double reference, which returns `{$ty}` instead of dereferencing the inner type

lint_tail_expr_drop_order = these values and local bindings have significant drop implementation that will have a different drop order from that of Edition 2021
.label = these values have significant drop implementation and will observe changes in drop order under Edition 2024

lint_trailing_semi_macro = trailing semicolon in macro used in expression position
.note1 = macro invocations at the end of a block are treated as expressions
.note2 = to ignore the value produced by the macro, add a semicolon after the invocation of `{$name}`
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ mod redundant_semicolon;
mod reference_casting;
mod shadowed_into_iter;
mod static_mut_refs;
mod tail_expr_drop_order;
mod traits;
mod types;
mod unit_bindings;
Expand Down Expand Up @@ -116,7 +115,6 @@ use rustc_middle::ty::TyCtxt;
use shadowed_into_iter::ShadowedIntoIter;
pub use shadowed_into_iter::{ARRAY_INTO_ITER, BOXED_SLICE_INTO_ITER};
use static_mut_refs::*;
use tail_expr_drop_order::TailExprDropOrder;
use traits::*;
use types::*;
use unit_bindings::*;
Expand Down Expand Up @@ -240,7 +238,6 @@ late_lint_methods!(
AsyncFnInTrait: AsyncFnInTrait,
NonLocalDefinitions: NonLocalDefinitions::default(),
ImplTraitOvercaptures: ImplTraitOvercaptures,
TailExprDropOrder: TailExprDropOrder,
IfLetRescope: IfLetRescope::default(),
StaticMutRefs: StaticMutRefs,
UnqualifiedLocalImports: UnqualifiedLocalImports,
Expand Down
Loading
Loading