Auto merge of rust-lang#101454 - cjgillot:concat-binders, r=estebank

Do not overwrite lifetime binders for another HirId.

This PR makes higher-ranked bounds in where clauses a bit more principled.
We used to conflate `for<'a> T: Trait` with `(for<'a> T): Trait`.
This PR separates both binders.

This caused issued with fn types, which have their own binder, causing us to overwrite the predicates's binders with `fn`'s binders, ICEing.

Fixes rust-lang#98594

Author: bors

compiler/rustc_ast_lowering/src/item.rs

@@ -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,
@@ -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)),

compiler/rustc_hir/src/hir.rs

@@ -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,

compiler/rustc_hir/src/intravisit.rs

@@ -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);
         }

compiler/rustc_hir_analysis/src/collect.rs

@@ -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 {
@@ -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`

compiler/rustc_resolve/src/late/lifetimes.rs

@@ -326,6 +326,7 @@ fn convert_named_region_map(named_region_map: NamedRegionMap) -> ResolveLifetime
     }
 
     debug!(?rl.defs);
+    debug!(?rl.late_bound_vars);
     rl
 }
 
@@ -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,
@@ -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());
                 }
             }
             _ => {}
@@ -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(),
                                     );
@@ -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,
@@ -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,
@@ -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,
@@ -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,
@@ -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,
@@ -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,
@@ -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(),
@@ -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
@@ -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
@@ -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,

src/test/ui/stats/hir-stats.stderr

@@ -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
@@ -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

src/test/ui/where-clauses/higher-ranked-fn-type.quiet.stderr

@@ -0,0 +1,18 @@
+error[E0277]: the trait bound `for<'b> for<'b> fn(&'b ()): Foo` is not satisfied
+  --> $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`.

src/test/ui/where-clauses/higher-ranked-fn-type.rs

@@ -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() {}

src/test/ui/where-clauses/higher-ranked-fn-type.verbose.stderr

@@ -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`.