Skip to content

Commit 51fe635

Browse files
dingxiangfei2009compiler-errors
authored andcommitted
switch to MaybeInitializedPlaces
1 parent 777af6e commit 51fe635

File tree

4 files changed

+132
-217
lines changed

4 files changed

+132
-217
lines changed

compiler/rustc_index/src/bit_set.rs

+58-2
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 {
@@ -672,8 +676,60 @@ impl<T: Idx> BitRelations<ChunkedBitSet<T>> for ChunkedBitSet<T> {
672676
unimplemented!("implement if/when necessary");
673677
}
674678

675-
fn intersect(&mut self, _other: &ChunkedBitSet<T>) -> bool {
676-
unimplemented!("implement if/when necessary");
679+
fn intersect(&mut self, other: &ChunkedBitSet<T>) -> bool {
680+
assert_eq!(self.domain_size, other.domain_size);
681+
debug_assert_eq!(self.chunks.len(), other.chunks.len());
682+
683+
let mut changed = false;
684+
for (mut self_chunk, other_chunk) in self.chunks.iter_mut().zip(other.chunks.iter()) {
685+
match (&mut self_chunk, &other_chunk) {
686+
(Zeros(..), _) | (_, Ones(..)) => {}
687+
(
688+
Ones(self_chunk_domain_size),
689+
Zeros(other_chunk_domain_size) | Mixed(other_chunk_domain_size, ..),
690+
)
691+
| (Mixed(self_chunk_domain_size, ..), Zeros(other_chunk_domain_size)) => {
692+
debug_assert_eq!(self_chunk_domain_size, other_chunk_domain_size);
693+
changed = true;
694+
*self_chunk = other_chunk.clone();
695+
}
696+
(
697+
Mixed(
698+
self_chunk_domain_size,
699+
ref mut self_chunk_count,
700+
ref mut self_chunk_words,
701+
),
702+
Mixed(_other_chunk_domain_size, _other_chunk_count, other_chunk_words),
703+
) => {
704+
// See [`<Self as BitRelations<ChunkedBitSet<T>>>::union`] for the explanation
705+
let op = |a, b| a & b;
706+
let num_words = num_words(*self_chunk_domain_size as usize);
707+
if bitwise_changes(
708+
&self_chunk_words[0..num_words],
709+
&other_chunk_words[0..num_words],
710+
op,
711+
) {
712+
let self_chunk_words = Rc::make_mut(self_chunk_words);
713+
let has_changed = bitwise(
714+
&mut self_chunk_words[0..num_words],
715+
&other_chunk_words[0..num_words],
716+
op,
717+
);
718+
debug_assert!(has_changed);
719+
*self_chunk_count = self_chunk_words[0..num_words]
720+
.iter()
721+
.map(|w| w.count_ones() as ChunkSize)
722+
.sum();
723+
if *self_chunk_count == 0 {
724+
*self_chunk = Zeros(*self_chunk_domain_size);
725+
}
726+
changed = true;
727+
}
728+
}
729+
}
730+
}
731+
732+
changed
677733
}
678734
}
679735

Original file line numberDiff line numberDiff line change
@@ -1,20 +1,15 @@
11
use rustc_hir::def_id::LocalDefId;
22
use rustc_hir::{ExprKind, HirId, HirIdSet, OwnerId};
3-
use rustc_index::bit_set::BitSet;
3+
use rustc_index::bit_set::{BitSet, ChunkedBitSet};
44
use rustc_index::IndexVec;
55
use rustc_lint::{self as lint, Level};
66
use rustc_macros::LintDiagnostic;
7-
use rustc_middle::mir::visit::{
8-
MutatingUseContext, NonMutatingUseContext, NonUseContext, PlaceContext, Visitor,
9-
};
10-
use rustc_middle::mir::{
11-
dump_mir, BasicBlock, Body, CallReturnPlaces, HasLocalDecls, Local, Location, Place,
12-
ProjectionElem, Statement, Terminator, TerminatorEdges, TerminatorKind,
13-
};
7+
use rustc_middle::mir::{dump_mir, BasicBlock, Body, Local, Location, TerminatorKind};
148
use rustc_middle::ty::{self, Ty, TyCtxt};
15-
use rustc_mir_dataflow::{Analysis, AnalysisDomain, GenKill, GenKillAnalysis};
9+
use rustc_mir_dataflow::impls::MaybeInitializedPlaces;
10+
use rustc_mir_dataflow::move_paths::MoveData;
11+
use rustc_mir_dataflow::{Analysis, MaybeReachable};
1612
use rustc_span::Span;
17-
use rustc_type_ir::data_structures::IndexMap;
1813
use tracing::debug;
1914

2015
fn blocks_reachable_from_tail_expr_rev(body: &Body<'_>, blocks: &mut BitSet<BasicBlock>) {
@@ -78,7 +73,7 @@ fn is_descendent_of_hir_id(
7873

7974
pub(crate) fn run_lint<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &Body<'tcx>) {
8075
if matches!(tcx.def_kind(def_id), rustc_hir::def::DefKind::SyntheticCoroutineBody) {
81-
// Synthetic coroutine has no HIR body and it is enough to just analyse the original body
76+
// A synthetic coroutine has no HIR body and it is enough to just analyse the original body
8277
return;
8378
}
8479
let (tail_expr_hir_id, tail_expr_span) = {
@@ -127,35 +122,7 @@ pub(crate) fn run_lint<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &Body<
127122
let mut tail_expr_descendants: HirIdSet = [tail_expr_hir_id].into_iter().collect();
128123

129124
let param_env = tcx.param_env(def_id);
130-
let mut droppy_locals = IndexMap::default();
131-
let (nr_upvars, nr_locals, mut droppy_local_mask) = if tcx.is_closure_like(def_id.to_def_id()) {
132-
let captures = tcx.closure_captures(def_id);
133-
let nr_upvars = captures.len();
134-
let nr_body_locals = body.local_decls.len();
135-
let mut mask = BitSet::new_empty(nr_body_locals + nr_upvars);
136-
for (idx, &capture) in captures.iter().enumerate() {
137-
let ty = capture.place.ty();
138-
if ty.has_significant_drop(tcx, param_env) {
139-
let upvar = Local::from_usize(nr_body_locals + idx);
140-
mask.insert(upvar);
141-
droppy_locals.insert(upvar, (capture.var_ident.span, ty));
142-
}
143-
}
144-
(nr_upvars, nr_upvars + nr_body_locals, mask)
145-
} else {
146-
let nr_locals = body.local_decls.len();
147-
(0, nr_locals, BitSet::new_empty(nr_locals))
148-
};
149-
for (local, decl) in body.local_decls().iter_enumerated() {
150-
if local == Local::ZERO {
151-
// return place is ignored
152-
continue;
153-
}
154-
if decl.ty.has_significant_drop(tcx, param_env) {
155-
droppy_local_mask.insert(local);
156-
droppy_locals.insert(local, (decl.source_info.span, decl.ty));
157-
}
158-
}
125+
let is_closure_like = tcx.is_closure_like(def_id.to_def_id());
159126

160127
let nr_blocks = body.basic_blocks.len();
161128
let mut tail_expr_blocks = BitSet::new_empty(nr_blocks);
@@ -218,58 +185,87 @@ pub(crate) fn run_lint<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &Body<
218185
}
219186
debug!(?tail_expr_exit_boundary_blocks);
220187

221-
let mut maybe_live = LiveLocals { nr_upvars, nr_body_locals: body.local_decls.len() }
222-
.into_engine(tcx, body)
223-
.iterate_to_fixpoint()
224-
.into_results_cursor(body);
225-
let mut observables = BitSet::new_empty(nr_locals);
188+
let move_data = MoveData::gather_moves(body, tcx, param_env, |_| true);
189+
let mut droppy_paths = ChunkedBitSet::new_empty(move_data.move_paths.len());
190+
let mut droppy_paths_not_in_tail = droppy_paths.clone();
191+
for (idx, path) in move_data.move_paths.iter_enumerated() {
192+
if path.place.ty(&body.local_decls, tcx).ty.has_significant_drop(tcx, param_env) {
193+
droppy_paths.insert(idx);
194+
if !tail_expr_span.contains(body.local_decls[path.place.local].source_info.span) {
195+
droppy_paths_not_in_tail.insert(idx);
196+
}
197+
}
198+
}
199+
let maybe_init = MaybeInitializedPlaces::new(tcx, body, &move_data);
200+
let mut maybe_init =
201+
maybe_init.into_engine(tcx, body).iterate_to_fixpoint().into_results_cursor(body);
202+
let mut observables = ChunkedBitSet::new_empty(move_data.move_paths.len());
226203
for block in tail_expr_exit_boundary_blocks.iter() {
227204
debug!(?observables);
228205
if boundary_lies_on_statement.contains(block) {
229206
let target = Location { block, statement_index: tail_expr_stmts[block] };
230207
debug!(?target, "in block");
231-
maybe_live.seek_after_primary_effect(target);
208+
maybe_init.seek_after_primary_effect(target);
232209
} else {
233-
maybe_live.seek_to_block_start(block);
210+
maybe_init.seek_to_block_start(block);
211+
}
212+
if let MaybeReachable::Reachable(alive) = maybe_init.get() {
213+
debug!(?block, ?alive);
214+
let mut mask = droppy_paths.clone();
215+
mask.intersect(alive);
216+
observables.union(&mask);
234217
}
235-
let mut mask = droppy_local_mask.clone();
236-
let alive = maybe_live.get();
237-
debug!(?block, ?alive);
238-
mask.intersect(alive);
239-
observables.union(&mask);
240-
}
241-
if observables.is_empty() {
242-
debug!("no observable, bail");
243-
return;
244218
}
245-
debug!(?observables, ?droppy_locals);
219+
debug!(?observables);
246220
// A linted local has a span contained in the tail expression span
247-
let Some((linted_local, (linted_local_span, linted_local_ty))) = observables
221+
let Some(linted_move_path) = observables
248222
.iter()
249-
.filter_map(|local| droppy_locals.get(&local).map(|&info| (local, info)))
250-
.filter(|(_, (span, _))| tail_expr_span.contains(*span))
223+
.map(|path| &move_data.move_paths[path])
224+
.filter(|move_path| {
225+
if move_path.place.local == Local::ZERO {
226+
return false;
227+
}
228+
if is_closure_like && matches!(move_path.place.local, ty::CAPTURE_STRUCT_LOCAL) {
229+
return false;
230+
}
231+
tail_expr_span.contains(body.local_decls[move_path.place.local].source_info.span)
232+
})
251233
.nth(0)
252234
else {
253235
debug!("nothing to lint");
254236
return;
255237
};
256-
debug!(?linted_local);
238+
debug!(?linted_move_path);
257239
let body_observables: Vec<_> = observables
258240
.iter()
259-
.filter(|&local| local != linted_local)
260-
.filter_map(|local| droppy_locals.get(&local))
261-
.map(|&(span, _)| span)
241+
.filter_map(|idx| {
242+
// We want to lint on place/local in body, not another tail expression temporary.
243+
// Also, closures always drops their upvars last, so we don't have to lint about them.
244+
let base_local = move_data.base_local(idx);
245+
if base_local == linted_move_path.place.local || base_local == Local::ZERO {
246+
debug!(?base_local, "skip");
247+
return None;
248+
}
249+
if is_closure_like && matches!(base_local, ty::CAPTURE_STRUCT_LOCAL) {
250+
debug!(?base_local, "skip in closure");
251+
return None;
252+
}
253+
droppy_paths_not_in_tail
254+
.contains(idx)
255+
.then_some(body.local_decls[base_local].source_info.span)
256+
})
262257
.collect();
263258
if body_observables.is_empty() {
264259
debug!("nothing observable from body");
265260
return;
266261
}
267-
debug!(?linted_local, ?body_observables);
268-
let decorator = TailExprDropOrderLint { spans: body_observables, ty: linted_local_ty };
262+
debug!(?body_observables);
263+
let linted_local_decl = &body.local_decls[linted_move_path.place.local];
264+
let decorator = TailExprDropOrderLint { spans: body_observables, ty: linted_local_decl.ty };
269265
tcx.emit_node_span_lint(
270266
lint::builtin::TAIL_EXPR_DROP_ORDER,
271267
tail_expr_hir_id,
272-
linted_local_span,
268+
linted_local_decl.source_info.span,
273269
decorator,
274270
);
275271
}
@@ -281,123 +277,3 @@ struct TailExprDropOrderLint<'a> {
281277
pub spans: Vec<Span>,
282278
pub ty: Ty<'a>,
283279
}
284-
285-
struct LiveLocals {
286-
nr_upvars: usize,
287-
nr_body_locals: usize,
288-
}
289-
290-
impl<'tcx> AnalysisDomain<'tcx> for LiveLocals {
291-
type Domain = BitSet<Local>;
292-
const NAME: &'static str = "liveness-by-use";
293-
294-
fn bottom_value(&self, body: &Body<'tcx>) -> Self::Domain {
295-
debug_assert_eq!(self.nr_body_locals, body.local_decls.len());
296-
BitSet::new_empty(self.nr_body_locals + self.nr_upvars)
297-
}
298-
299-
fn initialize_start_block(&self, body: &Body<'tcx>, state: &mut Self::Domain) {
300-
for arg in body.args_iter() {
301-
state.insert(arg);
302-
}
303-
debug_assert_eq!(self.nr_body_locals, body.local_decls.len());
304-
for upvar in 0..self.nr_upvars {
305-
state.insert(Local::from_usize(self.nr_body_locals + upvar));
306-
}
307-
}
308-
}
309-
310-
impl<'tcx> GenKillAnalysis<'tcx> for LiveLocals {
311-
type Idx = Local;
312-
313-
fn domain_size(&self, body: &Body<'tcx>) -> usize {
314-
body.local_decls.len()
315-
}
316-
317-
fn statement_effect(
318-
&mut self,
319-
transfer: &mut impl GenKill<Self::Idx>,
320-
statement: &Statement<'tcx>,
321-
location: Location,
322-
) {
323-
TransferFunction {
324-
nr_upvars: self.nr_upvars,
325-
nr_body_locals: self.nr_body_locals,
326-
transfer,
327-
}
328-
.visit_statement(statement, location);
329-
}
330-
331-
fn terminator_effect<'mir>(
332-
&mut self,
333-
transfer: &mut Self::Domain,
334-
terminator: &'mir Terminator<'tcx>,
335-
location: Location,
336-
) -> TerminatorEdges<'mir, 'tcx> {
337-
TransferFunction {
338-
nr_upvars: self.nr_upvars,
339-
nr_body_locals: self.nr_body_locals,
340-
transfer,
341-
}
342-
.visit_terminator(terminator, location);
343-
terminator.edges()
344-
}
345-
346-
fn call_return_effect(
347-
&mut self,
348-
trans: &mut Self::Domain,
349-
_block: BasicBlock,
350-
return_places: CallReturnPlaces<'_, 'tcx>,
351-
) {
352-
return_places.for_each(|place| {
353-
if let Some(local) = place.as_local() {
354-
trans.gen_(local);
355-
}
356-
})
357-
}
358-
}
359-
360-
struct TransferFunction<'a, T> {
361-
nr_upvars: usize,
362-
nr_body_locals: usize,
363-
transfer: &'a mut T,
364-
}
365-
366-
impl<'tcx, T> Visitor<'tcx> for TransferFunction<'_, T>
367-
where
368-
T: GenKill<Local>,
369-
{
370-
fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _location: Location) {
371-
let local = if let Some(local) = place.as_local() {
372-
local
373-
} else if let Place { local: ty::CAPTURE_STRUCT_LOCAL, projection } = *place
374-
&& let [ProjectionElem::Field(idx, _)] = **projection
375-
&& idx.as_usize() < self.nr_upvars
376-
{
377-
Local::from_usize(self.nr_body_locals + idx.as_usize())
378-
} else {
379-
return;
380-
};
381-
382-
match context {
383-
PlaceContext::NonUse(NonUseContext::StorageDead)
384-
| PlaceContext::MutatingUse(MutatingUseContext::Drop | MutatingUseContext::Deinit)
385-
| PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) => {
386-
self.transfer.kill(local);
387-
if matches!(local, ty::CAPTURE_STRUCT_LOCAL) && self.nr_upvars > 0 {
388-
for upvar in 0..self.nr_upvars {
389-
self.transfer.kill(Local::from_usize(self.nr_body_locals + upvar));
390-
}
391-
}
392-
}
393-
PlaceContext::MutatingUse(
394-
MutatingUseContext::Store
395-
| MutatingUseContext::AsmOutput
396-
| MutatingUseContext::Call,
397-
) => {
398-
self.transfer.gen_(local);
399-
}
400-
_ => {}
401-
}
402-
}
403-
}

tests/ui/drop/lint-tail-expr-drop-order.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,11 @@ fn should_lint() -> i32 {
2828
//~| WARN: this changes meaning in Rust 2024
2929
}
3030

31-
fn should_lint_closure() -> impl FnOnce() -> i32 {
31+
fn should_not_lint_closure() -> impl FnOnce() -> i32 {
3232
let x = LoudDropper;
3333
move || {
34+
// Should not lint
3435
x.get() + LoudDropper.get()
35-
//~^ ERROR: this value of type `LoudDropper` has significant drop implementation that will have a different drop order from that of Edition 2021
36-
//~| WARN: this changes meaning in Rust 2024
3736
}
3837
}
3938

0 commit comments

Comments
 (0)