From f1f1d56d93b40228296e6306fadbcc352edbafc5 Mon Sep 17 00:00:00 2001 From: jackh726 Date: Sun, 15 Aug 2021 00:53:40 -0400 Subject: [PATCH] Don't move ?Trait bounds to param bounds if they're in where clauses --- compiler/rustc_ast_lowering/src/item.rs | 46 +++++-------- compiler/rustc_ast_lowering/src/lib.rs | 25 ++----- compiler/rustc_hir/src/hir.rs | 2 - compiler/rustc_hir/src/intravisit.rs | 1 - compiler/rustc_hir_pretty/src/lib.rs | 3 - compiler/rustc_middle/src/ty/diagnostics.rs | 12 +--- .../rustc_save_analysis/src/dump_visitor.rs | 1 - compiler/rustc_typeck/src/astconv/mod.rs | 67 ++++++++++++++++--- compiler/rustc_typeck/src/collect.rs | 9 ++- .../rustc_typeck/src/collect/item_bounds.rs | 4 ++ src/test/ui/maybe-bounds-where.rs | 1 + src/test/ui/maybe-bounds-where.stderr | 14 ++-- 12 files changed, 103 insertions(+), 82 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index e0d4095d769..57a680f45b3 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -2,7 +2,6 @@ use super::{AnonymousLifetimeMode, LoweringContext, ParamMode}; use super::{ImplTraitContext, ImplTraitPosition}; use crate::Arena; -use rustc_ast::node_id::NodeMap; use rustc_ast::ptr::P; use rustc_ast::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor}; use rustc_ast::*; @@ -1351,8 +1350,11 @@ impl<'hir> LoweringContext<'_, 'hir> { generics: &Generics, itctx: ImplTraitContext<'_, 'hir>, ) -> GenericsCtor<'hir> { - // Collect `?Trait` bounds in where clause and move them to parameter definitions. - let mut add_bounds: NodeMap> = Default::default(); + // Error if `?Trait` bounds in where clauses don't refer directly to type paramters. + // Note: we used to clone these bounds directly onto the type parameter (and avoid lowering + // these into hir when we lower thee where clauses), but this makes it quite difficult to + // keep track of the Span info. Now, `is_unsized` in `AstConv` checks both param bounds and + // where clauses for `?Sized`. for pred in &generics.where_clause.predicates { if let WherePredicate::BoundPredicate(ref bound_pred) = *pred { 'next_bound: for bound in &bound_pred.bounds { @@ -1368,7 +1370,6 @@ impl<'hir> LoweringContext<'_, 'hir> { { for param in &generics.params { if def_id == self.resolver.local_def_id(param.id).to_def_id() { - add_bounds.entry(param.id).or_default().push(bound.clone()); continue 'next_bound; } } @@ -1386,7 +1387,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } GenericsCtor { - params: self.lower_generic_params_mut(&generics.params, &add_bounds, itctx).collect(), + params: self.lower_generic_params_mut(&generics.params, itctx).collect(), where_clause: self.lower_where_clause(&generics.where_clause), span: self.lower_span(generics.span), } @@ -1419,32 +1420,17 @@ impl<'hir> LoweringContext<'_, 'hir> { ref bounded_ty, ref bounds, span, - }) => { - self.with_in_scope_lifetime_defs(&bound_generic_params, |this| { - hir::WherePredicate::BoundPredicate(hir::WhereBoundPredicate { - bound_generic_params: this.lower_generic_params( - bound_generic_params, - &NodeMap::default(), - ImplTraitContext::disallowed(), - ), - bounded_ty: this.lower_ty(bounded_ty, ImplTraitContext::disallowed()), - bounds: this.arena.alloc_from_iter(bounds.iter().map( - |bound| match bound { - // We used to ignore `?Trait` bounds, as they were copied into type - // parameters already, but we need to keep them around only for - // diagnostics when we suggest removal of `?Sized` bounds. See - // `suggest_constraining_type_param`. This will need to change if - // we ever allow something *other* than `?Sized`. - GenericBound::Trait(p, TraitBoundModifier::Maybe) => { - hir::GenericBound::Unsized(this.lower_span(p.span)) - } - _ => this.lower_param_bound(bound, ImplTraitContext::disallowed()), - }, - )), - span: this.lower_span(span), - }) + }) => self.with_in_scope_lifetime_defs(&bound_generic_params, |this| { + hir::WherePredicate::BoundPredicate(hir::WhereBoundPredicate { + bound_generic_params: this + .lower_generic_params(bound_generic_params, ImplTraitContext::disallowed()), + bounded_ty: this.lower_ty(bounded_ty, ImplTraitContext::disallowed()), + bounds: this.arena.alloc_from_iter(bounds.iter().map(|bound| { + this.lower_param_bound(bound, ImplTraitContext::disallowed()) + })), + span: this.lower_span(span), }) - } + }), WherePredicate::RegionPredicate(WhereRegionPredicate { ref lifetime, ref bounds, diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 5391d4b0c93..fa14764c42a 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -1313,7 +1313,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { hir::TyKind::BareFn(this.arena.alloc(hir::BareFnTy { generic_params: this.lower_generic_params( &f.generic_params, - &NodeMap::default(), ImplTraitContext::disallowed(), ), unsafety: this.lower_unsafety(f.unsafety), @@ -1998,30 +1997,25 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_generic_params_mut<'s>( &'s mut self, params: &'s [GenericParam], - add_bounds: &'s NodeMap>, mut itctx: ImplTraitContext<'s, 'hir>, ) -> impl Iterator> + Captures<'a> + Captures<'s> { - params - .iter() - .map(move |param| self.lower_generic_param(param, add_bounds, itctx.reborrow())) + params.iter().map(move |param| self.lower_generic_param(param, itctx.reborrow())) } fn lower_generic_params( &mut self, params: &[GenericParam], - add_bounds: &NodeMap>, itctx: ImplTraitContext<'_, 'hir>, ) -> &'hir [hir::GenericParam<'hir>] { - self.arena.alloc_from_iter(self.lower_generic_params_mut(params, add_bounds, itctx)) + self.arena.alloc_from_iter(self.lower_generic_params_mut(params, itctx)) } fn lower_generic_param( &mut self, param: &GenericParam, - add_bounds: &NodeMap>, mut itctx: ImplTraitContext<'_, 'hir>, ) -> hir::GenericParam<'hir> { - let mut bounds: Vec<_> = self + let bounds: Vec<_> = self .with_anonymous_lifetime_mode(AnonymousLifetimeMode::ReportError, |this| { this.lower_param_bounds_mut(¶m.bounds, itctx.reborrow()).collect() }); @@ -2057,12 +2051,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { (param_name, kind) } GenericParamKind::Type { ref default, .. } => { - let add_bounds = add_bounds.get(¶m.id).map_or(&[][..], |x| &x); - if !add_bounds.is_empty() { - let params = self.lower_param_bounds_mut(add_bounds, itctx.reborrow()); - bounds.extend(params); - } - let kind = hir::GenericParamKind::Type { default: default.as_ref().map(|x| { self.lower_ty(x, ImplTraitContext::Disallowed(ImplTraitPosition::Other)) @@ -2123,11 +2111,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { p: &PolyTraitRef, mut itctx: ImplTraitContext<'_, 'hir>, ) -> hir::PolyTraitRef<'hir> { - let bound_generic_params = self.lower_generic_params( - &p.bound_generic_params, - &NodeMap::default(), - itctx.reborrow(), - ); + let bound_generic_params = + self.lower_generic_params(&p.bound_generic_params, itctx.reborrow()); let trait_ref = self.with_in_scope_lifetime_defs(&p.bound_generic_params, |this| { // Any impl Trait types defined within this scope can capture diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 4821b81153a..84bc37170c6 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -441,7 +441,6 @@ pub enum GenericBound<'hir> { Trait(PolyTraitRef<'hir>, TraitBoundModifier), // FIXME(davidtwco): Introduce `PolyTraitRef::LangItem` LangItemTrait(LangItem, Span, HirId, &'hir GenericArgs<'hir>), - Unsized(Span), Outlives(Lifetime), } @@ -461,7 +460,6 @@ impl GenericBound<'_> { GenericBound::Trait(t, ..) => t.span, GenericBound::LangItemTrait(_, span, ..) => *span, GenericBound::Outlives(l) => l.span, - GenericBound::Unsized(span) => *span, } } } diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index 2dd49eba442..137782a6dc7 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -871,7 +871,6 @@ pub fn walk_param_bound<'v, V: Visitor<'v>>(visitor: &mut V, bound: &'v GenericB visitor.visit_generic_args(span, args); } GenericBound::Outlives(ref lifetime) => visitor.visit_lifetime(lifetime), - GenericBound::Unsized(_) => {} } } diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs index 42e51f4bb48..36054c04847 100644 --- a/compiler/rustc_hir_pretty/src/lib.rs +++ b/compiler/rustc_hir_pretty/src/lib.rs @@ -2232,9 +2232,6 @@ impl<'a> State<'a> { GenericBound::Outlives(lt) => { self.print_lifetime(lt); } - GenericBound::Unsized(_) => { - self.s.word("?Sized"); - } } } } diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index 4cfb104bee3..092eae0fc5c 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -2,7 +2,6 @@ use crate::ty::TyKind::*; use crate::ty::{InferTy, TyCtxt, TyS}; -use rustc_data_structures::fx::FxHashSet; use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::def_id::DefId; @@ -114,10 +113,8 @@ fn suggest_removing_unsized_bound( def_id: Option, ) { // See if there's a `?Sized` bound that can be removed to suggest that. - // First look at the `where` clause because we can have `where T: ?Sized`, but that - // `?Sized` bound is *also* included in the `GenericParam` as a bound, which breaks - // the spans. Hence the somewhat involved logic that follows. - let mut where_unsized_bounds = FxHashSet::default(); + // First look at the `where` clause because we can have `where T: ?Sized`, + // then look at params. for (where_pos, predicate) in generics.where_clause.predicates.iter().enumerate() { match predicate { WherePredicate::BoundPredicate(WhereBoundPredicate { @@ -140,7 +137,6 @@ fn suggest_removing_unsized_bound( }) if segment.ident.as_str() == param_name => { for (pos, bound) in bounds.iter().enumerate() { match bound { - hir::GenericBound::Unsized(_) => {} hir::GenericBound::Trait(poly, hir::TraitBoundModifier::Maybe) if poly.trait_ref.trait_def_id() == def_id => {} _ => continue, @@ -173,7 +169,6 @@ fn suggest_removing_unsized_bound( // ^^^^^^^^^ (_, pos, _, _) => bounds[pos - 1].span().shrink_to_hi().to(bound.span()), }; - where_unsized_bounds.insert(bound.span()); err.span_suggestion_verbose( sp, "consider removing the `?Sized` bound to make the \ @@ -189,8 +184,7 @@ fn suggest_removing_unsized_bound( for (pos, bound) in param.bounds.iter().enumerate() { match bound { hir::GenericBound::Trait(poly, hir::TraitBoundModifier::Maybe) - if poly.trait_ref.trait_def_id() == def_id - && !where_unsized_bounds.contains(&bound.span()) => + if poly.trait_ref.trait_def_id() == def_id => { let sp = match (param.bounds.len(), pos) { // T: ?Sized, diff --git a/compiler/rustc_save_analysis/src/dump_visitor.rs b/compiler/rustc_save_analysis/src/dump_visitor.rs index 08ed9ec73c8..3e99f4e29ef 100644 --- a/compiler/rustc_save_analysis/src/dump_visitor.rs +++ b/compiler/rustc_save_analysis/src/dump_visitor.rs @@ -693,7 +693,6 @@ impl<'tcx> DumpVisitor<'tcx> { (Some(self.tcx.require_lang_item(lang_item, Some(span))), span) } hir::GenericBound::Outlives(..) => continue, - hir::GenericBound::Unsized(_) => continue, }; if let Some(id) = def_id { diff --git a/compiler/rustc_typeck/src/astconv/mod.rs b/compiler/rustc_typeck/src/astconv/mod.rs index a795a52a25c..d72d3a88e34 100644 --- a/compiler/rustc_typeck/src/astconv/mod.rs +++ b/compiler/rustc_typeck/src/astconv/mod.rs @@ -854,7 +854,13 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { } // Returns `true` if a bounds list includes `?Sized`. - pub fn is_unsized(&self, ast_bounds: &[hir::GenericBound<'_>], span: Span) -> bool { + fn is_unsized( + &self, + ast_bounds: &[hir::GenericBound<'_>], + self_ty: Option, + where_clause: Option<&[hir::WherePredicate<'_>]>, + span: Span, + ) -> bool { let tcx = self.tcx(); // Try to find an unbound in bounds. @@ -868,11 +874,38 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { } } } + if let (Some(self_ty), Some(where_clause)) = (self_ty, where_clause) { + let self_ty_def_id = tcx.hir().local_def_id(self_ty).to_def_id(); + for clause in where_clause { + match clause { + hir::WherePredicate::BoundPredicate(pred) => { + match pred.bounded_ty.kind { + hir::TyKind::Path(hir::QPath::Resolved(_, path)) => match path.res { + Res::Def(DefKind::TyParam, def_id) if def_id == self_ty_def_id => {} + _ => continue, + }, + _ => continue, + } + for ab in pred.bounds { + if let hir::GenericBound::Trait(ptr, hir::TraitBoundModifier::Maybe) = + ab + { + if unbound.is_none() { + unbound = Some(&ptr.trait_ref); + } else { + tcx.sess.emit_err(MultipleRelaxedDefaultBounds { span }); + } + } + } + } + _ => {} + } + } + } let kind_id = tcx.lang_items().require(LangItem::Sized); match unbound { Some(tpb) => { - // FIXME(#8559) currently requires the unbound to be built-in. if let Ok(kind_id) = kind_id { if tpb.path.res != Res::Def(DefKind::Trait, kind_id) { tcx.sess.span_warn( @@ -940,8 +973,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { false, ); } - hir::GenericBound::Trait(_, hir::TraitBoundModifier::Maybe) - | hir::GenericBound::Unsized(_) => {} + hir::GenericBound::Trait(_, hir::TraitBoundModifier::Maybe) => {} hir::GenericBound::LangItemTrait(lang_item, span, hir_id, args) => self .instantiate_lang_item_trait_ref( lang_item, span, hir_id, args, param_ty, bounds, @@ -970,22 +1002,33 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { /// example above, but is not true in supertrait listings like `trait Foo: Bar + Baz`. /// /// `span` should be the declaration size of the parameter. - pub fn compute_bounds( + pub(crate) fn compute_bounds( &self, param_ty: Ty<'tcx>, ast_bounds: &[hir::GenericBound<'_>], + self_ty: Option, + where_clause: Option<&[hir::WherePredicate<'_>]>, sized_by_default: SizedByDefault, span: Span, ) -> Bounds<'tcx> { - self.compute_bounds_inner(param_ty, &ast_bounds, sized_by_default, span) + self.compute_bounds_inner( + param_ty, + &ast_bounds, + self_ty, + where_clause, + sized_by_default, + span, + ) } /// Convert the bounds in `ast_bounds` that refer to traits which define an associated type /// named `assoc_name` into ty::Bounds. Ignore the rest. - pub fn compute_bounds_that_match_assoc_type( + pub(crate) fn compute_bounds_that_match_assoc_type( &self, param_ty: Ty<'tcx>, ast_bounds: &[hir::GenericBound<'_>], + self_ty: Option, + where_clause: Option<&[hir::WherePredicate<'_>]>, sized_by_default: SizedByDefault, span: Span, assoc_name: Ident, @@ -1002,13 +1045,15 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { } } - self.compute_bounds_inner(param_ty, &result, sized_by_default, span) + self.compute_bounds_inner(param_ty, &result, self_ty, where_clause, sized_by_default, span) } fn compute_bounds_inner( &self, param_ty: Ty<'tcx>, ast_bounds: &[hir::GenericBound<'_>], + self_ty: Option, + where_clause: Option<&[hir::WherePredicate<'_>]>, sized_by_default: SizedByDefault, span: Span, ) -> Bounds<'tcx> { @@ -1017,7 +1062,11 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { self.add_bounds(param_ty, ast_bounds, &mut bounds, ty::List::empty()); bounds.implicitly_sized = if let SizedByDefault::Yes = sized_by_default { - if !self.is_unsized(ast_bounds, span) { Some(span) } else { None } + if !self.is_unsized(ast_bounds, self_ty, where_clause, span) { + Some(span) + } else { + None + } } else { None }; diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index 64120510e56..74ea7247ab2 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -1156,6 +1156,8 @@ fn super_predicates_that_define_assoc_type( &icx, self_param_ty, &bounds, + None, + None, SizedByDefault::No, item.span, assoc_name, @@ -1165,6 +1167,8 @@ fn super_predicates_that_define_assoc_type( &icx, self_param_ty, &bounds, + None, + None, SizedByDefault::No, item.span, ) @@ -2181,6 +2185,8 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericP &icx, param_ty, ¶m.bounds, + Some(param.hir_id), + Some(ast_generics.where_clause.predicates), sized, param.span, ); @@ -2267,8 +2273,6 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericP predicates.extend(bounds.predicates(tcx, ty)); } - hir::GenericBound::Unsized(_) => {} - hir::GenericBound::Outlives(lifetime) => { let region = >::ast_region_to_region(&icx, lifetime, None); @@ -2529,7 +2533,6 @@ fn predicates_from_bound<'tcx>( ); bounds.predicates(astconv.tcx(), param_ty) } - hir::GenericBound::Unsized(_) => vec![], hir::GenericBound::Outlives(ref lifetime) => { let region = astconv.ast_region_to_region(lifetime, None); let pred = ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate(param_ty, region)) diff --git a/compiler/rustc_typeck/src/collect/item_bounds.rs b/compiler/rustc_typeck/src/collect/item_bounds.rs index 1d08c4450af..cc524b34344 100644 --- a/compiler/rustc_typeck/src/collect/item_bounds.rs +++ b/compiler/rustc_typeck/src/collect/item_bounds.rs @@ -29,6 +29,8 @@ fn associated_type_bounds<'tcx>( &ItemCtxt::new(tcx, assoc_item_def_id), item_ty, &bounds, + None, + None, SizedByDefault::Yes, span, ); @@ -70,6 +72,8 @@ fn opaque_type_bounds<'tcx>( &ItemCtxt::new(tcx, opaque_def_id), item_ty, &bounds, + None, + None, SizedByDefault::Yes, span, ) diff --git a/src/test/ui/maybe-bounds-where.rs b/src/test/ui/maybe-bounds-where.rs index cf011653c20..d7af0c42480 100644 --- a/src/test/ui/maybe-bounds-where.rs +++ b/src/test/ui/maybe-bounds-where.rs @@ -11,6 +11,7 @@ trait Trait<'a> {} struct S4(T) where for<'a> T: ?Trait<'a>; //~^ ERROR `?Trait` bounds are only permitted at the point where a type parameter is declared +//~| WARN default bound relaxed for a type parameter struct S5(*const T) where T: ?Trait<'static> + ?Sized; //~^ ERROR type parameter has more than one relaxed default bound diff --git a/src/test/ui/maybe-bounds-where.stderr b/src/test/ui/maybe-bounds-where.stderr index 0ef8e9e9c79..2aa6a8a3822 100644 --- a/src/test/ui/maybe-bounds-where.stderr +++ b/src/test/ui/maybe-bounds-where.stderr @@ -23,23 +23,29 @@ LL | struct S4(T) where for<'a> T: ?Trait<'a>; | ^ error: `?Trait` bounds are only permitted at the point where a type parameter is declared - --> $DIR/maybe-bounds-where.rs:20:18 + --> $DIR/maybe-bounds-where.rs:21:18 | LL | fn f() where T: ?Sized {} | ^ +warning: default bound relaxed for a type parameter, but this does nothing because the given bound is not a default; only `?Sized` is supported + --> $DIR/maybe-bounds-where.rs:12:11 + | +LL | struct S4(T) where for<'a> T: ?Trait<'a>; + | ^ + error[E0203]: type parameter has more than one relaxed default bound, only one is supported - --> $DIR/maybe-bounds-where.rs:15:11 + --> $DIR/maybe-bounds-where.rs:16:11 | LL | struct S5(*const T) where T: ?Trait<'static> + ?Sized; | ^ warning: default bound relaxed for a type parameter, but this does nothing because the given bound is not a default; only `?Sized` is supported - --> $DIR/maybe-bounds-where.rs:15:11 + --> $DIR/maybe-bounds-where.rs:16:11 | LL | struct S5(*const T) where T: ?Trait<'static> + ?Sized; | ^ -error: aborting due to 6 previous errors; 1 warning emitted +error: aborting due to 6 previous errors; 2 warnings emitted For more information about this error, try `rustc --explain E0203`.