Skip to content

Commit 72911fb

Browse files
davidtwcodavidtwco
committed
Update logic to search for casts.
This commit updates the captured trait object search logic to look for unsized casts to boxed types rather than for functions that returned trait objects.
1 parent 91b71f5 commit 72911fb

File tree

3 files changed

+149
-61
lines changed

3 files changed

+149
-61
lines changed

‎src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs‎

Lines changed: 109 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ use borrow_check::nll::region_infer::Cause;
1414
use borrow_check::{Context, MirBorrowckCtxt, WriteKind};
1515
use rustc::ty::{self, Region, TyCtxt};
1616
use rustc::mir::{
17-
FakeReadCause, Local, Location, Mir, Operand, Place, Rvalue, Statement, StatementKind,
18-
TerminatorKind
17+
CastKind, FakeReadCause, Local, Location, Mir, Operand, Place, Projection, ProjectionElem,
18+
Rvalue, Statement, StatementKind, TerminatorKind
1919
};
2020
use rustc_errors::DiagnosticBuilder;
2121
use syntax_pos::Span;
@@ -65,7 +65,7 @@ impl<'tcx> BorrowExplanation<'tcx> {
6565
BorrowExplanation::UsedLaterInLoop(later_use_kind, var_or_use_span) => {
6666
let message = match later_use_kind {
6767
LaterUseKind::TraitCapture =>
68-
"borrow later captured here by trait object, in later iteration of loop",
68+
"borrow captured here by trait object, in later iteration of loop",
6969
LaterUseKind::ClosureCapture =>
7070
"borrow captured here by closure, in later iteration of loop",
7171
LaterUseKind::Call => "borrow used by call, in later iteration of loop",
@@ -373,20 +373,20 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
373373
}
374374
}
375375

376-
/// Check if a borrowed value was captured by a trait object.
376+
/// Check if a borrowed value was captured by a trait object. We do this by
377+
/// looking forward in the MIR from the reserve location and checking if we see
378+
/// a unsized cast to a trait object on our data.
377379
fn was_captured_by_trait_object(&self, borrow: &BorrowData<'tcx>) -> bool {
378-
// In order to check if a value was captured by a trait object, we want to look through
379-
// statements after the reserve location in the current block. We expect the reserve
380-
// location to be a statement assigning to a local. We follow that local in the subsequent
381-
// statements, checking for an assignment of our local (or something intermediate that
382-
// it was assigned into) that results in a trait object.
380+
// Start at the reserve location, find the place that we want to see cast to a trait object.
383381
let location = borrow.reserve_location;
384382
let block = &self.mir[location.block];
385383
let stmt = block.statements.get(location.statement_index);
386-
debug!(
387-
"was_captured_by_trait_object: location={:?} block={:?} stmt={:?}",
388-
location, block, stmt
389-
);
384+
debug!("was_captured_by_trait_object: location={:?} stmt={:?}", location, stmt);
385+
386+
// We make a `queue` vector that has the locations we want to visit. As of writing, this
387+
// will only ever have one item at any given time, but by using a vector, we can pop from
388+
// it which simplifies the termination logic.
389+
let mut queue = vec![location];
390390
let mut target = if let Some(&Statement {
391391
kind: StatementKind::Assign(Place::Local(local), _),
392392
..
@@ -396,61 +396,109 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
396396
return false;
397397
};
398398

399-
debug!("was_captured_by_trait_object: target={:?}", target);
400-
for stmt in &block.statements[location.statement_index + 1..] {
401-
debug!("was_captured_by_trait_object: stmt={:?}", stmt);
402-
// Simple case where our target is assigned into another local, and we start
403-
// watching that local instead.
404-
if let StatementKind::Assign(
405-
Place::Local(into),
406-
box Rvalue::Use(operand),
407-
) = &stmt.kind {
408-
debug!("was_captured_by_trait_object: target={:?} operand={:?}", target, operand);
409-
match operand {
410-
Operand::Copy(Place::Local(from)) |
411-
Operand::Move(Place::Local(from)) if *from == target => target = *into,
412-
_ => {},
413-
}
414-
}
415-
}
416-
417-
if let Some(terminator) = &block.terminator {
418-
if let TerminatorKind::Call {
419-
destination: Some((Place::Local(dest), _)),
420-
args,
421-
..
422-
} = &terminator.kind {
423-
debug!(
424-
"was_captured_by_trait_object: target={:?} dest={:?} args={:?}",
425-
target, dest, args
426-
);
427-
let mut found_target = false;
428-
for arg in args {
429-
if let Operand::Move(Place::Local(potential)) = arg {
430-
if *potential == target {
431-
found_target = true;
432-
}
399+
debug!("was_captured_by_trait: target={:?} queue={:?}", target, queue);
400+
while let Some(current_location) = queue.pop() {
401+
debug!("was_captured_by_trait: target={:?}", target);
402+
let block = &self.mir[current_location.block];
403+
// We need to check the current location to find out if it is a terminator.
404+
let is_terminator = current_location.statement_index == block.statements.len();
405+
if !is_terminator {
406+
let stmt = &block.statements[current_location.statement_index];
407+
debug!("was_captured_by_trait_object: stmt={:?}", stmt);
408+
409+
// The only kind of statement that we care about is assignments...
410+
if let StatementKind::Assign(
411+
place,
412+
box rvalue,
413+
) = &stmt.kind {
414+
let into = match place {
415+
Place::Local(into) => into,
416+
Place::Projection(box Projection {
417+
base: Place::Local(into),
418+
elem: ProjectionElem::Deref,
419+
}) => into,
420+
_ => {
421+
// Continue at the next location.
422+
queue.push(current_location.successor_within_block());
423+
continue;
424+
},
425+
};
426+
427+
match rvalue {
428+
// If we see a use, we should check whether it is our data, and if so
429+
// update the place that we're looking for to that new place.
430+
Rvalue::Use(operand) => match operand {
431+
Operand::Copy(Place::Local(from)) |
432+
Operand::Move(Place::Local(from)) if *from == target => {
433+
target = *into;
434+
},
435+
_ => {},
436+
},
437+
// If we see a unsized cast, then if it is our data we should check
438+
// whether it is being cast to a trait object.
439+
Rvalue::Cast(CastKind::Unsize, operand, ty) => match operand {
440+
Operand::Copy(Place::Local(from)) |
441+
Operand::Move(Place::Local(from)) if *from == target => {
442+
debug!("was_captured_by_trait_object: ty={:?}", ty);
443+
// Check the type for a trait object.
444+
match ty.sty {
445+
// `&dyn Trait`
446+
ty::TyKind::Ref(_, ty, _) if ty.is_trait() => return true,
447+
// `Box<dyn Trait>`
448+
_ if ty.is_box() && ty.boxed_ty().is_trait() =>
449+
return true,
450+
// `dyn Trait`
451+
_ if ty.is_trait() => return true,
452+
// Anything else.
453+
_ => return false,
454+
}
455+
},
456+
_ => return false,
457+
},
458+
_ => {},
433459
}
434460
}
435461

436-
if found_target {
437-
let local_decl_ty = &self.mir.local_decls[*dest].ty;
438-
debug!("was_captured_by_trait_object: local_decl_ty={:?}", local_decl_ty);
439-
match local_decl_ty.sty {
440-
// `&dyn Trait`
441-
ty::TyKind::Ref(_, ty, _) if ty.is_trait() => return true,
442-
// `Box<dyn Trait>`
443-
_ if local_decl_ty.is_box() && local_decl_ty.boxed_ty().is_trait() =>
444-
return true,
445-
// `dyn Trait`
446-
_ if local_decl_ty.is_trait() => return true,
447-
// Anything else.
448-
_ => return false,
449-
}
462+
// Continue at the next location.
463+
queue.push(current_location.successor_within_block());
464+
} else {
465+
// The only thing we need to do for terminators is progress to the next block.
466+
let terminator = block.terminator();
467+
debug!("was_captured_by_trait_object: terminator={:?}", terminator);
468+
469+
match &terminator.kind {
470+
TerminatorKind::Call {
471+
destination: Some((Place::Local(dest), block)),
472+
args,
473+
..
474+
} => {
475+
debug!(
476+
"was_captured_by_trait_object: target={:?} dest={:?} args={:?}",
477+
target, dest, args
478+
);
479+
// Check if one of the arguments to this function is the target place.
480+
let found_target = args.iter().any(|arg| {
481+
if let Operand::Move(Place::Local(potential)) = arg {
482+
*potential == target
483+
} else {
484+
false
485+
}
486+
});
487+
488+
// If it is, follow this to the next block and update the target.
489+
if found_target {
490+
target = *dest;
491+
queue.push(block.start_location());
492+
}
493+
},
494+
_ => {},
450495
}
451496
}
497+
498+
debug!("was_captured_by_trait: queue={:?}", queue);
452499
}
453500

501+
// We didn't find anything and ran out of locations to check.
454502
false
455503
}
456504
}

‎src/test/ui/nll/issue-52663-trait-object.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![feature(box_syntax)]
12+
#![feature(nll)]
13+
14+
trait Foo { fn get(&self); }
15+
16+
impl<A> Foo for A {
17+
fn get(&self) { }
18+
}
19+
20+
fn main() {
21+
let _ = {
22+
let tmp0 = 3;
23+
let tmp1 = &tmp0;
24+
box tmp1 as Box<Foo + '_>
25+
};
26+
//~^^^ ERROR `tmp0` does not live long enough
27+
}

‎src/test/ui/nll/issue-52663-trait-object.stderr‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error[E0597]: `tmp0` does not live long enough
2+
--> $DIR/issue-52663-trait-object.rs:23:20
3+
|
4+
LL | let tmp1 = &tmp0;
5+
| ^^^^^ borrowed value does not live long enough
6+
LL | box tmp1 as Box<Foo + '_>
7+
| ------------------------- borrow later captured here by trait object
8+
LL | };
9+
| - `tmp0` dropped here while still borrowed
10+
11+
error: aborting due to previous error
12+
13+
For more information about this error, try `rustc --explain E0597`.

0 commit comments

Comments
 (0)