Skip to content

Commit 5771665

Browse files
committed
Auto merge of rust-lang#145240 - Zalathar:rollup-7r97lia, r=Zalathar
Rollup of 5 pull requests Successful merges: - rust-lang#135331 (Reject relaxed bounds inside associated type bounds (ATB)) - rust-lang#144156 (Check coroutine upvars in dtorck constraint) - rust-lang#145091 (`NllRegionVariableOrigin` remove `from_forall`) - rust-lang#145194 (Ignore coroutine witness type region args in auto trait confirmation) - rust-lang#145225 (Fix macro infinite recursion test to not trigger warning about semicolon in expr) r? `@ghost` `@rustbot` modify labels: rollup
2 parents a6620a4 + 3ce0ee2 commit 5771665

22 files changed

+239
-82
lines changed

compiler/rustc_ast_lowering/src/lib.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ enum RelaxedBoundPolicy<'a> {
296296
enum RelaxedBoundForbiddenReason {
297297
TraitObjectTy,
298298
SuperTrait,
299+
AssocTyBounds,
299300
LateBoundVarsInScope,
300301
}
301302

@@ -1109,9 +1110,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
11091110
&*self.arena.alloc(self.ty(constraint.span, hir::TyKind::Err(guar)));
11101111
hir::AssocItemConstraintKind::Equality { term: err_ty.into() }
11111112
} else {
1112-
// FIXME(#135229): These should be forbidden!
1113-
let bounds =
1114-
self.lower_param_bounds(bounds, RelaxedBoundPolicy::Allowed, itctx);
1113+
let bounds = self.lower_param_bounds(
1114+
bounds,
1115+
RelaxedBoundPolicy::Forbidden(RelaxedBoundForbiddenReason::AssocTyBounds),
1116+
itctx,
1117+
);
11151118
hir::AssocItemConstraintKind::Bound { bounds }
11161119
}
11171120
}
@@ -2124,7 +2127,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
21242127
diag.emit();
21252128
return;
21262129
}
2127-
RelaxedBoundForbiddenReason::LateBoundVarsInScope => {}
2130+
RelaxedBoundForbiddenReason::AssocTyBounds
2131+
| RelaxedBoundForbiddenReason::LateBoundVarsInScope => {}
21282132
};
21292133
}
21302134
}

compiler/rustc_borrowck/src/handle_placeholders.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ fn region_definitions<'tcx>(
157157
for info in var_infos.iter() {
158158
let origin = match info.origin {
159159
RegionVariableOrigin::Nll(origin) => origin,
160-
_ => NllRegionVariableOrigin::Existential { from_forall: false, name: None },
160+
_ => NllRegionVariableOrigin::Existential { name: None },
161161
};
162162

163163
let definition = RegionDefinition { origin, universe: info.universe, external_name: None };

compiler/rustc_borrowck/src/region_infer/mod.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,10 +1939,15 @@ impl<'tcx> RegionInferenceContext<'tcx> {
19391939
//
19401940
// and here we prefer to blame the source (the y = x statement).
19411941
let blame_source = match from_region_origin {
1942-
NllRegionVariableOrigin::FreeRegion
1943-
| NllRegionVariableOrigin::Existential { from_forall: false, name: _ } => true,
1944-
NllRegionVariableOrigin::Placeholder(_)
1945-
| NllRegionVariableOrigin::Existential { from_forall: true, name: _ } => false,
1942+
NllRegionVariableOrigin::FreeRegion => true,
1943+
NllRegionVariableOrigin::Placeholder(_) => false,
1944+
// `'existential: 'whatever` never results in a region error by itself.
1945+
// We may always infer it to `'static` afterall. This means while an error
1946+
// path may go through an existential, these existentials are never the
1947+
// `from_region`.
1948+
NllRegionVariableOrigin::Existential { name: _ } => {
1949+
unreachable!("existentials can outlive everything")
1950+
}
19461951
};
19471952

19481953
// To pick a constraint to blame, we organize constraints by how interesting we expect them

compiler/rustc_borrowck/src/renumber.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ impl<'a, 'tcx> RegionRenumberer<'a, 'tcx> {
6666
T: TypeFoldable<TyCtxt<'tcx>>,
6767
F: Fn() -> RegionCtxt,
6868
{
69-
let origin = NllRegionVariableOrigin::Existential { from_forall: false, name: None };
69+
let origin = NllRegionVariableOrigin::Existential { name: None };
7070
fold_regions(self.infcx.tcx, value, |_region, _depth| {
7171
self.infcx.next_nll_region_var(origin, || region_ctxt_fn())
7272
})

compiler/rustc_borrowck/src/type_check/relate_tys.rs

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ impl<'a, 'b, 'tcx> NllTypeRelating<'a, 'b, 'tcx> {
216216
*ex_reg_var
217217
} else {
218218
let ex_reg_var =
219-
self.next_existential_region_var(true, br.kind.get_name(infcx.infcx.tcx));
219+
self.next_existential_region_var(br.kind.get_name(infcx.infcx.tcx));
220220
debug!(?ex_reg_var);
221221
reg_map.insert(br, ex_reg_var);
222222

@@ -244,17 +244,9 @@ impl<'a, 'b, 'tcx> NllTypeRelating<'a, 'b, 'tcx> {
244244
}
245245

246246
#[instrument(skip(self), level = "debug")]
247-
fn next_existential_region_var(
248-
&mut self,
249-
from_forall: bool,
250-
name: Option<Symbol>,
251-
) -> ty::Region<'tcx> {
252-
let origin = NllRegionVariableOrigin::Existential { name, from_forall };
253-
254-
let reg_var =
255-
self.type_checker.infcx.next_nll_region_var(origin, || RegionCtxt::Existential(name));
256-
257-
reg_var
247+
fn next_existential_region_var(&mut self, name: Option<Symbol>) -> ty::Region<'tcx> {
248+
let origin = NllRegionVariableOrigin::Existential { name };
249+
self.type_checker.infcx.next_nll_region_var(origin, || RegionCtxt::Existential(name))
258250
}
259251

260252
#[instrument(skip(self), level = "debug")]

compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,10 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
199199
// However, this can easily get out of sync! Ideally, we would perform this step
200200
// where we are guaranteed to catch *all* bounds like in
201201
// `Self::lower_poly_trait_ref`. List of concrete issues:
202-
// FIXME(more_maybe_bounds): We don't call this for e.g., trait object tys or
203-
// supertrait bounds!
202+
// FIXME(more_maybe_bounds): We don't call this for trait object tys, supertrait
203+
// bounds or associated type bounds (ATB)!
204204
// FIXME(trait_alias, #143122): We don't call it for the RHS. Arguably however,
205-
// AST lowering should reject them outright.
206-
// FIXME(associated_type_bounds): We don't call this for them. However, AST
207-
// lowering should reject them outright (#135229).
205+
// AST lowering should reject them outright.
208206
let bounds = collect_relaxed_bounds(hir_bounds, self_ty_where_predicates);
209207
self.check_and_report_invalid_relaxed_bounds(bounds);
210208
}

compiler/rustc_infer/src/infer/mod.rs

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -485,17 +485,6 @@ pub enum NllRegionVariableOrigin {
485485

486486
Existential {
487487
name: Option<Symbol>,
488-
/// If this is true, then this variable was created to represent a lifetime
489-
/// bound in a `for` binder. For example, it might have been created to
490-
/// represent the lifetime `'a` in a type like `for<'a> fn(&'a u32)`.
491-
/// Such variables are created when we are trying to figure out if there
492-
/// is any valid instantiation of `'a` that could fit into some scenario.
493-
///
494-
/// This is used to inform error reporting: in the case that we are trying to
495-
/// determine whether there is any valid instantiation of a `'a` variable that meets
496-
/// some constraint C, we want to blame the "source" of that `for` type,
497-
/// rather than blaming the source of the constraint C.
498-
from_forall: bool,
499488
},
500489
}
501490

compiler/rustc_trait_selection/src/traits/query/dropck_outlives.rs

Lines changed: 50 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -319,39 +319,65 @@ pub fn dtorck_constraint_for_ty_inner<'tcx>(
319319
}
320320

321321
ty::Coroutine(def_id, args) => {
322-
// rust-lang/rust#49918: types can be constructed, stored
323-
// in the interior, and sit idle when coroutine yields
324-
// (and is subsequently dropped).
322+
// rust-lang/rust#49918: Locals can be stored across await points in the coroutine,
323+
// called interior/witness types. Since we do not compute these witnesses until after
324+
// building MIR, we consider all coroutines to unconditionally require a drop during
325+
// MIR building. However, considering the coroutine to unconditionally require a drop
326+
// here may unnecessarily require its upvars' regions to be live when they don't need
327+
// to be, leading to borrowck errors: <https://github.com/rust-lang/rust/issues/116242>.
325328
//
326-
// It would be nice to descend into interior of a
327-
// coroutine to determine what effects dropping it might
328-
// have (by looking at any drop effects associated with
329-
// its interior).
329+
// Here, we implement a more precise approximation for the coroutine's dtorck constraint
330+
// by considering whether any of the interior types needs drop. Note that this is still
331+
// an approximation because the coroutine interior has its regions erased, so we must add
332+
// *all* of the upvars to live types set if we find that *any* interior type needs drop.
333+
// This is because any of the regions captured in the upvars may be stored in the interior,
334+
// which then has its regions replaced by a binder (conceptually erasing the regions),
335+
// so there's no way to enforce that the precise region in the interior type is live
336+
// since we've lost that information by this point.
330337
//
331-
// However, the interior's representation uses things like
332-
// CoroutineWitness that explicitly assume they are not
333-
// traversed in such a manner. So instead, we will
334-
// simplify things for now by treating all coroutines as
335-
// if they were like trait objects, where its upvars must
336-
// all be alive for the coroutine's (potential)
337-
// destructor.
338+
// Note also that this check requires that the coroutine's upvars are use-live, since
339+
// a region from a type that does not have a destructor that was captured in an upvar
340+
// may flow into an interior type with a destructor. This is stronger than requiring
341+
// the upvars are drop-live.
338342
//
339-
// In particular, skipping over `_interior` is safe
340-
// because any side-effects from dropping `_interior` can
341-
// only take place through references with lifetimes
342-
// derived from lifetimes attached to the upvars and resume
343-
// argument, and we *do* incorporate those here.
343+
// For example, if we capture two upvar references `&'1 (), &'2 ()` and have some type
344+
// in the interior, `for<'r> { NeedsDrop<'r> }`, we have no way to tell whether the
345+
// region `'r` came from the `'1` or `'2` region, so we require both are live. This
346+
// could even be unnecessary if `'r` was actually a `'static` region or some region
347+
// local to the coroutine! That's why it's an approximation.
344348
let args = args.as_coroutine();
345349

346-
// While we conservatively assume that all coroutines require drop
347-
// to avoid query cycles during MIR building, we can check the actual
348-
// witness during borrowck to avoid unnecessary liveness constraints.
350+
// Note that we don't care about whether the resume type has any drops since this is
351+
// redundant; there is no storage for the resume type, so if it is actually stored
352+
// in the interior, we'll already detect the need for a drop by checking the interior.
349353
let typing_env = tcx.erase_regions(typing_env);
350-
if tcx.mir_coroutine_witnesses(def_id).is_some_and(|witness| {
354+
let needs_drop = tcx.mir_coroutine_witnesses(def_id).is_some_and(|witness| {
351355
witness.field_tys.iter().any(|field| field.ty.needs_drop(tcx, typing_env))
352-
}) {
356+
});
357+
if needs_drop {
358+
// Pushing types directly to `constraints.outlives` is equivalent
359+
// to requiring them to be use-live, since if we were instead to
360+
// recurse on them like we do below, we only end up collecting the
361+
// types that are relevant for drop-liveness.
353362
constraints.outlives.extend(args.upvar_tys().iter().map(ty::GenericArg::from));
354363
constraints.outlives.push(args.resume_ty().into());
364+
} else {
365+
// Even if a witness type doesn't need a drop, we still require that
366+
// the upvars are drop-live. This is only needed if we aren't already
367+
// counting *all* of the upvars as use-live above, since use-liveness
368+
// is a *stronger requirement* than drop-liveness. Recursing here
369+
// unconditionally would just be collecting duplicated types for no
370+
// reason.
371+
for ty in args.upvar_tys() {
372+
dtorck_constraint_for_ty_inner(
373+
tcx,
374+
typing_env,
375+
span,
376+
depth + 1,
377+
ty,
378+
constraints,
379+
);
380+
}
355381
}
356382
}
357383

compiler/rustc_trait_selection/src/traits/select/mod.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2333,10 +2333,23 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
23332333

23342334
ty::Coroutine(def_id, args) => {
23352335
let ty = self.infcx.shallow_resolve(args.as_coroutine().tupled_upvars_ty());
2336+
let tcx = self.tcx();
23362337
let witness = Ty::new_coroutine_witness(
2337-
self.tcx(),
2338+
tcx,
23382339
def_id,
2339-
self.tcx().mk_args(args.as_coroutine().parent_args()),
2340+
ty::GenericArgs::for_item(tcx, def_id, |def, _| match def.kind {
2341+
// HACK: Coroutine witnesse types are lifetime erased, so they
2342+
// never reference any lifetime args from the coroutine. We erase
2343+
// the regions here since we may get into situations where a
2344+
// coroutine is recursively contained within itself, leading to
2345+
// witness types that differ by region args. This means that
2346+
// cycle detection in fulfillment will not kick in, which leads
2347+
// to unnecessary overflows in async code. See the issue:
2348+
// <https://github.com/rust-lang/rust/issues/145151>.
2349+
ty::GenericParamDefKind::Lifetime => tcx.lifetimes.re_erased.into(),
2350+
ty::GenericParamDefKind::Type { .. }
2351+
| ty::GenericParamDefKind::Const { .. } => args[def.index as usize],
2352+
}),
23402353
);
23412354
ty::Binder::dummy(AutoImplConstituents {
23422355
types: [ty].into_iter().chain(iter::once(witness)).collect(),
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error[E0597]: `y` does not live long enough
2+
--> $DIR/drop-live-upvar-2.rs:31:26
3+
|
4+
LL | let y = ();
5+
| - binding `y` declared here
6+
LL | drop_me = Droppy(&y);
7+
| ^^ borrowed value does not live long enough
8+
...
9+
LL | }
10+
| - `y` dropped here while still borrowed
11+
LL | }
12+
| - borrow might be used here, when `fut` is dropped and runs the destructor for coroutine
13+
|
14+
= note: values in a scope are dropped in the opposite order they are defined
15+
16+
error: aborting due to 1 previous error
17+
18+
For more information about this error, try `rustc --explain E0597`.

0 commit comments

Comments
 (0)