Skip to content

Commit 81aa120

Browse files
jackh726jackh726
committed
Don't move ?Trait bounds to param bounds if they're in where clauses
1 parent 752c793 commit 81aa120

File tree

12 files changed

+103
-82
lines changed

12 files changed

+103
-82
lines changed

‎compiler/rustc_ast_lowering/src/item.rs

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use super::{AnonymousLifetimeMode, LoweringContext, ParamMode};
22
use super::{ImplTraitContext, ImplTraitPosition};
33
use crate::Arena;
44

5-
use rustc_ast::node_id::NodeMap;
65
use rustc_ast::ptr::P;
76
use rustc_ast::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor};
87
use rustc_ast::*;
@@ -1366,8 +1365,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
13661365
generics: &Generics,
13671366
itctx: ImplTraitContext<'_, 'hir>,
13681367
) -> GenericsCtor<'hir> {
1369-
// Collect `?Trait` bounds in where clause and move them to parameter definitions.
1370-
let mut add_bounds: NodeMap<Vec<_>> = Default::default();
1368+
// Error if `?Trait` bounds in where clauses don't refer directly to type paramters.
1369+
// Note: we used to clone these bounds directly onto the type parameter (and avoid lowering
1370+
// these into hir when we lower thee where clauses), but this makes it quite difficult to
1371+
// keep track of the Span info. Now, `is_unsized` in `AstConv` checks both param bounds and
1372+
// where clauses for `?Sized`.
13711373
for pred in &generics.where_clause.predicates {
13721374
if let WherePredicate::BoundPredicate(ref bound_pred) = *pred {
13731375
'next_bound: for bound in &bound_pred.bounds {
@@ -1383,7 +1385,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
13831385
{
13841386
for param in &generics.params {
13851387
if def_id == self.resolver.local_def_id(param.id).to_def_id() {
1386-
add_bounds.entry(param.id).or_default().push(bound.clone());
13871388
continue 'next_bound;
13881389
}
13891390
}
@@ -1401,7 +1402,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
14011402
}
14021403

14031404
GenericsCtor {
1404-
params: self.lower_generic_params_mut(&generics.params, &add_bounds, itctx).collect(),
1405+
params: self.lower_generic_params_mut(&generics.params, itctx).collect(),
14051406
where_clause: self.lower_where_clause(&generics.where_clause),
14061407
span: generics.span,
14071408
}
@@ -1434,32 +1435,17 @@ impl<'hir> LoweringContext<'_, 'hir> {
14341435
ref bounded_ty,
14351436
ref bounds,
14361437
span,
1437-
}) => {
1438-
self.with_in_scope_lifetime_defs(&bound_generic_params, |this| {
1439-
hir::WherePredicate::BoundPredicate(hir::WhereBoundPredicate {
1440-
bound_generic_params: this.lower_generic_params(
1441-
bound_generic_params,
1442-
&NodeMap::default(),
1443-
ImplTraitContext::disallowed(),
1444-
),
1445-
bounded_ty: this.lower_ty(bounded_ty, ImplTraitContext::disallowed()),
1446-
bounds: this.arena.alloc_from_iter(bounds.iter().map(
1447-
|bound| match bound {
1448-
// We used to ignore `?Trait` bounds, as they were copied into type
1449-
// parameters already, but we need to keep them around only for
1450-
// diagnostics when we suggest removal of `?Sized` bounds. See
1451-
// `suggest_constraining_type_param`. This will need to change if
1452-
// we ever allow something *other* than `?Sized`.
1453-
GenericBound::Trait(p, TraitBoundModifier::Maybe) => {
1454-
hir::GenericBound::Unsized(p.span)
1455-
}
1456-
_ => this.lower_param_bound(bound, ImplTraitContext::disallowed()),
1457-
},
1458-
)),
1459-
span,
1460-
})
1438+
}) => self.with_in_scope_lifetime_defs(&bound_generic_params, |this| {
1439+
hir::WherePredicate::BoundPredicate(hir::WhereBoundPredicate {
1440+
bound_generic_params: this
1441+
.lower_generic_params(bound_generic_params, ImplTraitContext::disallowed()),
1442+
bounded_ty: this.lower_ty(bounded_ty, ImplTraitContext::disallowed()),
1443+
bounds: this.arena.alloc_from_iter(bounds.iter().map(|bound| {
1444+
this.lower_param_bound(bound, ImplTraitContext::disallowed())
1445+
})),
1446+
span,
14611447
})
1462-
}
1448+
}),
14631449
WherePredicate::RegionPredicate(WhereRegionPredicate {
14641450
ref lifetime,
14651451
ref bounds,

‎compiler/rustc_ast_lowering/src/lib.rs

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,7 +1364,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
13641364
hir::TyKind::BareFn(this.arena.alloc(hir::BareFnTy {
13651365
generic_params: this.lower_generic_params(
13661366
&f.generic_params,
1367-
&NodeMap::default(),
13681367
ImplTraitContext::disallowed(),
13691368
),
13701369
unsafety: this.lower_unsafety(f.unsafety),
@@ -2209,30 +2208,25 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
22092208
fn lower_generic_params_mut<'s>(
22102209
&'s mut self,
22112210
params: &'s [GenericParam],
2212-
add_bounds: &'s NodeMap<Vec<GenericBound>>,
22132211
mut itctx: ImplTraitContext<'s, 'hir>,
22142212
) -> impl Iterator<Item = hir::GenericParam<'hir>> + Captures<'a> + Captures<'s> {
2215-
params
2216-
.iter()
2217-
.map(move |param| self.lower_generic_param(param, add_bounds, itctx.reborrow()))
2213+
params.iter().map(move |param| self.lower_generic_param(param, itctx.reborrow()))
22182214
}
22192215

22202216
fn lower_generic_params(
22212217
&mut self,
22222218
params: &[GenericParam],
2223-
add_bounds: &NodeMap<Vec<GenericBound>>,
22242219
itctx: ImplTraitContext<'_, 'hir>,
22252220
) -> &'hir [hir::GenericParam<'hir>] {
2226-
self.arena.alloc_from_iter(self.lower_generic_params_mut(params, add_bounds, itctx))
2221+
self.arena.alloc_from_iter(self.lower_generic_params_mut(params, itctx))
22272222
}
22282223

22292224
fn lower_generic_param(
22302225
&mut self,
22312226
param: &GenericParam,
2232-
add_bounds: &NodeMap<Vec<GenericBound>>,
22332227
mut itctx: ImplTraitContext<'_, 'hir>,
22342228
) -> hir::GenericParam<'hir> {
2235-
let mut bounds: Vec<_> = self
2229+
let bounds: Vec<_> = self
22362230
.with_anonymous_lifetime_mode(AnonymousLifetimeMode::ReportError, |this| {
22372231
this.lower_param_bounds_mut(&param.bounds, itctx.reborrow()).collect()
22382232
});
@@ -2268,12 +2262,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
22682262
(param_name, kind)
22692263
}
22702264
GenericParamKind::Type { ref default, .. } => {
2271-
let add_bounds = add_bounds.get(&param.id).map_or(&[][..], |x| &x);
2272-
if !add_bounds.is_empty() {
2273-
let params = self.lower_param_bounds_mut(add_bounds, itctx.reborrow());
2274-
bounds.extend(params);
2275-
}
2276-
22772265
let kind = hir::GenericParamKind::Type {
22782266
default: default.as_ref().map(|x| {
22792267
self.lower_ty(x, ImplTraitContext::Disallowed(ImplTraitPosition::Other))
@@ -2327,11 +2315,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
23272315
p: &PolyTraitRef,
23282316
mut itctx: ImplTraitContext<'_, 'hir>,
23292317
) -> hir::PolyTraitRef<'hir> {
2330-
let bound_generic_params = self.lower_generic_params(
2331-
&p.bound_generic_params,
2332-
&NodeMap::default(),
2333-
itctx.reborrow(),
2334-
);
2318+
let bound_generic_params =
2319+
self.lower_generic_params(&p.bound_generic_params, itctx.reborrow());
23352320

23362321
let trait_ref = self.with_in_scope_lifetime_defs(&p.bound_generic_params, |this| {
23372322
// Any impl Trait types defined within this scope can capture

‎compiler/rustc_hir/src/hir.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,6 @@ pub enum GenericBound<'hir> {
441441
Trait(PolyTraitRef<'hir>, TraitBoundModifier),
442442
// FIXME(davidtwco): Introduce `PolyTraitRef::LangItem`
443443
LangItemTrait(LangItem, Span, HirId, &'hir GenericArgs<'hir>),
444-
Unsized(Span),
445444
Outlives(Lifetime),
446445
}
447446

@@ -461,7 +460,6 @@ impl GenericBound<'_> {
461460
GenericBound::Trait(t, ..) => t.span,
462461
GenericBound::LangItemTrait(_, span, ..) => *span,
463462
GenericBound::Outlives(l) => l.span,
464-
GenericBound::Unsized(span) => *span,
465463
}
466464
}
467465
}

‎compiler/rustc_hir/src/intravisit.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,6 @@ pub fn walk_param_bound<'v, V: Visitor<'v>>(visitor: &mut V, bound: &'v GenericB
889889
visitor.visit_generic_args(span, args);
890890
}
891891
GenericBound::Outlives(ref lifetime) => visitor.visit_lifetime(lifetime),
892-
GenericBound::Unsized(_) => {}
893892
}
894893
}
895894

‎compiler/rustc_hir_pretty/src/lib.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2230,9 +2230,6 @@ impl<'a> State<'a> {
22302230
GenericBound::Outlives(lt) => {
22312231
self.print_lifetime(lt);
22322232
}
2233-
GenericBound::Unsized(_) => {
2234-
self.s.word("?Sized");
2235-
}
22362233
}
22372234
}
22382235
}

‎compiler/rustc_middle/src/ty/diagnostics.rs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
33
use crate::ty::TyKind::*;
44
use crate::ty::{InferTy, TyCtxt, TyS};
5-
use rustc_data_structures::fx::FxHashSet;
65
use rustc_errors::{Applicability, DiagnosticBuilder};
76
use rustc_hir as hir;
87
use rustc_hir::def_id::DefId;
@@ -114,10 +113,8 @@ fn suggest_removing_unsized_bound(
114113
def_id: Option<DefId>,
115114
) {
116115
// See if there's a `?Sized` bound that can be removed to suggest that.
117-
// First look at the `where` clause because we can have `where T: ?Sized`, but that
118-
// `?Sized` bound is *also* included in the `GenericParam` as a bound, which breaks
119-
// the spans. Hence the somewhat involved logic that follows.
120-
let mut where_unsized_bounds = FxHashSet::default();
116+
// First look at the `where` clause because we can have `where T: ?Sized`,
117+
// then look at params.
121118
for (where_pos, predicate) in generics.where_clause.predicates.iter().enumerate() {
122119
match predicate {
123120
WherePredicate::BoundPredicate(WhereBoundPredicate {
@@ -140,7 +137,6 @@ fn suggest_removing_unsized_bound(
140137
}) if segment.ident.as_str() == param_name => {
141138
for (pos, bound) in bounds.iter().enumerate() {
142139
match bound {
143-
hir::GenericBound::Unsized(_) => {}
144140
hir::GenericBound::Trait(poly, hir::TraitBoundModifier::Maybe)
145141
if poly.trait_ref.trait_def_id() == def_id => {}
146142
_ => continue,
@@ -173,7 +169,6 @@ fn suggest_removing_unsized_bound(
173169
// ^^^^^^^^^
174170
(_, pos, _, _) => bounds[pos - 1].span().shrink_to_hi().to(bound.span()),
175171
};
176-
where_unsized_bounds.insert(bound.span());
177172
err.span_suggestion_verbose(
178173
sp,
179174
"consider removing the `?Sized` bound to make the \
@@ -189,8 +184,7 @@ fn suggest_removing_unsized_bound(
189184
for (pos, bound) in param.bounds.iter().enumerate() {
190185
match bound {
191186
hir::GenericBound::Trait(poly, hir::TraitBoundModifier::Maybe)
192-
if poly.trait_ref.trait_def_id() == def_id
193-
&& !where_unsized_bounds.contains(&bound.span()) =>
187+
if poly.trait_ref.trait_def_id() == def_id =>
194188
{
195189
let sp = match (param.bounds.len(), pos) {
196190
// T: ?Sized,

‎compiler/rustc_save_analysis/src/dump_visitor.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -693,7 +693,6 @@ impl<'tcx> DumpVisitor<'tcx> {
693693
(Some(self.tcx.require_lang_item(lang_item, Some(span))), span)
694694
}
695695
hir::GenericBound::Outlives(..) => continue,
696-
hir::GenericBound::Unsized(_) => continue,
697696
};
698697

699698
if let Some(id) = def_id {

‎compiler/rustc_typeck/src/astconv/mod.rs

Lines changed: 58 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -856,7 +856,13 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
856856
}
857857

858858
// Returns `true` if a bounds list includes `?Sized`.
859-
pub fn is_unsized(&self, ast_bounds: &[hir::GenericBound<'_>], span: Span) -> bool {
859+
fn is_unsized(
860+
&self,
861+
ast_bounds: &[hir::GenericBound<'_>],
862+
self_ty: Option<hir::HirId>,
863+
where_clause: Option<&[hir::WherePredicate<'_>]>,
864+
span: Span,
865+
) -> bool {
860866
let tcx = self.tcx();
861867

862868
// Try to find an unbound in bounds.
@@ -870,11 +876,38 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
870876
}
871877
}
872878
}
879+
if let (Some(self_ty), Some(where_clause)) = (self_ty, where_clause) {
880+
let self_ty_def_id = tcx.hir().local_def_id(self_ty).to_def_id();
881+
for clause in where_clause {
882+
match clause {
883+
hir::WherePredicate::BoundPredicate(pred) => {
884+
match pred.bounded_ty.kind {
885+
hir::TyKind::Path(hir::QPath::Resolved(_, path)) => match path.res {
886+
Res::Def(DefKind::TyParam, def_id) if def_id == self_ty_def_id => {}
887+
_ => continue,
888+
},
889+
_ => continue,
890+
}
891+
for ab in pred.bounds {
892+
if let hir::GenericBound::Trait(ptr, hir::TraitBoundModifier::Maybe) =
893+
ab
894+
{
895+
if unbound.is_none() {
896+
unbound = Some(&ptr.trait_ref);
897+
} else {
898+
tcx.sess.emit_err(MultipleRelaxedDefaultBounds { span });
899+
}
900+
}
901+
}
902+
}
903+
_ => {}
904+
}
905+
}
906+
}
873907

874908
let kind_id = tcx.lang_items().require(LangItem::Sized);
875909
match unbound {
876910
Some(tpb) => {
877-
// FIXME(#8559) currently requires the unbound to be built-in.
878911
if let Ok(kind_id) = kind_id {
879912
if tpb.path.res != Res::Def(DefKind::Trait, kind_id) {
880913
tcx.sess.span_warn(
@@ -943,8 +976,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
943976
false,
944977
);
945978
}
946-
hir::GenericBound::Trait(_, hir::TraitBoundModifier::Maybe)
947-
| hir::GenericBound::Unsized(_) => {}
979+
hir::GenericBound::Trait(_, hir::TraitBoundModifier::Maybe) => {}
948980
hir::GenericBound::LangItemTrait(lang_item, span, hir_id, args) => self
949981
.instantiate_lang_item_trait_ref(
950982
lang_item, span, hir_id, args, param_ty, bounds,
@@ -973,22 +1005,33 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
9731005
/// example above, but is not true in supertrait listings like `trait Foo: Bar + Baz`.
9741006
///
9751007
/// `span` should be the declaration size of the parameter.
976-
pub fn compute_bounds(
1008+
pub(crate) fn compute_bounds(
9771009
&self,
9781010
param_ty: Ty<'tcx>,
9791011
ast_bounds: &[hir::GenericBound<'_>],
1012+
self_ty: Option<hir::HirId>,
1013+
where_clause: Option<&[hir::WherePredicate<'_>]>,
9801014
sized_by_default: SizedByDefault,
9811015
span: Span,
9821016
) -> Bounds<'tcx> {
983-
self.compute_bounds_inner(param_ty, &ast_bounds, sized_by_default, span)
1017+
self.compute_bounds_inner(
1018+
param_ty,
1019+
&ast_bounds,
1020+
self_ty,
1021+
where_clause,
1022+
sized_by_default,
1023+
span,
1024+
)
9841025
}
9851026

9861027
/// Convert the bounds in `ast_bounds` that refer to traits which define an associated type
9871028
/// named `assoc_name` into ty::Bounds. Ignore the rest.
988-
pub fn compute_bounds_that_match_assoc_type(
1029+
pub(crate) fn compute_bounds_that_match_assoc_type(
9891030
&self,
9901031
param_ty: Ty<'tcx>,
9911032
ast_bounds: &[hir::GenericBound<'_>],
1033+
self_ty: Option<hir::HirId>,
1034+
where_clause: Option<&[hir::WherePredicate<'_>]>,
9921035
sized_by_default: SizedByDefault,
9931036
span: Span,
9941037
assoc_name: Ident,
@@ -1005,13 +1048,15 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
10051048
}
10061049
}
10071050

1008-
self.compute_bounds_inner(param_ty, &result, sized_by_default, span)
1051+
self.compute_bounds_inner(param_ty, &result, self_ty, where_clause, sized_by_default, span)
10091052
}
10101053

10111054
fn compute_bounds_inner(
10121055
&self,
10131056
param_ty: Ty<'tcx>,
10141057
ast_bounds: &[hir::GenericBound<'_>],
1058+
self_ty: Option<hir::HirId>,
1059+
where_clause: Option<&[hir::WherePredicate<'_>]>,
10151060
sized_by_default: SizedByDefault,
10161061
span: Span,
10171062
) -> Bounds<'tcx> {
@@ -1020,7 +1065,11 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
10201065
self.add_bounds(param_ty, ast_bounds, &mut bounds, ty::List::empty());
10211066

10221067
bounds.implicitly_sized = if let SizedByDefault::Yes = sized_by_default {
1023-
if !self.is_unsized(ast_bounds, span) { Some(span) } else { None }
1068+
if !self.is_unsized(ast_bounds, self_ty, where_clause, span) {
1069+
Some(span)
1070+
} else {
1071+
None
1072+
}
10241073
} else {
10251074
None
10261075
};

0 commit comments

Comments
 (0)