Skip to content

Commit fa33670

Browse files
borsbors
committed
Auto merge of rust-lang#131326 - dingxiangfei2009:issue-130836-attempt-2, r=<try>
Reduce false positives of tail-expr-drop-order from consumed values (attempt #2) r? `@nikomatsakis` Related to rust-lang#129864 but not replacing, yet. Related to rust-lang#130836. This is an implementation of the approach suggested in the [Zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/temporary.20drop.20order.20changes). A new MIR statement `BackwardsIncompatibleDrop` is added to the MIR syntax. The lint now works by inspecting possibly live move paths before at the `BackwardsIncompatibleDrop` location and the actual drop under the current edition, which should be one before Edition 2024 in practice.
2 parents 7caad69 + 80c5c8f commit fa33670

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

62 files changed

+1051
-529
lines changed

Diff for: ‎compiler/rustc_borrowck/src/dataflow.rs

+1
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,7 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
569569
| mir::StatementKind::Coverage(..)
570570
| mir::StatementKind::Intrinsic(..)
571571
| mir::StatementKind::ConstEvalCounter
572+
| mir::StatementKind::BackwardIncompatibleDropHint { .. }
572573
| mir::StatementKind::Nop => {}
573574
}
574575
}

Diff for: ‎compiler/rustc_borrowck/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,8 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R>
655655
| StatementKind::Coverage(..)
656656
// These do not actually affect borrowck
657657
| StatementKind::ConstEvalCounter
658+
// This do not affect borrowck
659+
| StatementKind::BackwardIncompatibleDropHint { .. }
658660
| StatementKind::StorageLive(..) => {}
659661
StatementKind::StorageDead(local) => {
660662
self.access_place(

Diff for: ‎compiler/rustc_borrowck/src/polonius/loan_invalidations.rs

+1
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LoanInvalidationsGenerator<'a, 'tcx> {
8888
| StatementKind::Nop
8989
| StatementKind::Retag { .. }
9090
| StatementKind::Deinit(..)
91+
| StatementKind::BackwardIncompatibleDropHint { .. }
9192
| StatementKind::SetDiscriminant { .. } => {
9293
bug!("Statement not allowed in this MIR phase")
9394
}

Diff for: ‎compiler/rustc_borrowck/src/type_check/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1309,6 +1309,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
13091309
| StatementKind::Coverage(..)
13101310
| StatementKind::ConstEvalCounter
13111311
| StatementKind::PlaceMention(..)
1312+
| StatementKind::BackwardIncompatibleDropHint { .. }
13121313
| StatementKind::Nop => {}
13131314
StatementKind::Deinit(..) | StatementKind::SetDiscriminant { .. } => {
13141315
bug!("Statement not allowed in this MIR phase")

Diff for: ‎compiler/rustc_codegen_cranelift/src/base.rs

+1
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,7 @@ fn codegen_stmt<'tcx>(
920920
| StatementKind::FakeRead(..)
921921
| StatementKind::Retag { .. }
922922
| StatementKind::PlaceMention(..)
923+
| StatementKind::BackwardIncompatibleDropHint { .. }
923924
| StatementKind::AscribeUserType(..) => {}
924925

925926
StatementKind::Coverage { .. } => unreachable!(),

Diff for: ‎compiler/rustc_codegen_cranelift/src/constant.rs

+1
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,7 @@ pub(crate) fn mir_operand_get_const_val<'tcx>(
578578
| StatementKind::PlaceMention(..)
579579
| StatementKind::Coverage(_)
580580
| StatementKind::ConstEvalCounter
581+
| StatementKind::BackwardIncompatibleDropHint { .. }
581582
| StatementKind::Nop => {}
582583
}
583584
}

Diff for: ‎compiler/rustc_codegen_ssa/src/mir/statement.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
9292
| mir::StatementKind::AscribeUserType(..)
9393
| mir::StatementKind::ConstEvalCounter
9494
| mir::StatementKind::PlaceMention(..)
95-
| mir::StatementKind::Nop => {}
95+
| mir::StatementKind::Nop
96+
| mir::StatementKind::BackwardIncompatibleDropHint { .. } => {}
9697
}
9798
}
9899
}

Diff for: ‎compiler/rustc_const_eval/src/check_consts/check.rs

+1
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
542542
| StatementKind::Coverage(..)
543543
| StatementKind::Intrinsic(..)
544544
| StatementKind::ConstEvalCounter
545+
| StatementKind::BackwardIncompatibleDropHint { .. }
545546
| StatementKind::Nop => {}
546547
}
547548
}

Diff for: ‎compiler/rustc_const_eval/src/interpret/step.rs

+3
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
143143
// Defined to do nothing. These are added by optimization passes, to avoid changing the
144144
// size of MIR constantly.
145145
Nop => {}
146+
147+
// Only used for temporary lifetime lints
148+
BackwardIncompatibleDropHint { .. } => {}
146149
}
147150

148151
interp_ok(())

Diff for: ‎compiler/rustc_hir_analysis/src/check/region.rs

+17-3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use rustc_index::Idx;
1717
use rustc_middle::bug;
1818
use rustc_middle::middle::region::*;
1919
use rustc_middle::ty::TyCtxt;
20+
use rustc_session::lint;
2021
use rustc_span::source_map;
2122
use tracing::debug;
2223

@@ -167,10 +168,23 @@ fn resolve_block<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, blk: &'tcx h
167168
}
168169
}
169170
if let Some(tail_expr) = blk.expr {
170-
if visitor.tcx.features().shorter_tail_lifetimes
171-
&& blk.span.edition().at_least_rust_2024()
171+
let local_id = tail_expr.hir_id.local_id;
172+
let edition = blk.span.edition();
173+
if visitor.tcx.features().shorter_tail_lifetimes && edition.at_least_rust_2024() {
174+
visitor.terminating_scopes.insert(local_id);
175+
} else if edition < lint::Edition::Edition2024
176+
&& !matches!(
177+
visitor.tcx.lint_level_at_node(lint::builtin::TAIL_EXPR_DROP_ORDER, blk.hir_id),
178+
(lint::Level::Allow, _)
179+
)
172180
{
173-
visitor.terminating_scopes.insert(tail_expr.hir_id.local_id);
181+
// Note: we are unconditionally adding this information so that we can run
182+
// migration for Edition lower than 2024.
183+
// For future scope changes, we need to extend the mapping with edition information.
184+
visitor
185+
.scope_tree
186+
.backwards_incompatible_scope
187+
.insert(local_id, Scope { id: local_id, data: ScopeData::Node });
174188
}
175189
visitor.visit_expr(tail_expr);
176190
}

Diff for: ‎compiler/rustc_index/src/bit_set.rs

+134-4
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,10 @@ impl<T: Idx> ChunkedBitSet<T> {
460460
self.chunks.iter().map(|chunk| chunk.count()).sum()
461461
}
462462

463+
pub fn is_empty(&self) -> bool {
464+
self.chunks.iter().all(|chunk| matches!(chunk, Chunk::Zeros(..) | Chunk::Ones(0)))
465+
}
466+
463467
/// Returns `true` if `self` contains `elem`.
464468
#[inline]
465469
pub fn contains(&self, elem: T) -> bool {
@@ -668,12 +672,138 @@ impl<T: Idx> BitRelations<ChunkedBitSet<T>> for ChunkedBitSet<T> {
668672
changed
669673
}
670674

671-
fn subtract(&mut self, _other: &ChunkedBitSet<T>) -> bool {
672-
unimplemented!("implement if/when necessary");
675+
fn subtract(&mut self, other: &ChunkedBitSet<T>) -> bool {
676+
assert_eq!(self.domain_size, other.domain_size);
677+
debug_assert_eq!(self.chunks.len(), other.chunks.len());
678+
679+
let mut changed = false;
680+
for (mut self_chunk, other_chunk) in self.chunks.iter_mut().zip(other.chunks.iter()) {
681+
match (&mut self_chunk, &other_chunk) {
682+
(Zeros(..), _) | (_, Zeros(..)) => {}
683+
(Ones(self_chunk_domain_size), Ones(other_chunk_domain_size)) => {
684+
debug_assert_eq!(self_chunk_domain_size, other_chunk_domain_size);
685+
changed = true;
686+
*self_chunk = Zeros(*self_chunk_domain_size);
687+
}
688+
(_, Ones(_)) => {}
689+
(
690+
Ones(self_chunk_domain_size),
691+
Mixed(other_chunk_domain_size, other_chunk_count, other_chunk_words),
692+
) => {
693+
debug_assert_eq!(self_chunk_domain_size, other_chunk_domain_size);
694+
changed = true;
695+
let num_words = num_words(*self_chunk_domain_size as usize);
696+
debug_assert!(num_words > 0 && num_words <= CHUNK_WORDS);
697+
let mut tail_mask =
698+
1 << (*other_chunk_domain_size - ((num_words - 1) * WORD_BITS) as u16) - 1;
699+
let mut self_chunk_words = **other_chunk_words;
700+
for word in self_chunk_words[0..num_words].iter_mut().rev() {
701+
*word = !*word & tail_mask;
702+
tail_mask = u64::MAX;
703+
}
704+
let self_chunk_count = *self_chunk_domain_size - *other_chunk_count;
705+
debug_assert_eq!(
706+
self_chunk_count,
707+
self_chunk_words[0..num_words]
708+
.iter()
709+
.map(|w| w.count_ones() as ChunkSize)
710+
.sum()
711+
);
712+
*self_chunk =
713+
Mixed(*self_chunk_domain_size, self_chunk_count, Rc::new(self_chunk_words));
714+
}
715+
(
716+
Mixed(
717+
self_chunk_domain_size,
718+
ref mut self_chunk_count,
719+
ref mut self_chunk_words,
720+
),
721+
Mixed(_other_chunk_domain_size, _other_chunk_count, other_chunk_words),
722+
) => {
723+
// See [`<Self as BitRelations<ChunkedBitSet<T>>>::union`] for the explanation
724+
let op = |a: u64, b: u64| a & !b;
725+
let num_words = num_words(*self_chunk_domain_size as usize);
726+
if bitwise_changes(
727+
&self_chunk_words[0..num_words],
728+
&other_chunk_words[0..num_words],
729+
op,
730+
) {
731+
let self_chunk_words = Rc::make_mut(self_chunk_words);
732+
let has_changed = bitwise(
733+
&mut self_chunk_words[0..num_words],
734+
&other_chunk_words[0..num_words],
735+
op,
736+
);
737+
debug_assert!(has_changed);
738+
*self_chunk_count = self_chunk_words[0..num_words]
739+
.iter()
740+
.map(|w| w.count_ones() as ChunkSize)
741+
.sum();
742+
if *self_chunk_count == 0 {
743+
*self_chunk = Zeros(*self_chunk_domain_size);
744+
}
745+
changed = true;
746+
}
747+
}
748+
}
749+
}
750+
changed
673751
}
674752

675-
fn intersect(&mut self, _other: &ChunkedBitSet<T>) -> bool {
676-
unimplemented!("implement if/when necessary");
753+
fn intersect(&mut self, other: &ChunkedBitSet<T>) -> bool {
754+
assert_eq!(self.domain_size, other.domain_size);
755+
debug_assert_eq!(self.chunks.len(), other.chunks.len());
756+
757+
let mut changed = false;
758+
for (mut self_chunk, other_chunk) in self.chunks.iter_mut().zip(other.chunks.iter()) {
759+
match (&mut self_chunk, &other_chunk) {
760+
(Zeros(..), _) | (_, Ones(..)) => {}
761+
(
762+
Ones(self_chunk_domain_size),
763+
Zeros(other_chunk_domain_size) | Mixed(other_chunk_domain_size, ..),
764+
)
765+
| (Mixed(self_chunk_domain_size, ..), Zeros(other_chunk_domain_size)) => {
766+
debug_assert_eq!(self_chunk_domain_size, other_chunk_domain_size);
767+
changed = true;
768+
*self_chunk = other_chunk.clone();
769+
}
770+
(
771+
Mixed(
772+
self_chunk_domain_size,
773+
ref mut self_chunk_count,
774+
ref mut self_chunk_words,
775+
),
776+
Mixed(_other_chunk_domain_size, _other_chunk_count, other_chunk_words),
777+
) => {
778+
// See [`<Self as BitRelations<ChunkedBitSet<T>>>::union`] for the explanation
779+
let op = |a, b| a & b;
780+
let num_words = num_words(*self_chunk_domain_size as usize);
781+
if bitwise_changes(
782+
&self_chunk_words[0..num_words],
783+
&other_chunk_words[0..num_words],
784+
op,
785+
) {
786+
let self_chunk_words = Rc::make_mut(self_chunk_words);
787+
let has_changed = bitwise(
788+
&mut self_chunk_words[0..num_words],
789+
&other_chunk_words[0..num_words],
790+
op,
791+
);
792+
debug_assert!(has_changed);
793+
*self_chunk_count = self_chunk_words[0..num_words]
794+
.iter()
795+
.map(|w| w.count_ones() as ChunkSize)
796+
.sum();
797+
if *self_chunk_count == 0 {
798+
*self_chunk = Zeros(*self_chunk_domain_size);
799+
}
800+
changed = true;
801+
}
802+
}
803+
}
804+
}
805+
806+
changed
677807
}
678808
}
679809

Diff for: ‎compiler/rustc_lint/messages.ftl

-3
Original file line numberDiff line numberDiff line change
@@ -772,9 +772,6 @@ lint_suspicious_double_ref_clone =
772772
lint_suspicious_double_ref_deref =
773773
using `.deref()` on a double reference, which returns `{$ty}` instead of dereferencing the inner type
774774
775-
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
776-
.label = these values have significant drop implementation and will observe changes in drop order under Edition 2024
777-
778775
lint_trailing_semi_macro = trailing semicolon in macro used in expression position
779776
.note1 = macro invocations at the end of a block are treated as expressions
780777
.note2 = to ignore the value produced by the macro, add a semicolon after the invocation of `{$name}`

Diff for: ‎compiler/rustc_lint/src/lib.rs

-3
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ mod redundant_semicolon;
8181
mod reference_casting;
8282
mod shadowed_into_iter;
8383
mod static_mut_refs;
84-
mod tail_expr_drop_order;
8584
mod traits;
8685
mod types;
8786
mod unit_bindings;
@@ -122,7 +121,6 @@ use rustc_middle::ty::TyCtxt;
122121
use shadowed_into_iter::ShadowedIntoIter;
123122
pub use shadowed_into_iter::{ARRAY_INTO_ITER, BOXED_SLICE_INTO_ITER};
124123
use static_mut_refs::*;
125-
use tail_expr_drop_order::TailExprDropOrder;
126124
use traits::*;
127125
use types::*;
128126
use unit_bindings::*;
@@ -247,7 +245,6 @@ late_lint_methods!(
247245
AsyncFnInTrait: AsyncFnInTrait,
248246
NonLocalDefinitions: NonLocalDefinitions::default(),
249247
ImplTraitOvercaptures: ImplTraitOvercaptures,
250-
TailExprDropOrder: TailExprDropOrder,
251248
IfLetRescope: IfLetRescope::default(),
252249
StaticMutRefs: StaticMutRefs,
253250
UnqualifiedLocalImports: UnqualifiedLocalImports,

0 commit comments

Comments
 (0)