Skip to content
Open
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
102 changes: 79 additions & 23 deletions compiler/rustc_hir_typeck/src/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ use rustc_middle::ty::{
use rustc_middle::{bug, span_bug};
use rustc_session::lint;
use rustc_span::{BytePos, Pos, Span, Symbol, sym};
use rustc_trait_selection::error_reporting::InferCtxtErrorExt as _;
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::solve;
use tracing::{debug, instrument};

use super::FnCtxt;
Expand Down Expand Up @@ -197,17 +199,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let closure_def_id = closure_def_id.expect_local();

assert_eq!(self.tcx.hir_body_owner_def_id(body.id()), closure_def_id);

let closure_fcx = FnCtxt::new(self, self.tcx.param_env(closure_def_id), closure_def_id);

let mut delegate = InferBorrowKind {
fcx: &closure_fcx,
closure_def_id,
capture_information: Default::default(),
fake_reads: Default::default(),
};

let _ = euv::ExprUseVisitor::new(
&FnCtxt::new(self, self.tcx.param_env(closure_def_id), closure_def_id),
&mut delegate,
)
.consume_body(body);
let _ = euv::ExprUseVisitor::new(&closure_fcx, &mut delegate).consume_body(body);

// There are several curious situations with coroutine-closures where
// analysis is too aggressive with borrows when the coroutine-closure is
Expand Down Expand Up @@ -287,7 +289,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let hir::def::Res::Local(local_id) = path.res else {
bug!();
};
let place = self.place_for_root_variable(closure_def_id, local_id);
let place = closure_fcx.place_for_root_variable(closure_def_id, local_id);
delegate.capture_information.push((
place,
ty::CaptureInfo {
Expand Down Expand Up @@ -326,7 +328,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

if let Some(upvars) = self.tcx.upvars_mentioned(closure_def_id) {
for var_hir_id in upvars.keys() {
let place = self.place_for_root_variable(closure_def_id, *var_hir_id);
let place = closure_fcx.place_for_root_variable(closure_def_id, *var_hir_id);

debug!("seed place {:?}", place);

Expand Down Expand Up @@ -560,17 +562,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
bug!();
};

let coroutine_fcx =
FnCtxt::new(self, self.tcx.param_env(coroutine_def_id), coroutine_def_id);

let mut delegate = InferBorrowKind {
fcx: &coroutine_fcx,
closure_def_id: coroutine_def_id,
capture_information: Default::default(),
fake_reads: Default::default(),
};

let _ = euv::ExprUseVisitor::new(
&FnCtxt::new(self, self.tcx.param_env(coroutine_def_id), coroutine_def_id),
&mut delegate,
)
.consume_expr(body);
let _ = euv::ExprUseVisitor::new(&coroutine_fcx, &mut delegate).consume_expr(body);

let (_, kind, _) = self.process_collected_capture_information(
hir::CaptureBy::Ref,
Expand Down Expand Up @@ -1126,6 +1128,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
}
}
fn normalize_capture_place(&self, span: Span, place: Place<'tcx>) -> Place<'tcx> {
let mut place = self.resolve_vars_if_possible(place);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why &mut Place instead of taking by value and returning it again?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, addressed in dddbf96

// In the new solver, types in HIR `Place`s can contain unnormalized aliases,
// which can ICE later (e.g. when projecting fields for diagnostics).
if self.next_trait_solver() {
let cause = self.misc(span);
let at = self.at(&cause, self.param_env);
match solve::deeply_normalize_with_skipped_universes_and_ambiguous_coroutine_goals(
at,
place.clone(),
vec![],
) {
Ok((normalized, goals)) => {
if !goals.is_empty() {
let mut typeck_results = self.typeck_results.borrow_mut();
typeck_results.coroutine_stalled_predicates.extend(
goals
.into_iter()
// FIXME: throwing away the param-env :(
.map(|goal| (goal.predicate, self.misc(span))),
);
}
normalized
}
Err(errors) => {
let guar = self.infcx.err_ctxt().report_fulfillment_errors(errors);
place.base_ty = Ty::new_error(self.tcx, guar);
for proj in &mut place.projections {
proj.ty = Ty::new_error(self.tcx, guar);
}
place
}
}
} else {
// For the old solver we can rely on `normalize` to eagerly normalize aliases.
self.normalize(span, place)
}
}

/// Combines all the reasons for 2229 migrations
fn compute_2229_migrations_reasons(
Expand Down Expand Up @@ -1735,11 +1776,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) -> Place<'tcx> {
let upvar_id = ty::UpvarId::new(var_hir_id, closure_def_id);

Place {
let place = Place {
base_ty: self.node_ty(var_hir_id),
base: PlaceBase::Upvar(upvar_id),
projections: Default::default(),
}
};

// Normalize eagerly when inserting into `capture_information`, so all downstream
// capture analysis can assume a normalized `Place`.
self.normalize_capture_place(self.tcx.hir_span(var_hir_id), place)
}

fn should_log_capture_analysis(&self, closure_def_id: LocalDefId) -> bool {
Expand Down Expand Up @@ -1999,7 +2044,8 @@ fn drop_location_span(tcx: TyCtxt<'_>, hir_id: HirId) -> Span {
tcx.sess.source_map().end_point(owner_span)
}

struct InferBorrowKind<'tcx> {
struct InferBorrowKind<'fcx, 'a, 'tcx> {
fcx: &'fcx FnCtxt<'a, 'tcx>,
// The def-id of the closure whose kind and upvar accesses are being inferred.
closure_def_id: LocalDefId,

Expand Down Expand Up @@ -2033,7 +2079,7 @@ struct InferBorrowKind<'tcx> {
fake_reads: Vec<(Place<'tcx>, FakeReadCause, HirId)>,
}

impl<'tcx> euv::Delegate<'tcx> for InferBorrowKind<'tcx> {
impl<'fcx, 'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'fcx, 'a, 'tcx> {
#[instrument(skip(self), level = "debug")]
fn fake_read(
&mut self,
Expand All @@ -2047,8 +2093,10 @@ impl<'tcx> euv::Delegate<'tcx> for InferBorrowKind<'tcx> {
// such as deref of a raw pointer.
let dummy_capture_kind = ty::UpvarCapture::ByRef(ty::BorrowKind::Immutable);

let (place, _) =
restrict_capture_precision(place_with_id.place.clone(), dummy_capture_kind);
let span = self.fcx.tcx.hir_span(diag_expr_id);
let place = self.fcx.normalize_capture_place(span, place_with_id.place.clone());

let (place, _) = restrict_capture_precision(place, dummy_capture_kind);

let (place, _) = restrict_repr_packed_field_ref_capture(place, dummy_capture_kind);
self.fake_reads.push((place, cause, diag_expr_id));
Expand All @@ -2059,8 +2107,11 @@ impl<'tcx> euv::Delegate<'tcx> for InferBorrowKind<'tcx> {
let PlaceBase::Upvar(upvar_id) = place_with_id.place.base else { return };
assert_eq!(self.closure_def_id, upvar_id.closure_expr_id);

let span = self.fcx.tcx.hir_span(diag_expr_id);
let place = self.fcx.normalize_capture_place(span, place_with_id.place.clone());

self.capture_information.push((
place_with_id.place.clone(),
place,
ty::CaptureInfo {
capture_kind_expr_id: Some(diag_expr_id),
path_expr_id: Some(diag_expr_id),
Expand All @@ -2074,8 +2125,11 @@ impl<'tcx> euv::Delegate<'tcx> for InferBorrowKind<'tcx> {
let PlaceBase::Upvar(upvar_id) = place_with_id.place.base else { return };
assert_eq!(self.closure_def_id, upvar_id.closure_expr_id);

let span = self.fcx.tcx.hir_span(diag_expr_id);
let place = self.fcx.normalize_capture_place(span, place_with_id.place.clone());

self.capture_information.push((
place_with_id.place.clone(),
place,
ty::CaptureInfo {
capture_kind_expr_id: Some(diag_expr_id),
path_expr_id: Some(diag_expr_id),
Expand All @@ -2097,14 +2151,16 @@ impl<'tcx> euv::Delegate<'tcx> for InferBorrowKind<'tcx> {
// The region here will get discarded/ignored
let capture_kind = ty::UpvarCapture::ByRef(bk);

let span = self.fcx.tcx.hir_span(diag_expr_id);
let place = self.fcx.normalize_capture_place(span, place_with_id.place.clone());

// We only want repr packed restriction to be applied to reading references into a packed
// struct, and not when the data is being moved. Therefore we call this method here instead
// of in `restrict_capture_precision`.
let (place, mut capture_kind) =
restrict_repr_packed_field_ref_capture(place_with_id.place.clone(), capture_kind);
let (place, mut capture_kind) = restrict_repr_packed_field_ref_capture(place, capture_kind);

// Raw pointers don't inherit mutability
if place_with_id.place.deref_tys().any(Ty::is_raw_ptr) {
if place.deref_tys().any(Ty::is_raw_ptr) {
capture_kind = ty::UpvarCapture::ByRef(ty::BorrowKind::Immutable);
}

Expand Down
26 changes: 0 additions & 26 deletions tests/crashes/120911.rs

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,24 +1,34 @@
//@ known-bug: #120811
//@ check-pass

// Regression test for #120811.
// Used to ICE during closure capture analysis.

trait Container {
type Item<'a>;
}

impl Container for () {
type Item<'a> = ();
}

struct Exchange<C, F> {
_marker: std::marker::PhantomData<(C, F)>,
}

fn exchange<C, F>(_: F) -> Exchange<C, F>
where
C: Container,
for<'a> F: FnMut(&C::Item<'a>),
{
unimplemented!()
}

trait Parallelization<C> {}

impl<C, F> Parallelization<C> for Exchange<C, F> {}

fn unary_frontier<P: Parallelization<()>>(_: P) {}

fn main() {
let exchange = exchange(|_| ());
let _ = || {
Expand Down
30 changes: 30 additions & 0 deletions tests/ui/traits/next-solver/normalize-capture-place-151579.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Regression test for #151579
//@ compile-flags: -Znext-solver=globally
//@ edition:2018

#![deny(rust_2021_incompatible_closure_captures)]

struct Dummy;

trait Trait {
type Assoc;
}

impl Trait for Dummy {
type Assoc = (*mut i32,);
}

struct SyncPointer(<Dummy as Trait>::Assoc);
unsafe impl Sync for SyncPointer {}

fn test_assoc_capture(a: SyncPointer) {
let _ = move || {
//~^ ERROR: changes to closure capture
let _x = a.0.0;
};
}

fn main() {
let ptr = SyncPointer((std::ptr::null_mut(),));
test_assoc_capture(ptr);
}
23 changes: 23 additions & 0 deletions tests/ui/traits/next-solver/normalize-capture-place-151579.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error: changes to closure capture in Rust 2021 will affect which traits the closure implements
--> $DIR/normalize-capture-place-151579.rs:21:13
|
LL | let _ = move || {
| ^^^^^^^ in Rust 2018, this closure implements `Sync` as `a` implements `Sync`, but in Rust 2021, this closure will no longer implement `Sync` because `a` is not fully captured and `a.0.0` does not implement `Sync`
LL |
LL | let _x = a.0.0;
| ----- in Rust 2018, this closure captures all of `a`, but in Rust 2021, it will only capture `a.0.0`
|
= note: for more information, see <https://doc.rust-lang.org/edition-guide/rust-2021/disjoint-capture-in-closures.html>
note: the lint level is defined here
--> $DIR/normalize-capture-place-151579.rs:5:9
|
LL | #![deny(rust_2021_incompatible_closure_captures)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: add a dummy let to cause `a` to be fully captured
|
LL ~ let _ = move || {
LL + let _ = &a;
|

error: aborting due to 1 previous error

Loading