From 503bc7ca14146a88c5606a1bb82d1a3518936cae Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 4 Dec 2022 14:43:30 +0000 Subject: [PATCH] Report change in RPITIT lifetime capture clauses. --- .../src/check/compare_method.rs | 230 ++++++++++++------ compiler/rustc_middle/src/ty/sty.rs | 8 + .../in-trait/method-signature-matches.stderr | 6 +- .../impl-trait/in-trait/signature-mismatch.rs | 53 +++- .../in-trait/signature-mismatch.stderr | 69 +++++- 5 files changed, 280 insertions(+), 86 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/compare_method.rs b/compiler/rustc_hir_analysis/src/check/compare_method.rs index 82a77416a190c..6cee8cd7994b2 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_method.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_method.rs @@ -367,12 +367,9 @@ pub fn collect_trait_impl_trait_tys<'tcx>( let impl_sig = ocx.normalize( &norm_cause, param_env, - infcx.replace_bound_vars_with_fresh_vars( - return_span, - infer::HigherRankedType, - tcx.fn_sig(impl_m.def_id), - ), + tcx.liberate_late_bound_regions(impl_m.def_id, tcx.fn_sig(impl_m.def_id)), ); + debug!(?impl_sig); let impl_return_ty = impl_sig.output(); // Normalize the trait signature with liberated bound vars, passing it through @@ -380,13 +377,19 @@ pub fn collect_trait_impl_trait_tys<'tcx>( // them with inference variables. // We will use these inference variables to collect the hidden types of RPITITs. let mut collector = ImplTraitInTraitCollector::new(&ocx, return_span, param_env, impl_m_hir_id); - let unnormalized_trait_sig = tcx - .liberate_late_bound_regions( - impl_m.def_id, + let unnormalized_trait_sig = infcx + .replace_bound_vars_with_fresh_vars( + return_span, + infer::HigherRankedType, tcx.bound_fn_sig(trait_m.def_id).subst(tcx, trait_to_placeholder_substs), ) .fold_with(&mut collector); + if collector.types.is_empty() { + return Ok(&*tcx.arena.alloc(FxHashMap::default())); + } + let trait_sig = ocx.normalize(&norm_cause, param_env, unnormalized_trait_sig); + debug!(?trait_sig); let trait_return_ty = trait_sig.output(); let wf_tys = FxIndexSet::from_iter( @@ -422,7 +425,7 @@ pub fn collect_trait_impl_trait_tys<'tcx>( } } - debug!(?trait_sig, ?impl_sig, "equating function signatures"); + debug!("equating function signatures"); let trait_fty = tcx.mk_fn_ptr(ty::Binder::dummy(trait_sig)); let impl_fty = tcx.mk_fn_ptr(ty::Binder::dummy(impl_sig)); @@ -461,86 +464,167 @@ pub fn collect_trait_impl_trait_tys<'tcx>( // Finally, resolve all regions. This catches wily misuses of // lifetime parameters. + debug!("check_region_obligations"); let outlives_environment = OutlivesEnvironment::with_bounds( param_env, Some(infcx), infcx.implied_bounds_tys(param_env, impl_m_hir_id, wf_tys), ); + debug!(region_bound_pairs=?outlives_environment.region_bound_pairs()); infcx.check_region_obligations_and_report_errors( impl_m.def_id.expect_local(), &outlives_environment, ); + debug!("resolve collected"); let mut collected_tys = FxHashMap::default(); - for (def_id, (ty, substs)) in collector.types { - match infcx.fully_resolve(ty) { - Ok(ty) => { - // `ty` contains free regions that we created earlier while liberating the - // trait fn signature. However, projection normalization expects `ty` to - // contains `def_id`'s early-bound regions. - let id_substs = InternalSubsts::identity_for_item(tcx, def_id); - debug!(?id_substs, ?substs); - let map: FxHashMap, ty::GenericArg<'tcx>> = - std::iter::zip(substs, id_substs).collect(); - debug!(?map); - - // NOTE(compiler-errors): RPITITs, like all other RPITs, have early-bound - // region substs that are synthesized during AST lowering. These are substs - // that are appended to the parent substs (trait and trait method). However, - // we're trying to infer the unsubstituted type value of the RPITIT inside - // the *impl*, so we can later use the impl's method substs to normalize - // an RPITIT to a concrete type (`confirm_impl_trait_in_trait_candidate`). - // - // Due to the design of RPITITs, during AST lowering, we have no idea that - // an impl method corresponds to a trait method with RPITITs in it. Therefore, - // we don't have a list of early-bound region substs for the RPITIT in the impl. - // Since early region parameters are index-based, we can't just rebase these - // (trait method) early-bound region substs onto the impl, and there's no - // guarantee that the indices from the trait substs and impl substs line up. - // So to fix this, we subtract the number of trait substs and add the number of - // impl substs to *renumber* these early-bound regions to their corresponding - // indices in the impl's substitutions list. - // - // Also, we only need to account for a difference in trait and impl substs, - // since we previously enforce that the trait method and impl method have the - // same generics. - let num_trait_substs = trait_to_impl_substs.len(); - let num_impl_substs = tcx.generics_of(impl_m.container_id(tcx)).params.len(); - let ty = tcx.fold_regions(ty, |region, _| { - match region.kind() { - // Remap all free regions, which correspond to late-bound regions in the function. - ty::ReFree(_) => {} - // Remap early-bound regions as long as they don't come from the `impl` itself. - ty::ReEarlyBound(ebr) if tcx.parent(ebr.def_id) != impl_m.container_id(tcx) => {} - _ => return region, - } - let Some(ty::ReEarlyBound(e)) = map.get(®ion.into()).map(|r| r.expect_region().kind()) - else { - tcx - .sess - .delay_span_bug( - return_span, - "expected ReFree to map to ReEarlyBound" - ); - return tcx.lifetimes.re_static; - }; - tcx.mk_region(ty::ReEarlyBound(ty::EarlyBoundRegion { - def_id: e.def_id, - name: e.name, - index: (e.index as usize - num_trait_substs + num_impl_substs) as u32, - })) - }); - debug!(%ty); - collected_tys.insert(def_id, ty); + for (def_id, (raw_ty, substs)) in collector.types { + debug!(?def_id); + + let substs = match infcx.fully_resolve(substs) { + Ok(substs) => substs, + Err(err) => { + collected_tys.insert( + def_id, + tcx.ty_error_with_message( + return_span, + &format!("could not fully resolve: {substs:?} => {err:?}"), + ), + ); + continue; } + }; + debug!(?substs); + + let raw_ty = match infcx.fully_resolve(raw_ty) { + Ok(raw_ty) => raw_ty, Err(err) => { - let reported = tcx.sess.delay_span_bug( - return_span, - format!("could not fully resolve: {ty} => {err:?}"), + collected_tys.insert( + def_id, + tcx.ty_error_with_message( + return_span, + &format!("could not fully resolve: {raw_ty} => {err:?}"), + ), ); - collected_tys.insert(def_id, tcx.ty_error_with_guaranteed(reported)); + continue; } + }; + debug!(?raw_ty); + + // `raw_ty` contains free regions that we created earlier while liberating the + // trait fn signature. However, projection normalization expects `raw_ty` to + // contains `def_id`'s early-bound regions. + let id_substs = InternalSubsts::identity_for_item(tcx, def_id); + debug!(?id_substs, ?substs); + + let variances = tcx.variances_of(def_id); + debug!(?variances); + + // Opaque types may only use regions that are bound. So for + // ```rust + // type Foo<'a, 'b, 'c> = impl Trait<'a> + 'b; + // ``` + // we may not use `'c` in the hidden type. + let map: FxHashMap, ty::GenericArg<'tcx>> = + std::iter::zip(substs, id_substs) + .filter(|(_, v)| { + let ty::GenericArgKind::Lifetime(lt) = v.unpack() else { return true }; + let ty::ReEarlyBound(ebr) = lt.kind() else { bug!() }; + variances[ebr.index as usize] == ty::Variance::Invariant + }) + .collect(); + debug!(?map); + + // NOTE(compiler-errors): RPITITs, like all other RPITs, have early-bound + // region substs that are synthesized during AST lowering. These are substs + // that are appended to the parent substs (trait and trait method). However, + // we're trying to infer the unsubstituted type value of the RPITIT inside + // the *impl*, so we can later use the impl's method substs to normalize + // an RPITIT to a concrete type (`confirm_impl_trait_in_trait_candidate`). + // + // Due to the design of RPITITs, during AST lowering, we have no idea that + // an impl method corresponds to a trait method with RPITITs in it. Therefore, + // we don't have a list of early-bound region substs for the RPITIT in the impl. + // Since early region parameters are index-based, we can't just rebase these + // (trait method) early-bound region substs onto the impl, and there's no + // guarantee that the indices from the trait substs and impl substs line up. + // So to fix this, we subtract the number of trait substs and add the number of + // impl substs to *renumber* these early-bound regions to their corresponding + // indices in the impl's substitutions list. + // + // Also, we only need to account for a difference in trait and impl substs, + // since we previously enforce that the trait method and impl method have the + // same generics. + let num_trait_substs = trait_to_impl_substs.len(); + let num_impl_substs = tcx.generics_of(impl_m.def_id).parent_count; + debug!(?num_trait_substs, ?num_impl_substs); + + let mut bad_regions = vec![]; + + let ty = tcx.fold_regions(raw_ty, |region, _| { + debug!(?region); + match region.kind() { + // Remap all free regions, which correspond to late-bound regions in the function. + ty::ReFree(_) => {} + // Remap early-bound regions as long as they don't come from the `impl` itself. + ty::ReEarlyBound(ebr) if (ebr.index as usize) >= num_impl_substs => {} + _ => return region, + } + debug!(mapped = ?map.get(®ion.into())); + let Some(ty::ReEarlyBound(e)) = map.get(®ion.into()).map(|r| r.expect_region().kind()) + else { + bad_regions.push(region); + return tcx.lifetimes.re_static; + }; + tcx.mk_region(ty::ReEarlyBound(ty::EarlyBoundRegion { + def_id: e.def_id, + name: e.name, + index: (e.index as usize - num_trait_substs + num_impl_substs) as u32, + })) + }); + + if !bad_regions.is_empty() { + let mut err = tcx.sess.struct_span_err( + return_span, + "`impl` item return type captures lifetime that doesn't appear in `trait` item bounds", + ); + for r in bad_regions { + let span = match r.kind() { + ty::ReEarlyBound(ebr) => tcx.def_span(ebr.def_id), + ty::ReFree(fr) => fr.bound_region.opt_span(tcx).unwrap(), + _ => bug!(), + }; + let name = if r.has_name() { + format!("lifetime `{r}`") + } else { + "the anonymous lifetime".to_string() + }; + err.span_label(span, format!("type `{raw_ty}` captures {name} defined here")); + } + + let generics = tcx.generics_of(def_id); + match &generics.params[..] { + [] => { + err.span_label(tcx.def_span(def_id), "type declared not to capture lifetimes"); + } + [param] => { + err.span_label( + tcx.def_span(param.def_id), + "type can only capture this lifetime", + ); + } + params => { + err.span_labels( + params.iter().map(|param| tcx.def_span(param.def_id)), + "type can only capture lifetimes enumerated here", + ); + } + }; + err.emit(); } + + debug!(?ty); + collected_tys.insert(def_id, ty); } Ok(&*tcx.arena.alloc(collected_tys)) diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 5984686044b73..21e18b5aa423b 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -100,6 +100,14 @@ impl BoundRegionKind { None } + + pub fn opt_span(&self, tcx: TyCtxt<'_>) -> Option { + match *self { + ty::BrAnon(_, opt_span) => opt_span, + ty::BrNamed(def_id, _) => Some(tcx.def_span(def_id)), + ty::BrEnv => None, + } + } } pub trait Article { diff --git a/src/test/ui/impl-trait/in-trait/method-signature-matches.stderr b/src/test/ui/impl-trait/in-trait/method-signature-matches.stderr index 2b32c52c829ec..8e800de366bf6 100644 --- a/src/test/ui/impl-trait/in-trait/method-signature-matches.stderr +++ b/src/test/ui/impl-trait/in-trait/method-signature-matches.stderr @@ -67,7 +67,7 @@ LL | fn early<'late, T>(_: &'late ()) {} | - ^^^^^^^^^ | | | | | expected type parameter `T`, found `()` - | | help: change the parameter type to match the trait: `&'early T` + | | help: change the parameter type to match the trait: `&T` | this type parameter | note: type in trait @@ -75,8 +75,8 @@ note: type in trait | LL | fn early<'early, T>(x: &'early T) -> impl Sized; | ^^^^^^^^^ - = note: expected fn pointer `fn(&'early T)` - found fn pointer `fn(&())` + = note: expected fn pointer `fn(&T)` + found fn pointer `fn(&'late ())` error: aborting due to 5 previous errors diff --git a/src/test/ui/impl-trait/in-trait/signature-mismatch.rs b/src/test/ui/impl-trait/in-trait/signature-mismatch.rs index 90682631aa032..2000ed5b342f9 100644 --- a/src/test/ui/impl-trait/in-trait/signature-mismatch.rs +++ b/src/test/ui/impl-trait/in-trait/signature-mismatch.rs @@ -5,17 +5,68 @@ use std::future::Future; +trait Captures<'a> {} +impl Captures<'_> for T {} + pub trait AsyncTrait { fn async_fn(&self, buff: &[u8]) -> impl Future>; + fn async_fn_early<'a: 'a>(&self, buff: &'a [u8]) -> impl Future>; + fn async_fn_multiple<'a>(&'a self, buff: &[u8]) + -> impl Future> + Captures<'a>; + fn async_fn_reduce_outlive<'a, T>( + &'a self, + buff: &[u8], + t: T, + ) -> impl Future> + 'a; + fn async_fn_reduce<'a, T>( + &'a self, + buff: &[u8], + t: T, + ) -> impl Future> + Captures<'a>; } pub struct Struct; impl AsyncTrait for Struct { fn async_fn<'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { - //~^ ERROR `impl` item signature doesn't match `trait` item signature + //~^ ERROR `impl` item return type captures lifetime that doesn't appear in `trait` + async move { buff.to_vec() } + } + + fn async_fn_early<'a: 'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { + //~^ ERROR `impl` item return type captures lifetime that doesn't appear in `trait` async move { buff.to_vec() } } + + fn async_fn_multiple<'a, 'b>( + &'a self, + buff: &'b [u8], + ) -> impl Future> + Captures<'a> + Captures<'b> { + //~^ ERROR `impl` item return type captures lifetime that doesn't appear in `trait` + //~| ERROR lifetime mismatch + async move { buff.to_vec() } + } + + fn async_fn_reduce_outlive<'a, 'b, T>( + &'a self, + buff: &'b [u8], + t: T, + ) -> impl Future> { + //~^ ERROR the parameter type `T` may not live long enough + async move { + let _t = t; + vec![] + } + } + + // OK: We remove the `Captures<'a>`, providing a guarantee that we don't capture `'a`, + // but we still fulfill the `Captures<'a>` trait bound. + fn async_fn_reduce<'a, 'b, T>(&'a self, buff: &'b [u8], t: T) -> impl Future> { + async move { + let _t = t; + vec![] + } + } } fn main() {} diff --git a/src/test/ui/impl-trait/in-trait/signature-mismatch.stderr b/src/test/ui/impl-trait/in-trait/signature-mismatch.stderr index 6663d7faa1e57..fbc815e8ad3ee 100644 --- a/src/test/ui/impl-trait/in-trait/signature-mismatch.stderr +++ b/src/test/ui/impl-trait/in-trait/signature-mismatch.stderr @@ -1,16 +1,67 @@ -error: `impl` item signature doesn't match `trait` item signature - --> $DIR/signature-mismatch.rs:15:5 +error: `impl` item return type captures lifetime that doesn't appear in `trait` item bounds + --> $DIR/signature-mismatch.rs:31:47 | LL | fn async_fn(&self, buff: &[u8]) -> impl Future>; - | ----------------------------------------------------------------- expected `fn(&'1 Struct, &'2 [u8]) -> impl Future> + 'static` + | ----------------------------- type declared not to capture lifetimes ... LL | fn async_fn<'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&'1 Struct, &'2 [u8]) -> impl Future> + '2` + | -- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | type `impl Future> + 'a` captures lifetime `'a` defined here + +error: `impl` item return type captures lifetime that doesn't appear in `trait` item bounds + --> $DIR/signature-mismatch.rs:36:57 + | +LL | fn async_fn_early<'a: 'a>(&self, buff: &'a [u8]) -> impl Future>; + | ----------------------------- type declared not to capture lifetimes +... +LL | fn async_fn_early<'a: 'a>(&self, buff: &'a [u8]) -> impl Future> + 'a { + | -- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | type `impl Future> + 'a` captures lifetime `'a` defined here + | type `impl Future> + 'a` captures lifetime `'a` defined here + +error[E0623]: lifetime mismatch + --> $DIR/signature-mismatch.rs:44:10 + | +LL | buff: &'b [u8], + | -------- this parameter and the return type are declared with different lifetimes... +LL | ) -> impl Future> + Captures<'a> + Captures<'b> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | ...but data from `self` is returned here + +error: `impl` item return type captures lifetime that doesn't appear in `trait` item bounds + --> $DIR/signature-mismatch.rs:44:10 + | +LL | -> impl Future> + Captures<'a>; + | -- type can only capture this lifetime +... +LL | fn async_fn_multiple<'a, 'b>( + | -- -- type `impl Future> + Captures<'a> + Captures<'b>` captures lifetime `'b` defined here + | | + | type `impl Future> + Captures<'a> + Captures<'b>` captures lifetime `'a` defined here +... +LL | ) -> impl Future> + Captures<'a> + Captures<'b> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0309]: the parameter type `T` may not live long enough + --> $DIR/signature-mismatch.rs:54:10 + | +LL | ) -> impl Future> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ...so that the type `impl Future>` will meet its required lifetime bounds... + | +note: ...that is required by this bound + --> $DIR/signature-mismatch.rs:20:42 + | +LL | ) -> impl Future> + 'a; + | ^^ +help: consider adding an explicit lifetime bound... | - = note: expected `fn(&'1 Struct, &'2 [u8]) -> impl Future> + 'static` - found `fn(&'1 Struct, &'2 [u8]) -> impl Future> + '2` - = help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait` - = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output +LL | fn async_fn_reduce_outlive<'a, 'b, T: 'a>( + | ++++ -error: aborting due to previous error +error: aborting due to 5 previous errors +Some errors have detailed explanations: E0309, E0623. +For more information about an error, try `rustc --explain E0309`.