Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not overwrite lifetime binders for another HirId. #101454

Merged
merged 2 commits into from
Sep 28, 2022
Merged

Do not overwrite lifetime binders for another HirId. #101454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
337a73d
Do not overwrite binders for another HirId.
cjgillot Sep 5, 2022
dd69ce3
Bless stats.
cjgillot Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/rustc_ast_lowering/src/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1478,6 +1478,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let bounded_ty =
self.ty_path(ty_id, param_span, hir::QPath::Resolved(None, ty_path));
Some(hir::WherePredicate::BoundPredicate(hir::WhereBoundPredicate {
hir_id: self.next_id(),
bounded_ty: self.arena.alloc(bounded_ty),
bounds,
span,
Expand Down Expand Up @@ -1508,6 +1509,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
ref bounds,
span,
}) => hir::WherePredicate::BoundPredicate(hir::WhereBoundPredicate {
hir_id: self.next_id(),
bound_generic_params: self.lower_generic_params(bound_generic_params),
bounded_ty: self
.lower_ty(bounded_ty, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/hir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,7 @@ pub enum PredicateOrigin {
/// A type bound (e.g., `for<'c> Foo: Send + Clone + 'c`).
#[derive(Debug, HashStable_Generic)]
pub struct WhereBoundPredicate<'hir> {
pub hir_id: HirId,
pub span: Span,
/// Origin of the predicate.
pub origin: PredicateOrigin,
Expand Down
14 changes: 11 additions & 3 deletions compiler/rustc_hir/src/intravisit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -847,20 +847,28 @@ pub fn walk_where_predicate<'v, V: Visitor<'v>>(
) {
match *predicate {
WherePredicate::BoundPredicate(WhereBoundPredicate {
hir_id,
ref bounded_ty,
bounds,
bound_generic_params,
..
origin: _,
span: _,
}) => {
visitor.visit_id(hir_id);
visitor.visit_ty(bounded_ty);
walk_list!(visitor, visit_param_bound, bounds);
walk_list!(visitor, visit_generic_param, bound_generic_params);
}
WherePredicate::RegionPredicate(WhereRegionPredicate { ref lifetime, bounds, .. }) => {
WherePredicate::RegionPredicate(WhereRegionPredicate {
ref lifetime,
bounds,
span: _,
in_where_clause: _,
}) => {
visitor.visit_lifetime(lifetime);
walk_list!(visitor, visit_param_bound, bounds);
}
WherePredicate::EqPredicate(WhereEqPredicate { ref lhs_ty, ref rhs_ty, .. }) => {
WherePredicate::EqPredicate(WhereEqPredicate { ref lhs_ty, ref rhs_ty, span: _ }) => {
visitor.visit_ty(lhs_ty);
visitor.visit_ty(rhs_ty);
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/collect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ impl<'tcx> ItemCtxt<'tcx> {
} else {
None
};
let bvars = self.tcx.late_bound_vars(bp.bounded_ty.hir_id);
let bvars = self.tcx.late_bound_vars(bp.hir_id);

bp.bounds.iter().filter_map(move |b| bt.map(|bt| (bt, b, bvars))).filter(
|(_, b, _)| match assoc_name {
Expand Down Expand Up @@ -2295,7 +2295,7 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericP
match predicate {
hir::WherePredicate::BoundPredicate(bound_pred) => {
let ty = icx.to_ty(bound_pred.bounded_ty);
let bound_vars = icx.tcx.late_bound_vars(bound_pred.bounded_ty.hir_id);
let bound_vars = icx.tcx.late_bound_vars(bound_pred.hir_id);

// Keep the type around in a dummy predicate, in case of no bounds.
// That way, `where Ty:` is not a complete noop (see #53696) and `Ty`
Expand Down
55 changes: 35 additions & 20 deletions compiler/rustc_resolve/src/late/lifetimes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ fn convert_named_region_map(named_region_map: NamedRegionMap) -> ResolveLifetime
}

debug!(?rl.defs);
debug!(?rl.late_bound_vars);
rl
}

Expand Down Expand Up @@ -507,7 +508,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
})
.unzip();

self.map.late_bound_vars.insert(e.hir_id, binders);
self.record_late_bound_vars(e.hir_id, binders);
let scope = Scope::Binder {
hir_id: e.hir_id,
lifetimes,
Expand All @@ -531,7 +532,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
match &item.kind {
hir::ItemKind::Impl(hir::Impl { of_trait, .. }) => {
if let Some(of_trait) = of_trait {
self.map.late_bound_vars.insert(of_trait.hir_ref_id, Vec::default());
self.record_late_bound_vars(of_trait.hir_ref_id, Vec::default());
}
}
_ => {}
Expand Down Expand Up @@ -583,7 +584,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
resolved_lifetimes.late_bound_vars.iter()
{
late_bound_vars.iter().for_each(|(&local_id, late_bound_vars)| {
self.map.late_bound_vars.insert(
self.record_late_bound_vars(
hir::HirId { owner, local_id },
late_bound_vars.clone(),
);
Expand Down Expand Up @@ -614,7 +615,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
GenericParamKind::Type { .. } | GenericParamKind::Const { .. } => None,
})
.collect();
self.map.late_bound_vars.insert(item.hir_id(), vec![]);
self.record_late_bound_vars(item.hir_id(), vec![]);
let scope = Scope::Binder {
hir_id: item.hir_id(),
lifetimes,
Expand Down Expand Up @@ -663,7 +664,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
(pair, r)
})
.unzip();
self.map.late_bound_vars.insert(ty.hir_id, binders);
self.record_late_bound_vars(ty.hir_id, binders);
let scope = Scope::Binder {
hir_id: ty.hir_id,
lifetimes,
Expand Down Expand Up @@ -817,7 +818,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
GenericParamKind::Type { .. } | GenericParamKind::Const { .. } => {}
}
}
self.map.late_bound_vars.insert(ty.hir_id, vec![]);
self.record_late_bound_vars(ty.hir_id, vec![]);

let scope = Scope::Binder {
hir_id: ty.hir_id,
Expand Down Expand Up @@ -861,7 +862,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
GenericParamKind::Type { .. } | GenericParamKind::Const { .. } => None,
})
.collect();
self.map.late_bound_vars.insert(trait_item.hir_id(), vec![]);
self.record_late_bound_vars(trait_item.hir_id(), vec![]);
let scope = Scope::Binder {
hir_id: trait_item.hir_id(),
lifetimes,
Expand Down Expand Up @@ -909,9 +910,9 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
GenericParamKind::Const { .. } | GenericParamKind::Type { .. } => None,
})
.collect();
self.map.late_bound_vars.insert(ty.hir_id, vec![]);
self.record_late_bound_vars(impl_item.hir_id(), vec![]);
let scope = Scope::Binder {
hir_id: ty.hir_id,
hir_id: impl_item.hir_id(),
lifetimes,
s: self.scope,
scope_type: BinderScopeType::Normal,
Expand Down Expand Up @@ -995,33 +996,38 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
for predicate in generics.predicates {
match predicate {
&hir::WherePredicate::BoundPredicate(hir::WhereBoundPredicate {
hir_id,
ref bounded_ty,
bounds,
ref bound_generic_params,
origin,
..
}) => {
let (lifetimes, binders): (FxIndexMap<LocalDefId, Region>, Vec<_>) =
let lifetimes: FxIndexMap<LocalDefId, Region> =
bound_generic_params
.iter()
.filter(|param| {
matches!(param.kind, GenericParamKind::Lifetime { .. })
})
.enumerate()
.map(|(late_bound_idx, param)| {
let pair =
Region::late(late_bound_idx as u32, this.tcx.hir(), param);
let r = late_region_as_bound_region(this.tcx, &pair.1);
(pair, r)
Region::late(late_bound_idx as u32, this.tcx.hir(), param)
})
.collect();
let binders: Vec<_> =
lifetimes
.iter()
.map(|(_, region)| {
late_region_as_bound_region(this.tcx, region)
})
.unzip();
this.map.late_bound_vars.insert(bounded_ty.hir_id, binders.clone());
.collect();
this.record_late_bound_vars(hir_id, binders.clone());
// Even if there are no lifetimes defined here, we still wrap it in a binder
// scope. If there happens to be a nested poly trait ref (an error), that
// will be `Concatenating` anyways, so we don't have to worry about the depth
// being wrong.
let scope = Scope::Binder {
hir_id: bounded_ty.hir_id,
hir_id,
lifetimes,
s: this.scope,
scope_type: BinderScopeType::Normal,
Expand Down Expand Up @@ -1089,7 +1095,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
// imagine there's a better way to go about this.
let (binders, scope_type) = self.poly_trait_ref_binder_info();

self.map.late_bound_vars.insert(*hir_id, binders);
self.record_late_bound_vars(*hir_id, binders);
let scope = Scope::Binder {
hir_id: *hir_id,
lifetimes: FxIndexMap::default(),
Expand Down Expand Up @@ -1127,7 +1133,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
binders.extend(binders_iter);

debug!(?binders);
self.map.late_bound_vars.insert(trait_ref.trait_ref.hir_ref_id, binders);
self.record_late_bound_vars(trait_ref.trait_ref.hir_ref_id, binders);

// Always introduce a scope here, even if this is in a where clause and
// we introduced the binders around the bounded Ty. In that case, we
Expand Down Expand Up @@ -1211,6 +1217,15 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}
}

fn record_late_bound_vars(&mut self, hir_id: hir::HirId, binder: Vec<ty::BoundVariableKind>) {
if let Some(old) = self.map.late_bound_vars.insert(hir_id, binder) {
bug!(
"overwrote bound vars for {hir_id:?}:\nold={old:?}\nnew={:?}",
self.map.late_bound_vars[&hir_id]
)
}
}

/// Visits self by adding a scope and handling recursive walk over the contents with `walk`.
///
/// Handles visiting fns and methods. These are a bit complicated because we must distinguish
Expand Down Expand Up @@ -1268,7 +1283,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
late_region_as_bound_region(self.tcx, &pair.1)
})
.collect();
self.map.late_bound_vars.insert(hir_id, binders);
self.record_late_bound_vars(hir_id, binders);
let scope = Scope::Binder {
hir_id,
lifetimes,
Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/stats/hir-stats.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,11 @@ hir-stats - Expr 32 ( 0.3%) 1
hir-stats FnDecl 120 ( 1.2%) 3 40
hir-stats Attribute 128 ( 1.3%) 4 32
hir-stats GenericArgs 144 ( 1.5%) 3 48
hir-stats Variant 160 ( 1.7%) 2 80
hir-stats WherePredicate 168 ( 1.7%) 3 56
hir-stats - BoundPredicate 168 ( 1.7%) 3
hir-stats Variant 160 ( 1.6%) 2 80
hir-stats GenericBound 192 ( 2.0%) 4 48
hir-stats - Trait 192 ( 2.0%) 4
hir-stats WherePredicate 192 ( 2.0%) 3 64
hir-stats - BoundPredicate 192 ( 2.0%) 3
hir-stats Block 288 ( 3.0%) 6 48
hir-stats Pat 360 ( 3.7%) 5 72
hir-stats - Wild 72 ( 0.7%) 1
Expand All @@ -169,10 +169,10 @@ hir-stats - Enum 80 ( 0.8%) 1
hir-stats - ExternCrate 80 ( 0.8%) 1
hir-stats - ForeignMod 80 ( 0.8%) 1
hir-stats - Impl 80 ( 0.8%) 1
hir-stats - Fn 160 ( 1.7%) 2
hir-stats - Fn 160 ( 1.6%) 2
hir-stats - Use 400 ( 4.1%) 5
hir-stats Path 1_536 (15.9%) 32 48
hir-stats Path 1_536 (15.8%) 32 48
hir-stats PathSegment 2_240 (23.1%) 40 56
hir-stats ----------------------------------------------------------------
hir-stats Total 9_680
hir-stats Total 9_704
hir-stats
18 changes: 18 additions & 0 deletions src/test/ui/where-clauses/higher-ranked-fn-type.quiet.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error[E0277]: the trait bound `for<'b> for<'b> fn(&'b ()): Foo` is not satisfied
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diagnostic looks a bit silly. I don't know why we are choosing a 'b for the inner param.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit of a shame.

--> $DIR/higher-ranked-fn-type.rs:20:5
|
LL | called()
| ^^^^^^ the trait `for<'b> Foo` is not implemented for `for<'b> fn(&'b ())`
|
note: required by a bound in `called`
--> $DIR/higher-ranked-fn-type.rs:12:25
|
LL | fn called()
| ------ required by a bound in this
LL | where
LL | for<'b> fn(&'b ()): Foo,
| ^^^ required by this bound in `called`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
25 changes: 25 additions & 0 deletions src/test/ui/where-clauses/higher-ranked-fn-type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// revisions: quiet verbose
// [verbose]compile-flags: -Zverbose

#![allow(unused_parens)]

trait Foo {
type Assoc;
}

fn called()
where
for<'b> fn(&'b ()): Foo,
{
}

fn caller()
where
(for<'a> fn(&'a ())): Foo,
{
called()
//[quiet]~^ ERROR the trait bound `for<'b> for<'b> fn(&'b ()): Foo` is not satisfied
//[verbose]~^^ ERROR the trait bound `for<'b> fn(&ReLateBound(
}

fn main() {}
18 changes: 18 additions & 0 deletions src/test/ui/where-clauses/higher-ranked-fn-type.verbose.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error[E0277]: the trait bound `for<'b> fn(&ReLateBound(DebruijnIndex(1), BoundRegion { var: 0, kind: BrNamed(DefId(0:6 ~ higher_ranked_fn_type[1209]::called::'b), 'b) }) ()): Foo` is not satisfied
--> $DIR/higher-ranked-fn-type.rs:20:5
|
LL | called()
| ^^^^^^ the trait `for<'b> Foo` is not implemented for `fn(&ReLateBound(DebruijnIndex(1), BoundRegion { var: 0, kind: BrNamed(DefId(0:6 ~ higher_ranked_fn_type[1209]::called::'b), 'b) }) ())`
|
note: required by a bound in `called`
--> $DIR/higher-ranked-fn-type.rs:12:25
|
LL | fn called()
| ------ required by a bound in this
LL | where
LL | for<'b> fn(&'b ()): Foo,
| ^^^ required by this bound in `called`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.