Skip to content

Commit

Permalink
Auto merge of #88061 - jackh726:genericbound-cleanup, r=estebank
Browse files Browse the repository at this point in the history 
Remove `hir::GenericBound::Unsized`

Rather than "moving" the `?Sized` bounds to the param bounds, just also check where clauses in `astconv`. I also did some related cleanup here, but that's not strictly neccesary. Also going to do a perf run here.

r? `@estebank`
  • Loading branch information
@bors
bors committed Sep 8, 2021
2 parents 0d0d2fe + 22ef04e commit 72969f6
Show file tree
Hide file tree
Showing 15 changed files with 300 additions and 389 deletions.
46 changes: 16 additions & 30 deletions compiler/rustc_ast_lowering/src/item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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<Vec<_>> = 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, `add_implicitly_sized` 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 {
Expand All @@ -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;
}
}
Expand All @@ -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),
}
Expand Down Expand Up @@ -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,
Expand Down
25 changes: 5 additions & 20 deletions compiler/rustc_ast_lowering/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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<Vec<GenericBound>>,
mut itctx: ImplTraitContext<'s, 'hir>,
) -> impl Iterator<Item = hir::GenericParam<'hir>> + 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<Vec<GenericBound>>,
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<Vec<GenericBound>>,
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(&param.bounds, itctx.reborrow()).collect()
});
Expand Down Expand Up @@ -2057,12 +2051,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
(param_name, kind)
}
GenericParamKind::Type { ref default, .. } => {
let add_bounds = add_bounds.get(&param.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))
Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_hir/src/hir.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,10 +441,12 @@ pub enum GenericBound<'hir> {
Trait(PolyTraitRef<'hir>, TraitBoundModifier),
// FIXME(davidtwco): Introduce `PolyTraitRef::LangItem`
LangItemTrait(LangItem, Span, HirId, &'hir GenericArgs<'hir>),
Unsized(Span),
Outlives(Lifetime),
}

#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(GenericBound<'_>, 48);

impl GenericBound<'_> {
pub fn trait_ref(&self) -> Option<&TraitRef<'_>> {
match self {
Expand All @@ -458,7 +460,6 @@ impl GenericBound<'_> {
GenericBound::Trait(t, ..) => t.span,
GenericBound::LangItemTrait(_, span, ..) => *span,
GenericBound::Outlives(l) => l.span,
GenericBound::Unsized(span) => *span,
}
}
}
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_hir/src/intravisit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(_) => {}
}
}

Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_hir_pretty/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2232,9 +2232,6 @@ impl<'a> State<'a> {
GenericBound::Outlives(lt) => {
self.print_lifetime(lt);
}
GenericBound::Unsized(_) => {
self.s.word("?Sized");
}
}
}
}
Expand Down
12 changes: 3 additions & 9 deletions compiler/rustc_middle/src/ty/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -114,10 +113,8 @@ fn suggest_removing_unsized_bound(
def_id: Option<DefId>,
) {
// 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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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 \
Expand All @@ -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,
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_save_analysis/src/dump_visitor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 72969f6

Please sign in to comment.