From 5b2291cfa6ae9374ca22cf5d0aafde1a3f23ea57 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 10 Feb 2022 23:36:37 -0800 Subject: [PATCH 1/8] introduce gather_gat_bounds --- compiler/rustc_typeck/src/check/wfcheck.rs | 254 +++++++++++---------- 1 file changed, 131 insertions(+), 123 deletions(-) diff --git a/compiler/rustc_typeck/src/check/wfcheck.rs b/compiler/rustc_typeck/src/check/wfcheck.rs index 9bdeb3a679a3..9a269e024483 100644 --- a/compiler/rustc_typeck/src/check/wfcheck.rs +++ b/compiler/rustc_typeck/src/check/wfcheck.rs @@ -298,145 +298,49 @@ fn check_gat_where_clauses( for item in associated_items.in_definition_order().filter(|item| matches!(item.kind, ty::AssocKind::Fn)) { - // The clauses we that we would require from this function - let mut function_clauses = FxHashSet::default(); - let id = hir::HirId::make_owner(item.def_id.expect_local()); let param_env = tcx.param_env(item.def_id.expect_local()); - let sig = tcx.fn_sig(item.def_id); // Get the signature using placeholders. In our example, this would // convert the late-bound 'a into a free region. - let sig = tcx.liberate_late_bound_regions(item.def_id, sig); - // Collect the arguments that are given to this GAT in the return type - // of the function signature. In our example, the GAT in the return - // type is `::Item<'a>`, so 'a and Self are arguments. - let (regions, types) = - GATSubstCollector::visit(tcx, trait_item.def_id.to_def_id(), sig.output()); - - // If both regions and types are empty, then this GAT isn't in the - // return type, and we shouldn't try to do clause analysis - // (particularly, doing so would end up with an empty set of clauses, - // since the current method would require none, and we take the - // intersection of requirements of all methods) - if types.is_empty() && regions.is_empty() { - continue; - } + let sig = tcx.liberate_late_bound_regions(item.def_id, tcx.fn_sig(item.def_id)); // The types we can assume to be well-formed. In our example, this // would be &'a mut Self, from the first argument. let mut wf_tys = FxHashSet::default(); wf_tys.extend(sig.inputs()); - // For each region argument (e.g., 'a in our example), check for a - // relationship to the type arguments (e.g., Self). If there is an - // outlives relationship (`Self: 'a`), then we want to ensure that is - // reflected in a where clause on the GAT itself. - for (region, region_idx) in ®ions { - // Ignore `'static` lifetimes for the purpose of this lint: it's - // because we know it outlives everything and so doesn't give meaninful - // clues - if region.is_static() { - continue; - } - for (ty, ty_idx) in &types { - // In our example, requires that Self: 'a - if ty_known_to_outlive(tcx, id, param_env, &wf_tys, *ty, *region) { - debug!(?ty_idx, ?region_idx); - debug!("required clause: {} must outlive {}", ty, region); - // Translate into the generic parameters of the GAT. In - // our example, the type was Self, which will also be - // Self in the GAT. - let ty_param = generics.param_at(*ty_idx, tcx); - let ty_param = tcx.mk_ty(ty::Param(ty::ParamTy { - index: ty_param.index, - name: ty_param.name, - })); - // Same for the region. In our example, 'a corresponds - // to the 'me parameter. - let region_param = generics.param_at(*region_idx, tcx); - let region_param = tcx.mk_region(ty::ReEarlyBound(ty::EarlyBoundRegion { - def_id: region_param.def_id, - index: region_param.index, - name: region_param.name, - })); - // The predicate we expect to see. (In our example, - // `Self: 'me`.) - let clause = ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate( - ty_param, - region_param, - )); - let clause = tcx.mk_predicate(ty::Binder::dummy(clause)); - function_clauses.insert(clause); - } - } - } + // The clauses we that we would require from this function + let function_clauses = gather_gat_bounds( + tcx, + param_env, + id, + sig.output(), + &wf_tys, + trait_item.def_id, + generics, + ); - // For each region argument (e.g., 'a in our example), also check for a - // relationship to the other region arguments. If there is an - // outlives relationship, then we want to ensure that is - // reflected in a where clause on the GAT itself. - for (region_a, region_a_idx) in ®ions { - // Ignore `'static` lifetimes for the purpose of this lint: it's - // because we know it outlives everything and so doesn't give meaninful - // clues - if region_a.is_static() { - continue; - } - for (region_b, region_b_idx) in ®ions { - if region_a == region_b { - continue; - } - if region_b.is_static() { - continue; + if let Some(function_clauses) = function_clauses { + // Imagine we have: + // ``` + // trait Foo { + // type Bar<'me>; + // fn gimme(&self) -> Self::Bar<'_>; + // fn gimme_default(&self) -> Self::Bar<'static>; + // } + // ``` + // We only want to require clauses on `Bar` that we can prove from *all* functions (in this + // case, `'me` can be `static` from `gimme_default`) + match clauses.as_mut() { + Some(clauses) => { + clauses.drain_filter(|p| !function_clauses.contains(p)); } - - if region_known_to_outlive(tcx, id, param_env, &wf_tys, *region_a, *region_b) { - debug!(?region_a_idx, ?region_b_idx); - debug!("required clause: {} must outlive {}", region_a, region_b); - // Translate into the generic parameters of the GAT. - let region_a_param = generics.param_at(*region_a_idx, tcx); - let region_a_param = tcx.mk_region(ty::ReEarlyBound(ty::EarlyBoundRegion { - def_id: region_a_param.def_id, - index: region_a_param.index, - name: region_a_param.name, - })); - // Same for the region. - let region_b_param = generics.param_at(*region_b_idx, tcx); - let region_b_param = tcx.mk_region(ty::ReEarlyBound(ty::EarlyBoundRegion { - def_id: region_b_param.def_id, - index: region_b_param.index, - name: region_b_param.name, - })); - // The predicate we expect to see. - let clause = ty::PredicateKind::RegionOutlives(ty::OutlivesPredicate( - region_a_param, - region_b_param, - )); - let clause = tcx.mk_predicate(ty::Binder::dummy(clause)); - function_clauses.insert(clause); + None => { + clauses = Some(function_clauses); } } } - - // Imagine we have: - // ``` - // trait Foo { - // type Bar<'me>; - // fn gimme(&self) -> Self::Bar<'_>; - // fn gimme_default(&self) -> Self::Bar<'static>; - // } - // ``` - // We only want to require clauses on `Bar` that we can prove from *all* functions (in this - // case, `'me` can be `static` from `gimme_default`) - match clauses.as_mut() { - Some(clauses) => { - clauses.drain_filter(|p| !function_clauses.contains(p)); - } - None => { - clauses = Some(function_clauses); - } - } } // If there are any clauses that aren't provable, emit an error @@ -515,6 +419,110 @@ fn check_gat_where_clauses( } } +fn gather_gat_bounds<'tcx, T: TypeFoldable<'tcx>>( + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + item_hir: hir::HirId, + to_check: T, + wf_tys: &FxHashSet>, + gat_def_id: LocalDefId, + gat_generics: &'tcx ty::Generics, +) -> Option>> { + // The bounds we that we would require from this function + let mut bounds = FxHashSet::default(); + + let (regions, types) = GATSubstCollector::visit(tcx, gat_def_id.to_def_id(), to_check); + + // If both regions and types are empty, then this GAT isn't in the + // return type, and we shouldn't try to do clause analysis + // (particularly, doing so would end up with an empty set of clauses, + // since the current method would require none, and we take the + // intersection of requirements of all methods) + if types.is_empty() && regions.is_empty() { + return None; + } + + for (region_a, region_a_idx) in ®ions { + // Ignore `'static` lifetimes for the purpose of this lint: it's + // because we know it outlives everything and so doesn't give meaninful + // clues + if let ty::ReStatic = **region_a { + continue; + } + // For each region argument (e.g., 'a in our example), check for a + // relationship to the type arguments (e.g., Self). If there is an + // outlives relationship (`Self: 'a`), then we want to ensure that is + // reflected in a where clause on the GAT itself. + for (ty, ty_idx) in &types { + // In our example, requires that Self: 'a + if ty_known_to_outlive(tcx, item_hir, param_env, &wf_tys, *ty, *region_a) { + debug!(?ty_idx, ?region_a_idx); + debug!("required clause: {} must outlive {}", ty, region_a); + // Translate into the generic parameters of the GAT. In + // our example, the type was Self, which will also be + // Self in the GAT. + let ty_param = gat_generics.param_at(*ty_idx, tcx); + let ty_param = tcx + .mk_ty(ty::Param(ty::ParamTy { index: ty_param.index, name: ty_param.name })); + // Same for the region. In our example, 'a corresponds + // to the 'me parameter. + let region_param = gat_generics.param_at(*region_a_idx, tcx); + let region_param = + tcx.mk_region(ty::RegionKind::ReEarlyBound(ty::EarlyBoundRegion { + def_id: region_param.def_id, + index: region_param.index, + name: region_param.name, + })); + // The predicate we expect to see. (In our example, + // `Self: 'me`.) + let clause = + ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate(ty_param, region_param)); + let clause = tcx.mk_predicate(ty::Binder::dummy(clause)); + bounds.insert(clause); + } + } + + // For each region argument (e.g., 'a in our example), also check for a + // relationship to the other region arguments. If there is an + // outlives relationship, then we want to ensure that is + // reflected in a where clause on the GAT itself. + for (region_b, region_b_idx) in ®ions { + if ty::ReStatic == **region_b || region_a == region_b { + continue; + } + if region_known_to_outlive(tcx, item_hir, param_env, &wf_tys, *region_a, *region_b) { + debug!(?region_a_idx, ?region_b_idx); + debug!("required clause: {} must outlive {}", region_a, region_b); + // Translate into the generic parameters of the GAT. + let region_a_param = gat_generics.param_at(*region_a_idx, tcx); + let region_a_param = + tcx.mk_region(ty::RegionKind::ReEarlyBound(ty::EarlyBoundRegion { + def_id: region_a_param.def_id, + index: region_a_param.index, + name: region_a_param.name, + })); + // Same for the region. + let region_b_param = gat_generics.param_at(*region_b_idx, tcx); + let region_b_param = + tcx.mk_region(ty::RegionKind::ReEarlyBound(ty::EarlyBoundRegion { + def_id: region_b_param.def_id, + index: region_b_param.index, + name: region_b_param.name, + })); + // The predicate we expect to see. + let clause = ty::PredicateKind::RegionOutlives(ty::OutlivesPredicate( + region_a_param, + region_b_param, + )); + let clause = tcx.mk_predicate(ty::Binder::dummy(clause)); + bounds.insert(clause); + } + } + } + + Some(bounds) +} + /// Given a known `param_env` and a set of well formed types, can we prove that /// `ty` outlives `region`. fn ty_known_to_outlive<'tcx>( From 764839320c247b813528533c79d4b25a4f55f5fd Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 10 Feb 2022 23:42:28 -0800 Subject: [PATCH 2/8] rename some variables in gat wfcheck --- compiler/rustc_typeck/src/check/wfcheck.rs | 43 +++++++++------------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_typeck/src/check/wfcheck.rs b/compiler/rustc_typeck/src/check/wfcheck.rs index 9a269e024483..1864101467b7 100644 --- a/compiler/rustc_typeck/src/check/wfcheck.rs +++ b/compiler/rustc_typeck/src/check/wfcheck.rs @@ -275,30 +275,27 @@ pub fn check_trait_item(tcx: TyCtxt<'_>, def_id: LocalDefId) { /// fn next<'a>(&'a mut self) -> Option>; /// } /// ``` -fn check_gat_where_clauses( - tcx: TyCtxt<'_>, - trait_item: &hir::TraitItem<'_>, - encl_trait_def_id: DefId, -) { - let item = tcx.associated_item(trait_item.def_id); +fn check_gat_where_clauses(tcx: TyCtxt<'_>, gat_hir: &hir::TraitItem<'_>, gat_def_id: DefId) { + let gat_item = tcx.associated_item(gat_def_id); + let gat_def_id = gat_hir.def_id; // If the current trait item isn't a type, it isn't a GAT - if !matches!(item.kind, ty::AssocKind::Type) { + if !matches!(gat_item.kind, ty::AssocKind::Type) { return; } - let generics: &ty::Generics = tcx.generics_of(trait_item.def_id); + let gat_generics: &ty::Generics = tcx.generics_of(gat_def_id); // If the current associated type doesn't have any (own) params, it's not a GAT // FIXME(jackh726): we can also warn in the more general case - if generics.params.len() == 0 { + if gat_generics.params.len() == 0 { return; } - let associated_items: &ty::AssocItems<'_> = tcx.associated_items(encl_trait_def_id); + let associated_items: &ty::AssocItems<'_> = tcx.associated_items(gat_def_id); let mut clauses: Option>> = None; // For every function in this trait... // In our example, this would be the `next` method for item in associated_items.in_definition_order().filter(|item| matches!(item.kind, ty::AssocKind::Fn)) { - let id = hir::HirId::make_owner(item.def_id.expect_local()); + let item_hir_id = hir::HirId::make_owner(item.def_id.expect_local()); let param_env = tcx.param_env(item.def_id.expect_local()); // Get the signature using placeholders. In our example, this would @@ -314,11 +311,11 @@ fn check_gat_where_clauses( let function_clauses = gather_gat_bounds( tcx, param_env, - id, + item_hir_id, sig.output(), &wf_tys, - trait_item.def_id, - generics, + gat_def_id, + gat_generics, ); if let Some(function_clauses) = function_clauses { @@ -347,7 +344,7 @@ fn check_gat_where_clauses( let clauses = clauses.unwrap_or_default(); debug!(?clauses); if !clauses.is_empty() { - let param_env = tcx.param_env(trait_item.def_id); + let param_env = tcx.param_env(gat_def_id); let mut clauses: Vec<_> = clauses .into_iter() @@ -355,7 +352,7 @@ fn check_gat_where_clauses( ty::PredicateKind::RegionOutlives(ty::OutlivesPredicate(a, b)) => { !region_known_to_outlive( tcx, - trait_item.hir_id(), + gat_hir.hir_id(), param_env, &FxHashSet::default(), a, @@ -365,7 +362,7 @@ fn check_gat_where_clauses( ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate(a, b)) => { !ty_known_to_outlive( tcx, - trait_item.hir_id(), + gat_hir.hir_id(), param_env, &FxHashSet::default(), a, @@ -383,21 +380,17 @@ fn check_gat_where_clauses( if !clauses.is_empty() { let plural = if clauses.len() > 1 { "s" } else { "" }; let mut err = tcx.sess.struct_span_err( - trait_item.span, - &format!("missing required bound{} on `{}`", plural, trait_item.ident), + gat_hir.span, + &format!("missing required bound{} on `{}`", plural, gat_hir.ident), ); let suggestion = format!( "{} {}", - if !trait_item.generics.where_clause.predicates.is_empty() { - "," - } else { - " where" - }, + if !gat_hir.generics.where_clause.predicates.is_empty() { "," } else { " where" }, clauses.join(", "), ); err.span_suggestion( - trait_item.generics.where_clause.tail_span_for_suggestion(), + gat_hir.generics.where_clause.tail_span_for_suggestion(), &format!("add the required where clause{}", plural), suggestion, Applicability::MachineApplicable, From 453d2dbbd40b57c9d86c5178b94e9031cca40cbe Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 11 Feb 2022 00:08:17 -0800 Subject: [PATCH 3/8] check all GATs at once --- compiler/rustc_typeck/src/check/wfcheck.rs | 189 ++++++++++----------- 1 file changed, 90 insertions(+), 99 deletions(-) diff --git a/compiler/rustc_typeck/src/check/wfcheck.rs b/compiler/rustc_typeck/src/check/wfcheck.rs index 1864101467b7..d05c96da3317 100644 --- a/compiler/rustc_typeck/src/check/wfcheck.rs +++ b/compiler/rustc_typeck/src/check/wfcheck.rs @@ -3,7 +3,7 @@ use crate::check::{FnCtxt, Inherited}; use crate::constrained_generic_params::{identify_constrained_generic_params, Parameter}; use rustc_ast as ast; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId}; @@ -258,145 +258,131 @@ pub fn check_trait_item(tcx: TyCtxt<'_>, def_id: LocalDefId) { .emit(); } } - - check_gat_where_clauses(tcx, trait_item, encl_trait_def_id); } /// Require that the user writes where clauses on GATs for the implicit /// outlives bounds involving trait parameters in trait functions and /// lifetimes passed as GAT substs. See `self-outlives-lint` test. -/// -/// This trait will be our running example. We are currently WF checking the `Item` item... -/// -/// ```rust -/// trait LendingIterator { -/// type Item<'me>; // <-- WF checking this trait item -/// -/// fn next<'a>(&'a mut self) -> Option>; -/// } /// ``` -fn check_gat_where_clauses(tcx: TyCtxt<'_>, gat_hir: &hir::TraitItem<'_>, gat_def_id: DefId) { - let gat_item = tcx.associated_item(gat_def_id); - let gat_def_id = gat_hir.def_id; - // If the current trait item isn't a type, it isn't a GAT - if !matches!(gat_item.kind, ty::AssocKind::Type) { - return; - } - let gat_generics: &ty::Generics = tcx.generics_of(gat_def_id); - // If the current associated type doesn't have any (own) params, it's not a GAT - // FIXME(jackh726): we can also warn in the more general case - if gat_generics.params.len() == 0 { - return; - } - let associated_items: &ty::AssocItems<'_> = tcx.associated_items(gat_def_id); - let mut clauses: Option>> = None; - // For every function in this trait... - // In our example, this would be the `next` method - for item in - associated_items.in_definition_order().filter(|item| matches!(item.kind, ty::AssocKind::Fn)) - { - let item_hir_id = hir::HirId::make_owner(item.def_id.expect_local()); - let param_env = tcx.param_env(item.def_id.expect_local()); +fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRef]) { + let mut required_bounds_by_item = FxHashMap::default(); + + for gat_item in associated_items { + let gat_def_id = gat_item.id.def_id; + let gat_item = tcx.associated_item(gat_def_id); + // If this item is not an assoc ty, or has no substs, then it's not a GAT + if gat_item.kind != ty::AssocKind::Type { + continue; + } + let gat_generics = tcx.generics_of(gat_def_id); + if gat_generics.params.is_empty() { + continue; + } - // Get the signature using placeholders. In our example, this would - // convert the late-bound 'a into a free region. - let sig = tcx.liberate_late_bound_regions(item.def_id, tcx.fn_sig(item.def_id)); + let mut new_required_bounds: Option>> = None; + for item in associated_items { + if !matches!(&item.kind, hir::AssocItemKind::Fn { .. }) { + // FIXME: next commit will add items... + continue; + } - // The types we can assume to be well-formed. In our example, this - // would be &'a mut Self, from the first argument. - let mut wf_tys = FxHashSet::default(); - wf_tys.extend(sig.inputs()); + let item_def_id = item.id.def_id; + // Skip our own GAT, since it would blow away the required bounds + if item_def_id == gat_def_id { + continue; + } - // The clauses we that we would require from this function - let function_clauses = gather_gat_bounds( - tcx, - param_env, - item_hir_id, - sig.output(), - &wf_tys, - gat_def_id, - gat_generics, - ); + let item_hir_id = item.id.hir_id(); + let param_env = tcx.param_env(item_def_id); - if let Some(function_clauses) = function_clauses { - // Imagine we have: - // ``` - // trait Foo { - // type Bar<'me>; - // fn gimme(&self) -> Self::Bar<'_>; - // fn gimme_default(&self) -> Self::Bar<'static>; - // } - // ``` - // We only want to require clauses on `Bar` that we can prove from *all* functions (in this - // case, `'me` can be `static` from `gimme_default`) - match clauses.as_mut() { - Some(clauses) => { - clauses.drain_filter(|p| !function_clauses.contains(p)); - } - None => { - clauses = Some(function_clauses); + // Get the signature using placeholders. In our example, this would + // convert the late-bound 'a into a free region. + let sig = tcx.liberate_late_bound_regions( + item_def_id.to_def_id(), + tcx.fn_sig(item_def_id.to_def_id()), + ); + + // The types we can assume to be well-formed. In our example, this + // would be &'a mut Self, from the first argument. + let mut wf_tys = FxHashSet::default(); + wf_tys.extend(sig.inputs()); + + // The clauses we that we would require from this function + let item_required_bounds = gather_gat_bounds( + tcx, + param_env, + item_hir_id, + sig.output(), + &wf_tys, + gat_def_id, + gat_generics, + ); + + if let Some(item_required_bounds) = item_required_bounds { + // Take the intersection of the new_required_bounds and the item_required_bounds + // for this item. This is why we use an Option<_>, since we need to distinguish + // the empty set of bounds from the uninitialized set of bounds. + if let Some(new_required_bounds) = &mut new_required_bounds { + new_required_bounds.retain(|b| item_required_bounds.contains(b)); + } else { + new_required_bounds = Some(item_required_bounds); } } } + + if let Some(required_bounds) = new_required_bounds { + required_bounds_by_item.insert(gat_def_id, required_bounds); + } } - // If there are any clauses that aren't provable, emit an error - let clauses = clauses.unwrap_or_default(); - debug!(?clauses); - if !clauses.is_empty() { + for (gat_def_id, required_bounds) in required_bounds_by_item { + let gat_item_hir = tcx.hir().expect_trait_item(gat_def_id); + debug!(?required_bounds); let param_env = tcx.param_env(gat_def_id); + let gat_hir = gat_item_hir.hir_id(); - let mut clauses: Vec<_> = clauses + let mut unsatisfied_bounds: Vec<_> = required_bounds .into_iter() .filter(|clause| match clause.kind().skip_binder() { ty::PredicateKind::RegionOutlives(ty::OutlivesPredicate(a, b)) => { - !region_known_to_outlive( - tcx, - gat_hir.hir_id(), - param_env, - &FxHashSet::default(), - a, - b, - ) + !region_known_to_outlive(tcx, gat_hir, param_env, &FxHashSet::default(), a, b) } ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate(a, b)) => { - !ty_known_to_outlive( - tcx, - gat_hir.hir_id(), - param_env, - &FxHashSet::default(), - a, - b, - ) + !ty_known_to_outlive(tcx, gat_hir, param_env, &FxHashSet::default(), a, b) } _ => bug!("Unexpected PredicateKind"), }) - .map(|clause| format!("{}", clause)) + .map(|clause| clause.to_string()) .collect(); // We sort so that order is predictable - clauses.sort(); + unsatisfied_bounds.sort(); - if !clauses.is_empty() { - let plural = if clauses.len() > 1 { "s" } else { "" }; + if !unsatisfied_bounds.is_empty() { + let plural = if unsatisfied_bounds.len() > 1 { "s" } else { "" }; let mut err = tcx.sess.struct_span_err( - gat_hir.span, - &format!("missing required bound{} on `{}`", plural, gat_hir.ident), + gat_item_hir.span, + &format!("missing required bound{} on `{}`", plural, gat_item_hir.ident), ); let suggestion = format!( "{} {}", - if !gat_hir.generics.where_clause.predicates.is_empty() { "," } else { " where" }, - clauses.join(", "), + if !gat_item_hir.generics.where_clause.predicates.is_empty() { + "," + } else { + " where" + }, + unsatisfied_bounds.join(", "), ); err.span_suggestion( - gat_hir.generics.where_clause.tail_span_for_suggestion(), + gat_item_hir.generics.where_clause.tail_span_for_suggestion(), &format!("add the required where clause{}", plural), suggestion, Applicability::MachineApplicable, ); - let bound = if clauses.len() > 1 { "these bounds are" } else { "this bound is" }; + let bound = + if unsatisfied_bounds.len() > 1 { "these bounds are" } else { "this bound is" }; err.note(&format!( "{} currently required to ensure that impls have maximum flexibility", bound @@ -1025,6 +1011,11 @@ fn check_trait(tcx: TyCtxt<'_>, item: &hir::Item<'_>) { FxHashSet::default() }); + + // Only check traits, don't check trait aliases + if let hir::ItemKind::Trait(_, _, _, _, items) = item.kind { + check_gat_where_clauses(tcx, items); + } } /// Checks all associated type defaults of trait `trait_def_id`. From 852a8517122bf9a5c98b987eec910a83cfdaf63d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 11 Feb 2022 00:11:49 -0800 Subject: [PATCH 4/8] check associated types too --- compiler/rustc_typeck/src/check/wfcheck.rs | 54 +++++++++++----------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_typeck/src/check/wfcheck.rs b/compiler/rustc_typeck/src/check/wfcheck.rs index d05c96da3317..6788f5a371b0 100644 --- a/compiler/rustc_typeck/src/check/wfcheck.rs +++ b/compiler/rustc_typeck/src/check/wfcheck.rs @@ -281,11 +281,6 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRe let mut new_required_bounds: Option>> = None; for item in associated_items { - if !matches!(&item.kind, hir::AssocItemKind::Fn { .. }) { - // FIXME: next commit will add items... - continue; - } - let item_def_id = item.id.def_id; // Skip our own GAT, since it would blow away the required bounds if item_def_id == gat_def_id { @@ -295,28 +290,33 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRe let item_hir_id = item.id.hir_id(); let param_env = tcx.param_env(item_def_id); - // Get the signature using placeholders. In our example, this would - // convert the late-bound 'a into a free region. - let sig = tcx.liberate_late_bound_regions( - item_def_id.to_def_id(), - tcx.fn_sig(item_def_id.to_def_id()), - ); - - // The types we can assume to be well-formed. In our example, this - // would be &'a mut Self, from the first argument. - let mut wf_tys = FxHashSet::default(); - wf_tys.extend(sig.inputs()); - - // The clauses we that we would require from this function - let item_required_bounds = gather_gat_bounds( - tcx, - param_env, - item_hir_id, - sig.output(), - &wf_tys, - gat_def_id, - gat_generics, - ); + let item_required_bounds = match item.kind { + hir::AssocItemKind::Fn { .. } => { + let sig: ty::FnSig<'_> = tcx.liberate_late_bound_regions( + item_def_id.to_def_id(), + tcx.fn_sig(item_def_id), + ); + gather_gat_bounds( + tcx, + param_env, + item_hir_id, + sig.output(), + &sig.inputs().iter().copied().collect(), + gat_def_id, + gat_generics, + ) + } + hir::AssocItemKind::Type => gather_gat_bounds( + tcx, + param_env, + item_hir_id, + tcx.explicit_item_bounds(item_def_id).iter().copied().collect::>(), + &FxHashSet::default(), + gat_def_id, + gat_generics, + ), + hir::AssocItemKind::Const => None, + }; if let Some(item_required_bounds) = item_required_bounds { // Take the intersection of the new_required_bounds and the item_required_bounds From 477459795d074b8febf9973686f2b134af3ed818 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 11 Feb 2022 00:17:22 -0800 Subject: [PATCH 5/8] make the gat wfcheck algorithm a loop --- compiler/rustc_typeck/src/check/wfcheck.rs | 160 +++++++++++++-------- 1 file changed, 103 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_typeck/src/check/wfcheck.rs b/compiler/rustc_typeck/src/check/wfcheck.rs index 6788f5a371b0..d6f983d54656 100644 --- a/compiler/rustc_typeck/src/check/wfcheck.rs +++ b/compiler/rustc_typeck/src/check/wfcheck.rs @@ -266,72 +266,96 @@ pub fn check_trait_item(tcx: TyCtxt<'_>, def_id: LocalDefId) { /// ``` fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRef]) { let mut required_bounds_by_item = FxHashMap::default(); - - for gat_item in associated_items { - let gat_def_id = gat_item.id.def_id; - let gat_item = tcx.associated_item(gat_def_id); - // If this item is not an assoc ty, or has no substs, then it's not a GAT - if gat_item.kind != ty::AssocKind::Type { - continue; - } - let gat_generics = tcx.generics_of(gat_def_id); - if gat_generics.params.is_empty() { - continue; - } - - let mut new_required_bounds: Option>> = None; - for item in associated_items { - let item_def_id = item.id.def_id; - // Skip our own GAT, since it would blow away the required bounds - if item_def_id == gat_def_id { + loop { + let mut should_continue = false; + for gat_item in associated_items { + let gat_def_id = gat_item.id.def_id; + let gat_item = tcx.associated_item(gat_def_id); + // If this item is not an assoc ty, or has no substs, then it's not a GAT + if gat_item.kind != ty::AssocKind::Type { + continue; + } + let gat_generics = tcx.generics_of(gat_def_id); + if gat_generics.params.is_empty() { continue; } - let item_hir_id = item.id.hir_id(); - let param_env = tcx.param_env(item_def_id); + let mut new_required_bounds: Option>> = None; + for item in associated_items { + let item_def_id = item.id.def_id; + // Skip our own GAT, since it would blow away the required bounds + if item_def_id == gat_def_id { + continue; + } - let item_required_bounds = match item.kind { - hir::AssocItemKind::Fn { .. } => { - let sig: ty::FnSig<'_> = tcx.liberate_late_bound_regions( - item_def_id.to_def_id(), - tcx.fn_sig(item_def_id), - ); - gather_gat_bounds( - tcx, - param_env, - item_hir_id, - sig.output(), - &sig.inputs().iter().copied().collect(), - gat_def_id, - gat_generics, - ) + let item_hir_id = item.id.hir_id(); + let param_env = tcx.param_env(item_def_id); + + let item_required_bounds = match item.kind { + hir::AssocItemKind::Fn { .. } => { + let sig: ty::FnSig<'_> = tcx.liberate_late_bound_regions( + item_def_id.to_def_id(), + tcx.fn_sig(item_def_id), + ); + gather_gat_bounds( + tcx, + param_env, + item_hir_id, + sig.output(), + &sig.inputs().iter().copied().collect(), + gat_def_id, + gat_generics, + ) + } + hir::AssocItemKind::Type => { + // If our associated item is a GAT with missing bounds, add them to + // the param-env here. This allows this GAT to propagate missing bounds + // to other GATs. + let param_env = augment_param_env( + tcx, + param_env, + required_bounds_by_item.get(&item_def_id), + ); + gather_gat_bounds( + tcx, + param_env, + item_hir_id, + tcx.explicit_item_bounds(item_def_id) + .iter() + .copied() + .collect::>(), + &FxHashSet::default(), + gat_def_id, + gat_generics, + ) + } + hir::AssocItemKind::Const => None, + }; + + if let Some(item_required_bounds) = item_required_bounds { + // Take the intersection of the new_required_bounds and the item_required_bounds + // for this item. This is why we use an Option<_>, since we need to distinguish + // the empty set of bounds from the uninitialized set of bounds. + if let Some(new_required_bounds) = &mut new_required_bounds { + new_required_bounds.retain(|b| item_required_bounds.contains(b)); + } else { + new_required_bounds = Some(item_required_bounds); + } } - hir::AssocItemKind::Type => gather_gat_bounds( - tcx, - param_env, - item_hir_id, - tcx.explicit_item_bounds(item_def_id).iter().copied().collect::>(), - &FxHashSet::default(), - gat_def_id, - gat_generics, - ), - hir::AssocItemKind::Const => None, - }; + } - if let Some(item_required_bounds) = item_required_bounds { - // Take the intersection of the new_required_bounds and the item_required_bounds - // for this item. This is why we use an Option<_>, since we need to distinguish - // the empty set of bounds from the uninitialized set of bounds. - if let Some(new_required_bounds) = &mut new_required_bounds { - new_required_bounds.retain(|b| item_required_bounds.contains(b)); - } else { - new_required_bounds = Some(item_required_bounds); + if let Some(new_required_bounds) = new_required_bounds { + let required_bounds = required_bounds_by_item.entry(gat_def_id).or_default(); + if new_required_bounds != *required_bounds { + *required_bounds = new_required_bounds; + // Iterate until our required_bounds no longer change + // Since they changed here, we should continue the loop + should_continue = true; } } } - - if let Some(required_bounds) = new_required_bounds { - required_bounds_by_item.insert(gat_def_id, required_bounds); + if !should_continue { + break; } } @@ -398,6 +422,28 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRe } } +/// Add a new set of predicates to the caller_bounds of an existing param_env, +/// and normalize the param_env afterwards +fn augment_param_env<'tcx>( + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + new_predicates: Option<&FxHashSet>>, +) -> ty::ParamEnv<'tcx> { + let Some(new_predicates) = new_predicates else { + return param_env; + }; + + if new_predicates.is_empty() { + return param_env; + } + + let bounds = + tcx.mk_predicates(param_env.caller_bounds().iter().chain(new_predicates.iter().cloned())); + // FIXME(compiler-errors): Perhaps there is a case where we need to normalize this + // i.e. traits::normalize_param_env_or_error + ty::ParamEnv::new(bounds, param_env.reveal(), param_env.constness()) +} + fn gather_gat_bounds<'tcx, T: TypeFoldable<'tcx>>( tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, From d8b49f028213e8cfccb1738a628b694f3cfe4076 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 8 Feb 2022 22:28:00 -0800 Subject: [PATCH 6/8] add test for issue-93278, bless --- .../self-outlives-lint.rs | 11 ++-- .../self-outlives-lint.stderr | 50 +++++++++++++------ 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/test/ui/generic-associated-types/self-outlives-lint.rs b/src/test/ui/generic-associated-types/self-outlives-lint.rs index fcc53b4ede0c..b91e72c60197 100644 --- a/src/test/ui/generic-associated-types/self-outlives-lint.rs +++ b/src/test/ui/generic-associated-types/self-outlives-lint.rs @@ -1,7 +1,5 @@ #![feature(generic_associated_types)] -// check-fail - use std::fmt::Debug; // We have a `&'a self`, so we need a `Self: 'a` @@ -117,7 +115,6 @@ trait TraitLifetime<'a> { } // Like above, but we have a where clause that can prove what we want -// FIXME: we require two bounds (`where Self: 'a, Self: 'b`) when we should only require one trait TraitLifetimeWhere<'a> where Self: 'a { type Bar<'b>; //~^ missing required @@ -140,11 +137,19 @@ trait NotInReturn { // We obviously error for `Iterator`, but we should also error for `Item` trait IterableTwo { type Item<'a>; + //~^ missing required type Iterator<'a>: Iterator>; //~^ missing required fn iter<'a>(&'a self) -> Self::Iterator<'a>; } +trait IterableTwoWhere { + type Item<'a>; + //~^ missing required + type Iterator<'a>: Iterator> where Self: 'a; + fn iter<'a>(&'a self) -> Self::Iterator<'a>; +} + // We also should report region outlives clauses. Here, we know that `'y: 'x`, // because of `&'x &'y`, so we require that `'b: 'a`. trait RegionOutlives { diff --git a/src/test/ui/generic-associated-types/self-outlives-lint.stderr b/src/test/ui/generic-associated-types/self-outlives-lint.stderr index 3b9146ad875a..ccdfb0c1d794 100644 --- a/src/test/ui/generic-associated-types/self-outlives-lint.stderr +++ b/src/test/ui/generic-associated-types/self-outlives-lint.stderr @@ -1,5 +1,5 @@ error: missing required bound on `Item` - --> $DIR/self-outlives-lint.rs:9:5 + --> $DIR/self-outlives-lint.rs:7:5 | LL | type Item<'x>; | ^^^^^^^^^^^^^- @@ -10,7 +10,7 @@ LL | type Item<'x>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Out` - --> $DIR/self-outlives-lint.rs:25:5 + --> $DIR/self-outlives-lint.rs:23:5 | LL | type Out<'x>; | ^^^^^^^^^^^^- @@ -21,7 +21,7 @@ LL | type Out<'x>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Out` - --> $DIR/self-outlives-lint.rs:39:5 + --> $DIR/self-outlives-lint.rs:37:5 | LL | type Out<'x>; | ^^^^^^^^^^^^- @@ -32,7 +32,7 @@ LL | type Out<'x>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bounds on `Out` - --> $DIR/self-outlives-lint.rs:46:5 + --> $DIR/self-outlives-lint.rs:44:5 | LL | type Out<'x, 'y>; | ^^^^^^^^^^^^^^^^- @@ -43,7 +43,7 @@ LL | type Out<'x, 'y>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Out` - --> $DIR/self-outlives-lint.rs:61:5 + --> $DIR/self-outlives-lint.rs:59:5 | LL | type Out<'x, D>; | ^^^^^^^^^^^^^^^- @@ -54,7 +54,7 @@ LL | type Out<'x, D>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Out` - --> $DIR/self-outlives-lint.rs:77:5 + --> $DIR/self-outlives-lint.rs:75:5 | LL | type Out<'x, D>; | ^^^^^^^^^^^^^^^- @@ -65,7 +65,7 @@ LL | type Out<'x, D>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Out` - --> $DIR/self-outlives-lint.rs:92:5 + --> $DIR/self-outlives-lint.rs:90:5 | LL | type Out<'x, D>; | ^^^^^^^^^^^^^^^- @@ -76,7 +76,7 @@ LL | type Out<'x, D>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bounds on `Bar` - --> $DIR/self-outlives-lint.rs:114:5 + --> $DIR/self-outlives-lint.rs:112:5 | LL | type Bar<'b>; | ^^^^^^^^^^^^- @@ -87,7 +87,7 @@ LL | type Bar<'b>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Bar` - --> $DIR/self-outlives-lint.rs:122:5 + --> $DIR/self-outlives-lint.rs:119:5 | LL | type Bar<'b>; | ^^^^^^^^^^^^- @@ -98,7 +98,7 @@ LL | type Bar<'b>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Bar` - --> $DIR/self-outlives-lint.rs:129:5 + --> $DIR/self-outlives-lint.rs:126:5 | LL | type Bar<'b>; | ^^^^^^^^^^^^- @@ -108,8 +108,19 @@ LL | type Bar<'b>; = note: this bound is currently required to ensure that impls have maximum flexibility = note: we are soliciting feedback, see issue #87479 for more information +error: missing required bound on `Item` + --> $DIR/self-outlives-lint.rs:139:5 + | +LL | type Item<'a>; + | ^^^^^^^^^^^^^- + | | + | help: add the required where clause: `where Self: 'a` + | + = note: this bound is currently required to ensure that impls have maximum flexibility + = note: we are soliciting feedback, see issue #87479 for more information + error: missing required bound on `Iterator` - --> $DIR/self-outlives-lint.rs:143:5 + --> $DIR/self-outlives-lint.rs:141:5 | LL | type Iterator<'a>: Iterator>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -119,8 +130,19 @@ LL | type Iterator<'a>: Iterator>; = note: this bound is currently required to ensure that impls have maximum flexibility = note: we are soliciting feedback, see issue #87479 for more information +error: missing required bound on `Item` + --> $DIR/self-outlives-lint.rs:147:5 + | +LL | type Item<'a>; + | ^^^^^^^^^^^^^- + | | + | help: add the required where clause: `where Self: 'a` + | + = note: this bound is currently required to ensure that impls have maximum flexibility + = note: we are soliciting feedback, see issue #87479 for more information + error: missing required bound on `Bar` - --> $DIR/self-outlives-lint.rs:151:5 + --> $DIR/self-outlives-lint.rs:156:5 | LL | type Bar<'a, 'b>; | ^^^^^^^^^^^^^^^^- @@ -131,7 +153,7 @@ LL | type Bar<'a, 'b>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Fut` - --> $DIR/self-outlives-lint.rs:167:5 + --> $DIR/self-outlives-lint.rs:172:5 | LL | type Fut<'out>; | ^^^^^^^^^^^^^^- @@ -141,5 +163,5 @@ LL | type Fut<'out>; = note: this bound is currently required to ensure that impls have maximum flexibility = note: we are soliciting feedback, see issue #87479 for more information -error: aborting due to 13 previous errors +error: aborting due to 15 previous errors From ce16189d46c607483fd892cbf9353e3c9f2d8047 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 12 Feb 2022 14:55:53 -0800 Subject: [PATCH 7/8] add some more comments to GAT where clause check --- compiler/rustc_typeck/src/check/wfcheck.rs | 81 +++++++++++++++++----- 1 file changed, 62 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_typeck/src/check/wfcheck.rs b/compiler/rustc_typeck/src/check/wfcheck.rs index d6f983d54656..d18682f5b62b 100644 --- a/compiler/rustc_typeck/src/check/wfcheck.rs +++ b/compiler/rustc_typeck/src/check/wfcheck.rs @@ -263,9 +263,24 @@ pub fn check_trait_item(tcx: TyCtxt<'_>, def_id: LocalDefId) { /// Require that the user writes where clauses on GATs for the implicit /// outlives bounds involving trait parameters in trait functions and /// lifetimes passed as GAT substs. See `self-outlives-lint` test. +/// +/// We use the following trait as an example throughout this function: +/// ```rust,ignore (this code fails due to this lint) +/// trait IntoIter { +/// type Iter<'a>: Iterator>; +/// type Item<'a>; +/// fn into_iter<'a>(&'a self) -> Self::Iter<'a>; +/// } /// ``` fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRef]) { + // Associates every GAT's def_id to a list of possibly missing bounds detected by this lint. let mut required_bounds_by_item = FxHashMap::default(); + + // Loop over all GATs together, because if this lint suggests adding a where-clause bound + // to one GAT, it might then require us to an additional bound on another GAT. + // In our `IntoIter` example, we discover a missing `Self: 'a` bound on `Iter<'a>`, which + // then in a second loop adds a `Self: 'a` bound to `Item` due to the relationship between + // those GATs. loop { let mut should_continue = false; for gat_item in associated_items { @@ -276,14 +291,18 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRe continue; } let gat_generics = tcx.generics_of(gat_def_id); + // FIXME(jackh726): we can also warn in the more general case if gat_generics.params.is_empty() { continue; } + // Gather the bounds with which all other items inside of this trait constrain the GAT. + // This is calculated by taking the intersection of the bounds that each item + // constrains the GAT with individually. let mut new_required_bounds: Option>> = None; for item in associated_items { let item_def_id = item.id.def_id; - // Skip our own GAT, since it would blow away the required bounds + // Skip our own GAT, since it does not constrain itself at all. if item_def_id == gat_def_id { continue; } @@ -292,7 +311,11 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRe let param_env = tcx.param_env(item_def_id); let item_required_bounds = match item.kind { + // In our example, this corresponds to `into_iter` method hir::AssocItemKind::Fn { .. } => { + // For methods, we check the function signature's return type for any GATs + // to constrain. In the `into_iter` case, we see that the return type + // `Self::Iter<'a>` is a GAT we want to gather any potential missing bounds from. let sig: ty::FnSig<'_> = tcx.liberate_late_bound_regions( item_def_id.to_def_id(), tcx.fn_sig(item_def_id), @@ -302,11 +325,14 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRe param_env, item_hir_id, sig.output(), + // We also assume that all of the function signature's parameter types + // are well formed. &sig.inputs().iter().copied().collect(), gat_def_id, gat_generics, ) } + // In our example, this corresponds to the `Iter` and `Item` associated types hir::AssocItemKind::Type => { // If our associated item is a GAT with missing bounds, add them to // the param-env here. This allows this GAT to propagate missing bounds @@ -316,6 +342,7 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRe param_env, required_bounds_by_item.get(&item_def_id), ); + // FIXME(compiler-errors): Do we want to add a assoc ty default to the wf_tys? gather_gat_bounds( tcx, param_env, @@ -333,9 +360,11 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRe }; if let Some(item_required_bounds) = item_required_bounds { - // Take the intersection of the new_required_bounds and the item_required_bounds - // for this item. This is why we use an Option<_>, since we need to distinguish - // the empty set of bounds from the uninitialized set of bounds. + // Take the intersection of the required bounds for this GAT, and + // the item_required_bounds which are the ones implied by just + // this item alone. + // This is why we use an Option<_>, since we need to distinguish + // the empty set of bounds from the _uninitialized_ set of bounds. if let Some(new_required_bounds) = &mut new_required_bounds { new_required_bounds.retain(|b| item_required_bounds.contains(b)); } else { @@ -346,14 +375,17 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRe if let Some(new_required_bounds) = new_required_bounds { let required_bounds = required_bounds_by_item.entry(gat_def_id).or_default(); - if new_required_bounds != *required_bounds { - *required_bounds = new_required_bounds; + if new_required_bounds.into_iter().any(|p| required_bounds.insert(p)) { // Iterate until our required_bounds no longer change // Since they changed here, we should continue the loop should_continue = true; } } } + // We know that this loop will eventually halt, since we only set `should_continue` if the + // `required_bounds` for this item grows. Since we are not creating any new region or type + // variables, the set of all region and type bounds that we could ever insert are limited + // by the number of unique types and regions we observe in a given item. if !should_continue { break; } @@ -422,8 +454,7 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRe } } -/// Add a new set of predicates to the caller_bounds of an existing param_env, -/// and normalize the param_env afterwards +/// Add a new set of predicates to the caller_bounds of an existing param_env. fn augment_param_env<'tcx>( tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, @@ -444,6 +475,16 @@ fn augment_param_env<'tcx>( ty::ParamEnv::new(bounds, param_env.reveal(), param_env.constness()) } +/// We use the following trait as an example throughout this function. +/// Specifically, let's assume that `to_check` here is the return type +/// of `into_iter`, and the GAT we are checking this for is `Iter`. +/// ```rust,ignore (this code fails due to this lint) +/// trait IntoIter { +/// type Iter<'a>: Iterator>; +/// type Item<'a>; +/// fn into_iter<'a>(&'a self) -> Self::Iter<'a>; +/// } +/// ``` fn gather_gat_bounds<'tcx, T: TypeFoldable<'tcx>>( tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, @@ -453,13 +494,13 @@ fn gather_gat_bounds<'tcx, T: TypeFoldable<'tcx>>( gat_def_id: LocalDefId, gat_generics: &'tcx ty::Generics, ) -> Option>> { - // The bounds we that we would require from this function + // The bounds we that we would require from `to_check` let mut bounds = FxHashSet::default(); let (regions, types) = GATSubstCollector::visit(tcx, gat_def_id.to_def_id(), to_check); // If both regions and types are empty, then this GAT isn't in the - // return type, and we shouldn't try to do clause analysis + // set of types we are checking, and we shouldn't try to do clause analysis // (particularly, doing so would end up with an empty set of clauses, // since the current method would require none, and we take the // intersection of requirements of all methods) @@ -474,18 +515,18 @@ fn gather_gat_bounds<'tcx, T: TypeFoldable<'tcx>>( if let ty::ReStatic = **region_a { continue; } - // For each region argument (e.g., 'a in our example), check for a - // relationship to the type arguments (e.g., Self). If there is an + // For each region argument (e.g., `'a` in our example), check for a + // relationship to the type arguments (e.g., `Self`). If there is an // outlives relationship (`Self: 'a`), then we want to ensure that is // reflected in a where clause on the GAT itself. for (ty, ty_idx) in &types { - // In our example, requires that Self: 'a + // In our example, requires that `Self: 'a` if ty_known_to_outlive(tcx, item_hir, param_env, &wf_tys, *ty, *region_a) { debug!(?ty_idx, ?region_a_idx); debug!("required clause: {} must outlive {}", ty, region_a); // Translate into the generic parameters of the GAT. In - // our example, the type was Self, which will also be - // Self in the GAT. + // our example, the type was `Self`, which will also be + // `Self` in the GAT. let ty_param = gat_generics.param_at(*ty_idx, tcx); let ty_param = tcx .mk_ty(ty::Param(ty::ParamTy { index: ty_param.index, name: ty_param.name })); @@ -507,11 +548,13 @@ fn gather_gat_bounds<'tcx, T: TypeFoldable<'tcx>>( } } - // For each region argument (e.g., 'a in our example), also check for a - // relationship to the other region arguments. If there is an - // outlives relationship, then we want to ensure that is - // reflected in a where clause on the GAT itself. + // For each region argument (e.g., `'a` in our example), also check for a + // relationship to the other region arguments. If there is an outlives + // relationship, then we want to ensure that is reflected in the where clause + // on the GAT itself. for (region_b, region_b_idx) in ®ions { + // Again, skip `'static` because it outlives everything. Also, we trivially + // know that a region outlives itself. if ty::ReStatic == **region_b || region_a == region_b { continue; } From f045b214ea52eebdaf0b17a1ca6d4be807abb032 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 14 Feb 2022 09:08:06 -0800 Subject: [PATCH 8/8] Add removed comments back in self-outlives-lint --- compiler/rustc_typeck/src/check/wfcheck.rs | 1 - .../self-outlives-lint.rs | 3 ++ .../self-outlives-lint.stderr | 30 +++++++++---------- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_typeck/src/check/wfcheck.rs b/compiler/rustc_typeck/src/check/wfcheck.rs index d18682f5b62b..55757251e26f 100644 --- a/compiler/rustc_typeck/src/check/wfcheck.rs +++ b/compiler/rustc_typeck/src/check/wfcheck.rs @@ -342,7 +342,6 @@ fn check_gat_where_clauses(tcx: TyCtxt<'_>, associated_items: &[hir::TraitItemRe param_env, required_bounds_by_item.get(&item_def_id), ); - // FIXME(compiler-errors): Do we want to add a assoc ty default to the wf_tys? gather_gat_bounds( tcx, param_env, diff --git a/src/test/ui/generic-associated-types/self-outlives-lint.rs b/src/test/ui/generic-associated-types/self-outlives-lint.rs index b91e72c60197..300907adbfc1 100644 --- a/src/test/ui/generic-associated-types/self-outlives-lint.rs +++ b/src/test/ui/generic-associated-types/self-outlives-lint.rs @@ -1,5 +1,7 @@ #![feature(generic_associated_types)] +// check-fail + use std::fmt::Debug; // We have a `&'a self`, so we need a `Self: 'a` @@ -115,6 +117,7 @@ trait TraitLifetime<'a> { } // Like above, but we have a where clause that can prove what we want +// FIXME: we require two bounds (`where Self: 'a, Self: 'b`) when we should only require one trait TraitLifetimeWhere<'a> where Self: 'a { type Bar<'b>; //~^ missing required diff --git a/src/test/ui/generic-associated-types/self-outlives-lint.stderr b/src/test/ui/generic-associated-types/self-outlives-lint.stderr index ccdfb0c1d794..fdb1f50a7764 100644 --- a/src/test/ui/generic-associated-types/self-outlives-lint.stderr +++ b/src/test/ui/generic-associated-types/self-outlives-lint.stderr @@ -1,5 +1,5 @@ error: missing required bound on `Item` - --> $DIR/self-outlives-lint.rs:7:5 + --> $DIR/self-outlives-lint.rs:9:5 | LL | type Item<'x>; | ^^^^^^^^^^^^^- @@ -10,7 +10,7 @@ LL | type Item<'x>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Out` - --> $DIR/self-outlives-lint.rs:23:5 + --> $DIR/self-outlives-lint.rs:25:5 | LL | type Out<'x>; | ^^^^^^^^^^^^- @@ -21,7 +21,7 @@ LL | type Out<'x>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Out` - --> $DIR/self-outlives-lint.rs:37:5 + --> $DIR/self-outlives-lint.rs:39:5 | LL | type Out<'x>; | ^^^^^^^^^^^^- @@ -32,7 +32,7 @@ LL | type Out<'x>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bounds on `Out` - --> $DIR/self-outlives-lint.rs:44:5 + --> $DIR/self-outlives-lint.rs:46:5 | LL | type Out<'x, 'y>; | ^^^^^^^^^^^^^^^^- @@ -43,7 +43,7 @@ LL | type Out<'x, 'y>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Out` - --> $DIR/self-outlives-lint.rs:59:5 + --> $DIR/self-outlives-lint.rs:61:5 | LL | type Out<'x, D>; | ^^^^^^^^^^^^^^^- @@ -54,7 +54,7 @@ LL | type Out<'x, D>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Out` - --> $DIR/self-outlives-lint.rs:75:5 + --> $DIR/self-outlives-lint.rs:77:5 | LL | type Out<'x, D>; | ^^^^^^^^^^^^^^^- @@ -65,7 +65,7 @@ LL | type Out<'x, D>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Out` - --> $DIR/self-outlives-lint.rs:90:5 + --> $DIR/self-outlives-lint.rs:92:5 | LL | type Out<'x, D>; | ^^^^^^^^^^^^^^^- @@ -76,7 +76,7 @@ LL | type Out<'x, D>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bounds on `Bar` - --> $DIR/self-outlives-lint.rs:112:5 + --> $DIR/self-outlives-lint.rs:114:5 | LL | type Bar<'b>; | ^^^^^^^^^^^^- @@ -87,7 +87,7 @@ LL | type Bar<'b>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Bar` - --> $DIR/self-outlives-lint.rs:119:5 + --> $DIR/self-outlives-lint.rs:122:5 | LL | type Bar<'b>; | ^^^^^^^^^^^^- @@ -98,7 +98,7 @@ LL | type Bar<'b>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Bar` - --> $DIR/self-outlives-lint.rs:126:5 + --> $DIR/self-outlives-lint.rs:129:5 | LL | type Bar<'b>; | ^^^^^^^^^^^^- @@ -109,7 +109,7 @@ LL | type Bar<'b>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Item` - --> $DIR/self-outlives-lint.rs:139:5 + --> $DIR/self-outlives-lint.rs:142:5 | LL | type Item<'a>; | ^^^^^^^^^^^^^- @@ -120,7 +120,7 @@ LL | type Item<'a>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Iterator` - --> $DIR/self-outlives-lint.rs:141:5 + --> $DIR/self-outlives-lint.rs:144:5 | LL | type Iterator<'a>: Iterator>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -131,7 +131,7 @@ LL | type Iterator<'a>: Iterator>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Item` - --> $DIR/self-outlives-lint.rs:147:5 + --> $DIR/self-outlives-lint.rs:150:5 | LL | type Item<'a>; | ^^^^^^^^^^^^^- @@ -142,7 +142,7 @@ LL | type Item<'a>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Bar` - --> $DIR/self-outlives-lint.rs:156:5 + --> $DIR/self-outlives-lint.rs:159:5 | LL | type Bar<'a, 'b>; | ^^^^^^^^^^^^^^^^- @@ -153,7 +153,7 @@ LL | type Bar<'a, 'b>; = note: we are soliciting feedback, see issue #87479 for more information error: missing required bound on `Fut` - --> $DIR/self-outlives-lint.rs:172:5 + --> $DIR/self-outlives-lint.rs:175:5 | LL | type Fut<'out>; | ^^^^^^^^^^^^^^-