Move nested quantification check to ast_validation

jackh726 · jackh726 · commit 4568e7d62edc · 2021-04-21T03:12:04.000-04:00
diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs
@@ -81,6 +81,13 @@ struct AstValidator<'a> {
     is_assoc_ty_bound_banned: bool,
 
     lint_buffer: &'a mut LintBuffer,
+
+    /// This is slightly complicated. Our representation for poly-trait-refs contains a single
+    /// binder and thus we only allow a single level of quantification. However,
+    /// the syntax of Rust permits quantification in two places in where clauses,
+    /// e.g., `T: for <'a> Foo<'a>` and `for <'a, 'b> &'b T: Foo<'a>`. If both are
+    /// defined, then error.
+    trait_ref_hack: bool,
 }
 
 impl<'a> AstValidator<'a> {
@@ -1213,8 +1220,25 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 deny_equality_constraints(self, predicate, generics);
             }
         }
+        walk_list!(self, visit_generic_param, &generics.params);
+        for predicate in &generics.where_clause.predicates {
+            match predicate {
+                WherePredicate::BoundPredicate(bound_pred) => {
+                    // A type binding, eg `for<'c> Foo: Send+Clone+'c`
+                    self.check_late_bound_lifetime_defs(&bound_pred.bound_generic_params);
 
-        visit::walk_generics(self, generics)
+                    self.visit_ty(&bound_pred.bounded_ty);
+
+                    self.trait_ref_hack = !bound_pred.bound_generic_params.is_empty();
+                    walk_list!(self, visit_param_bound, &bound_pred.bounds);
+                    walk_list!(self, visit_generic_param, &bound_pred.bound_generic_params);
+                    self.trait_ref_hack = false;
+                }
+                _ => {
+                    self.visit_where_predicate(predicate);
+                }
+            }
+        }
     }
 
     fn visit_generic_param(&mut self, param: &'a GenericParam) {
@@ -1263,17 +1287,21 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         visit::walk_pat(self, pat)
     }
 
-    fn visit_where_predicate(&mut self, p: &'a WherePredicate) {
-        if let &WherePredicate::BoundPredicate(ref bound_predicate) = p {
-            // A type binding, eg `for<'c> Foo: Send+Clone+'c`
-            self.check_late_bound_lifetime_defs(&bound_predicate.bound_generic_params);
-        }
-        visit::walk_where_predicate(self, p);
-    }
-
     fn visit_poly_trait_ref(&mut self, t: &'a PolyTraitRef, m: &'a TraitBoundModifier) {
         self.check_late_bound_lifetime_defs(&t.bound_generic_params);
+        if self.trait_ref_hack && !t.bound_generic_params.is_empty() {
+            struct_span_err!(
+                self.err_handler(),
+                t.span,
+                E0316,
+                "nested quantification of lifetimes"
+            )
+            .emit();
+        }
+        let trait_ref_hack = self.trait_ref_hack;
+        self.trait_ref_hack = false;
         visit::walk_poly_trait_ref(self, t, m);
+        self.trait_ref_hack = trait_ref_hack;
     }
 
     fn visit_variant_data(&mut self, s: &'a VariantData) {
@@ -1492,6 +1520,7 @@ pub fn check_crate(session: &Session, krate: &Crate, lints: &mut LintBuffer) ->
         is_impl_trait_banned: false,
         is_assoc_ty_bound_banned: false,
         lint_buffer: lints,
+        trait_ref_hack: false,
     };
     visit::walk_crate(&mut validator, krate);
 
diff --git a/compiler/rustc_resolve/src/late/lifetimes.rs b/compiler/rustc_resolve/src/late/lifetimes.rs
@@ -278,29 +278,6 @@ enum BinderScopeType {
     /// you had `T: for<'a>  Foo<Bar: for<'b> Baz<'a, 'b>>`, then the `for<'a>`
     /// scope uses `PolyTraitRef`.
     PolyTraitRef,
-    /// This is slightly complicated. Our representation for poly-trait-refs contains a single
-    /// binder and thus we only allow a single level of quantification. However,
-    /// the syntax of Rust permits quantification in two places in where clauses,
-    /// e.g., `T: for <'a> Foo<'a>` and `for <'a, 'b> &'b T: Foo<'a>`. In order
-    /// to get the De Bruijn indices correct when representing these constraints,
-    /// we should only introduce one scope. However, we want to support both
-    /// locations for the quantifier and during lifetime resolution we want
-    /// precise information (so we can't desugar in an earlier phase). Moreso,
-    /// an error here doesn't cause a bail from type checking, so we need to be
-    /// extra careful that we don't lose any bound var information for *either*
-    /// syntactic binder and that we track all lifetimes defined in both binders.
-    ///
-    /// This mechanism is similar to the concatenation done in nested poly trait
-    /// refs, i.e. the inner syntactic binder extends upon the lifetimes on the
-    /// outer syntactic binder. However, we require a separate variant here to
-    /// distinguish `for<'a> T: for<'b> Foo<'a, 'b>` from
-    /// `T: for<'a> Bar<Baz: for<'b> Foo<'a, 'b>>`. In this case, the innermost
-    /// `: for<'b> Foo<'a, 'b>` both have a `for<'a>` scope above it. However,
-    /// in the former case, we must emit an error because this is invalid syntax.
-    /// Put another way: `PolyTraitRef` and `BoundedTy` behave identically except
-    /// that `BoundedTy` is used to signal that an error should be emitted if
-    /// another syntactic binder is found.
-    BoundedTy,
     /// Within a syntactic trait ref, there may be multiple poly trait refs that
     /// are nested (under the `associcated_type_bounds` feature). The binders of
     /// the innner poly trait refs are extended from the outer poly trait refs
@@ -309,8 +286,7 @@ enum BinderScopeType {
     /// would be `Concatenating`. This also used in trait refs in where clauses
     /// where we have two binders `for<> T: for<> Foo` (I've intentionally left
     /// out any lifetimes because they aren't needed to show the two scopes).
-    /// See `BoundedTy` for a bit more details, but the inner `for<>` has a scope
-    /// of `Concatenating`.
+    /// The inner `for<>` has a scope of `Concatenating`.
     Concatenating,
     /// Any other binder scopes. These are "normal" in that they increase the binder
     /// depth, are fully syntactic, don't concatenate, and don't have special syntactical
@@ -1311,7 +1287,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
                             next_early_index,
                             track_lifetime_uses: true,
                             opaque_type_parent: false,
-                            scope_type: BinderScopeType::BoundedTy,
+                            scope_type: BinderScopeType::PolyTraitRef,
                         };
                         this.with(scope, |old_scope, this| {
                             this.check_lifetime_params(old_scope, &bound_generic_params);
@@ -1344,30 +1320,24 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
         // FIXME(jackh726): This is pretty weird. `LangItemTrait` doesn't go
         // through the regular poly trait ref code, so we don't get another
         // chance to introduce a binder. For now, I'm keeping the existing logic
-        // of "if there isn't a `BoundedTy` scope above us, add one", but I
+        // of "if there isn't a Binder scope above us, add one", but I
         // imagine there's a better way to go about this.
         let mut scope = self.scope;
         let trait_ref_hack = loop {
             match scope {
-                Scope::Body { .. } | Scope::Root => {
+                Scope::TraitRefBoundary { .. } | Scope::Body { .. } | Scope::Root => {
                     break false;
                 }
 
+                Scope::Binder { .. } => {
+                    break true;
+                }
+
                 Scope::Elision { s, .. }
                 | Scope::ObjectLifetimeDefault { s, .. }
                 | Scope::Supertrait { s, .. } => {
                     scope = s;
                 }
-
-                Scope::TraitRefBoundary { .. } => {
-                    break false;
-                }
-
-                Scope::Binder { scope_type, lifetimes, .. } => {
-                    let trait_ref_hack =
-                        matches!(scope_type, BinderScopeType::BoundedTy) && !lifetimes.is_empty();
-                    break trait_ref_hack;
-                }
             }
         };
         match bound {
@@ -1402,10 +1372,10 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
         let next_early_index = self.next_early_index();
         let mut scope = self.scope;
         let mut supertrait_lifetimes = vec![];
-        let (mut binders, trait_ref_hack, scope_type) = loop {
+        let (mut binders, scope_type) = loop {
             match scope {
                 Scope::Body { .. } | Scope::Root => {
-                    break (vec![], false, BinderScopeType::PolyTraitRef);
+                    break (vec![], BinderScopeType::PolyTraitRef);
                 }
 
                 Scope::Elision { s, .. } | Scope::ObjectLifetimeDefault { s, .. } => {
@@ -1420,10 +1390,10 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
                 Scope::TraitRefBoundary { .. } => {
                     // We should only see super trait lifetimes if there is a `Binder` above
                     assert!(supertrait_lifetimes.is_empty());
-                    break (vec![], false, BinderScopeType::PolyTraitRef);
+                    break (vec![], BinderScopeType::PolyTraitRef);
                 }
 
-                Scope::Binder { hir_id, scope_type, lifetimes, .. } => {
+                Scope::Binder { hir_id, scope_type, .. } => {
                     if let BinderScopeType::Other = scope_type {
                         bug!(
                             "Expected all syntacic poly trait refs to be surrounded by a `TraitRefBoundary`"
@@ -1434,30 +1404,11 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
                     let mut full_binders =
                         self.map.late_bound_vars.entry(*hir_id).or_default().clone();
                     full_binders.extend(supertrait_lifetimes.into_iter());
-                    let trait_ref_hack =
-                        matches!(scope_type, BinderScopeType::BoundedTy) && !lifetimes.is_empty();
-                    break (full_binders, trait_ref_hack, BinderScopeType::Concatenating);
+                    break (full_binders, BinderScopeType::Concatenating);
                 }
             }
         };
 
-        // See note on `BinderScopeType::BoundedTy`. If `for<..>`
-        // has been defined in both the outer and inner part of the
-        // trait ref, emit an error.
-        let has_lifetimes = trait_ref.bound_generic_params.iter().any(|param| match param.kind {
-            GenericParamKind::Lifetime { .. } => true,
-            _ => false,
-        });
-        if trait_ref_hack && has_lifetimes {
-            struct_span_err!(
-                self.tcx.sess,
-                trait_ref.span,
-                E0316,
-                "nested quantification of lifetimes"
-            )
-            .emit();
-        }
-
         let initial_bound_vars = binders.len() as u32;
         let mut lifetimes: FxHashMap<hir::ParamName, Region> = FxHashMap::default();
         let binders_iter = trait_ref
@@ -1486,7 +1437,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
         // 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
         // just reuse the concatenation functionality also present in nested trait
-        // refs. See `BinderScopeType::BoundedTy` for more details on that case.
+        // refs.
         let scope = Scope::Binder {
             hir_id: trait_ref.trait_ref.hir_ref_id,
             lifetimes,
@@ -2319,7 +2270,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
                     }
                     match scope_type {
                         BinderScopeType::Other => late_depth += 1,
-                        BinderScopeType::BoundedTy => late_depth += 1,
                         BinderScopeType::PolyTraitRef => late_depth += 1,
                         BinderScopeType::Concatenating => {}
                     }
@@ -3051,7 +3001,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
                     }
                     match scope_type {
                         BinderScopeType::Other => late_depth += 1,
-                        BinderScopeType::BoundedTy => late_depth += 1,
                         BinderScopeType::PolyTraitRef => late_depth += 1,
                         BinderScopeType::Concatenating => {}
                     }
@@ -3216,7 +3165,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
                 Scope::Binder { s, scope_type, .. } => {
                     match scope_type {
                         BinderScopeType::Other => late_depth += 1,
-                        BinderScopeType::BoundedTy => late_depth += 1,
                         BinderScopeType::PolyTraitRef => late_depth += 1,
                         BinderScopeType::Concatenating => {}
                     }
