From d843e002bb836be3164bef80d6218228aec974a8 Mon Sep 17 00:00:00 2001
From: Dylan MacKenzie <ecstaticmorse@gmail.com>
Date: Sat, 4 Jan 2020 16:29:45 -0800
Subject: [PATCH] Check for `?const` in invalid contexts during AST validation

---
 src/librustc_passes/ast_validation.rs | 74 +++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs
index 5000cd5f52f..1e5e39217b7 100644
--- a/src/librustc_passes/ast_validation.rs
+++ b/src/librustc_passes/ast_validation.rs
@@ -24,6 +24,23 @@
 
 use rustc_error_codes::*;
 
+#[derive(Clone, Copy)]
+enum BoundContext {
+    ImplTrait,
+    TraitBounds,
+    TraitObject,
+}
+
+impl BoundContext {
+    fn description(&self) -> &'static str {
+        match self {
+            Self::ImplTrait => "`impl Trait`",
+            Self::TraitBounds => "supertraits",
+            Self::TraitObject => "trait objects",
+        }
+    }
+}
+
 struct AstValidator<'a> {
     session: &'a Session,
     has_proc_macro_decls: bool,
@@ -33,6 +50,11 @@ struct AstValidator<'a> {
     /// e.g., `impl Iterator<Item = impl Debug>`.
     outer_impl_trait: Option<Span>,
 
+    /// Tracks the context in which a bound can appear.
+    ///
+    /// This is used to forbid `?const Trait` bounds in certain contexts.
+    bound_context_stack: Vec<Option<BoundContext>>,
+
     /// Used to ban `impl Trait` in path projections like `<impl Iterator>::Item`
     /// or `Foo::Bar<impl Trait>`
     is_impl_trait_banned: bool,
@@ -58,9 +80,21 @@ fn with_banned_assoc_ty_bound(&mut self, f: impl FnOnce(&mut Self)) {
     }
 
     fn with_impl_trait(&mut self, outer: Option<Span>, f: impl FnOnce(&mut Self)) {
+        self.bound_context_stack.push(outer.map(|_| BoundContext::ImplTrait));
         let old = mem::replace(&mut self.outer_impl_trait, outer);
         f(self);
         self.outer_impl_trait = old;
+        self.bound_context_stack.pop();
+    }
+
+    fn with_bound_context(&mut self, ctx: Option<BoundContext>, f: impl FnOnce(&mut Self)) {
+        self.bound_context_stack.push(ctx);
+        f(self);
+        self.bound_context_stack.pop();
+    }
+
+    fn innermost_bound_context(&mut self) -> Option<BoundContext> {
+        self.bound_context_stack.iter().rev().find(|x| x.is_some()).copied().flatten()
     }
 
     fn visit_assoc_ty_constraint_from_generic_args(&mut self, constraint: &'a AssocTyConstraint) {
@@ -84,6 +118,11 @@ fn walk_ty(&mut self, t: &'a Ty) {
             TyKind::ImplTrait(..) => {
                 self.with_impl_trait(Some(t.span), |this| visit::walk_ty(this, t))
             }
+            TyKind::TraitObject(..) => {
+                self.with_bound_context(Some(BoundContext::TraitObject), |this| {
+                    visit::walk_ty(this, t)
+                });
+            }
             TyKind::Path(ref qself, ref path) => {
                 // We allow these:
                 //  - `Option<impl Trait>`
@@ -192,6 +231,8 @@ fn check_trait_fn_not_const(&self, constness: Spanned<Constness>) {
         }
     }
 
+    // FIXME(ecstaticmorse): Instead, use the `bound_context_stack` to check this in
+    // `visit_param_bound`.
     fn no_questions_in_bounds(&self, bounds: &GenericBounds, where_: &str, is_trait: bool) {
         for bound in bounds {
             if let GenericBound::Trait(ref poly, TraitBoundModifier::Maybe) = *bound {
@@ -697,6 +738,15 @@ fn visit_item(&mut self, item: &'a Item) {
                     }
                 }
                 self.no_questions_in_bounds(bounds, "supertraits", true);
+
+                // Equivalent of `visit::walk_item` for `ItemKind::Trait` that inserts a bound
+                // context for the supertraits.
+                self.visit_generics(generics);
+                self.with_bound_context(Some(BoundContext::TraitBounds), |this| {
+                    walk_list!(this, visit_param_bound, bounds);
+                });
+                walk_list!(self, visit_trait_item, trait_items);
+                return;
             }
             ItemKind::Mod(_) => {
                 // Ensure that `path` attributes on modules are recorded as used (cf. issue #35584).
@@ -841,6 +891,29 @@ fn visit_generic_param(&mut self, param: &'a GenericParam) {
         visit::walk_generic_param(self, param);
     }
 
+    fn visit_param_bound(&mut self, bound: &'a GenericBound) {
+        if let GenericBound::Trait(poly, maybe_bound) = bound {
+            match poly.trait_ref.constness {
+                Some(Constness::NotConst) => {
+                    if *maybe_bound == TraitBoundModifier::Maybe {
+                        self.err_handler()
+                            .span_err(bound.span(), "`?const` and `?` are mutually exclusive");
+                    }
+
+                    if let Some(ctx) = self.innermost_bound_context() {
+                        let msg = format!("`?const` is not permitted in {}", ctx.description());
+                        self.err_handler().span_err(bound.span(), &msg);
+                    }
+                }
+
+                Some(Constness::Const) => bug!("Parser should reject bare `const` on bounds"),
+                None => {}
+            }
+        }
+
+        visit::walk_param_bound(self, bound)
+    }
+
     fn visit_pat(&mut self, pat: &'a Pat) {
         match pat.kind {
             PatKind::Lit(ref expr) => {
@@ -949,6 +1022,7 @@ pub fn check_crate(session: &Session, krate: &Crate, lints: &mut lint::LintBuffe
         session,
         has_proc_macro_decls: false,
         outer_impl_trait: None,
+        bound_context_stack: Vec::new(),
         is_impl_trait_banned: false,
         is_assoc_ty_bound_banned: false,
         lint_buffer: lints,
-- 
2.44.0