Skip to content

Commit

Permalink
Auto merge of #129864 - dingxiangfei2009:reduce-false-positives-from-…
Browse files Browse the repository at this point in the history
…consumed-droppers, r=<try>

Reduce false positives of tail-expr-drop-order from consumed values

r? `@jieyouxu`

To reduce false positives, the lint does not fire if the locals are consumed/moved, or the values with significant drop are consumed/moved at the tail expression location.

For this, we rely on solving a small dataflow to determine whether a `Local` is either live, or moved/dropped.

I am also printing the type involved for easier diagnosis.
  • Loading branch information
bors committed Sep 25, 2024
2 parents 38352b0 + 2457860 commit 7014e13
Show file tree
Hide file tree
Showing 35 changed files with 612 additions and 384 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4090,6 +4090,7 @@ dependencies = [
"rustc_hir",
"rustc_index",
"rustc_infer",
"rustc_lint",
"rustc_macros",
"rustc_middle",
"rustc_mir_build",
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R>
TerminatorKind::SwitchInt { discr, targets: _ } => {
self.consume_operand(loc, (discr, span), state);
}
TerminatorKind::Drop { place, target: _, unwind: _, replace } => {
TerminatorKind::Drop { place, target: _, scope: _, unwind: _, replace } => {
debug!(
"visit_terminator_drop \
loc: {:?} term: {:?} place: {:?} span: {:?}",
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/polonius/loan_invalidations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LoanInvalidationsGenerator<'a, 'tcx> {
TerminatorKind::SwitchInt { discr, targets: _ } => {
self.consume_operand(location, discr);
}
TerminatorKind::Drop { place: drop_place, target: _, unwind: _, replace } => {
TerminatorKind::Drop { place: drop_place, target: _, scope: _, unwind: _, replace } => {
let write_kind =
if *replace { WriteKind::Replace } else { WriteKind::StorageDeadOrDrop };
self.access_place(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
| TerminatorKind::CoroutineDrop => {
bug!("shouldn't exist at codegen {:?}", bb_data.terminator());
}
TerminatorKind::Drop { place, target, unwind: _, replace: _ } => {
TerminatorKind::Drop { place, target, unwind: _, scope: _, replace: _ } => {
let drop_place = codegen_place(fx, *place);
crate::abi::codegen_drop(fx, source_info, drop_place, *target);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
MergingSucc::False
}

mir::TerminatorKind::Drop { place, target, unwind, replace: _ } => self
mir::TerminatorKind::Drop { place, target, unwind, scope: _, replace: _ } => self
.codegen_drop_terminator(
helper,
bx,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
}
}

Drop { place, target, unwind, replace: _ } => {
Drop { place, target, unwind, scope: _, replace: _ } => {
let place = self.eval_place(place)?;
let instance = Instance::resolve_drop_in_place(*self.tcx, place.layout.ty);
if let ty::InstanceKind::DropGlue(_, None) = instance.def {
Expand Down
60 changes: 58 additions & 2 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 @@ -672,8 +676,60 @@ impl<T: Idx> BitRelations<ChunkedBitSet<T>> for ChunkedBitSet<T> {
unimplemented!("implement if/when necessary");
}

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 @@ -778,9 +778,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 @@ -82,7 +82,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 @@ -123,7 +122,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 @@ -248,7 +246,6 @@ late_lint_methods!(
AsyncFnInTrait: AsyncFnInTrait,
NonLocalDefinitions: NonLocalDefinitions::default(),
ImplTraitOvercaptures: ImplTraitOvercaptures,
TailExprDropOrder: TailExprDropOrder,
IfLetRescope: IfLetRescope::default(),
StaticMutRefs: StaticMutRefs,
UnqualifiedLocalImports: UnqualifiedLocalImports,
Expand Down
Loading

0 comments on commit 7014e13

Please sign in to comment.