Enforce ordering for const generic parameters

Author: varkor

src/librustc/ty/relate.rs

@@ -476,7 +476,7 @@ pub fn super_relate_tys<'a, 'gcx, 'tcx, R>(relation: &mut R,
                 }
                 match x.val {
                     ConstValue::Unevaluated(def_id, substs) => {
-                        // TODO(varkor): this case should be entirely removed.
+                        // TODO(const_generics): this case should be entirely removed.
                         // FIXME(eddyb) get the right param_env.
                         let param_env = ty::ParamEnv::empty();
                         match tcx.lift_to_global(&substs) {

src/librustc_passes/ast_validation.rs

@@ -429,30 +429,32 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
     }
 
     fn visit_generics(&mut self, generics: &'a Generics) {
-        let mut seen_non_lifetime_param = false;
-        let mut seen_default = None;
+        let mut max_param: Option<ParamKindOrd> = None;
+        let mut prev_ty_default = None;
         for param in &generics.params {
-            match (&param.kind, seen_non_lifetime_param) {
-                (GenericParamKind::Lifetime { .. }, true) => {
-                    self.err_handler()
-                        .span_err(param.ident.span, "lifetime parameters must be leading");
-                },
-                (GenericParamKind::Lifetime { .. }, false) => {}
-                (GenericParamKind::Type { ref default, .. }, _) => {
-                    seen_non_lifetime_param = true;
+            let param_ord = match param.kind {
+                GenericParamKind::Lifetime { .. } => ParamKindOrd::Lifetime,
+                GenericParamKind::Type { ref default, .. } => {
                     if default.is_some() {
-                        seen_default = Some(param.ident.span);
-                    } else if let Some(span) = seen_default {
+                        prev_ty_default = Some(param.ident.span);
+                    } else if let Some(span) = prev_ty_default {
                         self.err_handler()
                             .span_err(span, "type parameters with a default must be trailing");
                         break;
                     }
+                    ParamKindOrd::Type
                 }
-                (GenericParamKind::Const { .. }, _) => {
-                    seen_non_lifetime_param = true;
-                    // TODO(const_generics): defaults
+                GenericParamKind::Const { .. } => ParamKindOrd::Const,
+            };
+            let max_param = &mut max_param;
+            match max_param {
+                Some(max_param) if *max_param > param_ord => {
+                    self.err_handler().span_err(param.ident.span,
+                        &format!("{} parameters must be placed before {} parameters",
+                            param_ord, max_param));
                 }
-            }
+                Some(_) | None => *max_param = Some(param_ord),
+            };
         }
         for predicate in &generics.where_clause.predicates {
             if let WherePredicate::EqPredicate(ref predicate) = *predicate {

src/librustdoc/clean/mod.rs

@@ -3120,7 +3120,7 @@ impl Clean<Path> for hir::Path {
 #[derive(Clone, RustcEncodable, RustcDecodable, PartialEq, Eq, Debug, Hash)]
 pub enum GenericArgs {
     AngleBracketed {
-        // TODO(varkor): clean this up
+        // TODO(const_generics): clean this up
         lifetimes: Vec<Lifetime>,
         types: Vec<Type>,
         consts: Vec<Constant>,

src/librustdoc/html/format.rs

@@ -315,7 +315,7 @@ impl fmt::Display for clean::GenericArgs {
                 ref consts,
                 ref bindings,
             } => {
-                // TODO(varkor): clean this up
+                // TODO(const_generics): clean this up
                 if !lifetimes.is_empty() ||
                    !types.is_empty() ||
                    !consts.is_empty() ||

src/libsyntax/ast.rs

@@ -299,6 +299,26 @@ impl GenericBound {
 
 pub type GenericBounds = Vec<GenericBound>;
 
+/// Specifies the enforced ordering for generic parameters. In the future,
+/// if we wanted to relax this order, we could override `PartialEq`/`PartialOrd`,
+/// to allow the kinds to be equal.
+#[derive(PartialEq, PartialOrd)]
+pub enum ParamKindOrd {
+    Lifetime,
+    Type,
+    Const,
+}
+
+impl fmt::Display for ParamKindOrd {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            ParamKindOrd::Lifetime => "lifetime".fmt(f),
+            ParamKindOrd::Type => "type".fmt(f),
+            ParamKindOrd::Const => "const".fmt(f),
+        }
+    }
+}
+
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
 pub enum GenericParamKind {
     /// A lifetime definition, e.g. `'a: 'b+'c+'d`.
@@ -1412,7 +1432,7 @@ impl fmt::Debug for ConstTy {
 
 impl fmt::Display for ConstTy {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{:?}", self) // TODO(varkor)
+        write!(f, "{:?}", self) // TODO(const_generics)
     }
 }
 

src/libsyntax/parse/parser.rs

@@ -41,6 +41,7 @@ use ast::{Visibility, VisibilityKind, WhereClause, CrateSugar};
 use ast::{UseTree, UseTreeKind};
 use ast::{BinOpKind, UnOp};
 use ast::{RangeEnd, RangeSyntax};
+use ast::ParamKindOrd;
 use {ast, attr};
 use source_map::{self, SourceMap, Spanned, respan};
 use syntax_pos::{self, Span, MultiSpan, BytePos, FileName, edition::Edition};
@@ -5064,17 +5065,29 @@ impl<'a> Parser<'a> {
     /// Parses (possibly empty) list of lifetime, const, and type parameters, possibly including
     /// trailing comma and erroneous trailing attributes.
     crate fn parse_generic_params(&mut self) -> PResult<'a, Vec<ast::GenericParam>> {
-        let mut params = Vec::new();
-        let mut seen_non_lifetime_param = false;
+        let mut params = vec![];
+        let mut max_param = None;
+        fn enforce_param_order(parser: &Parser,
+                               max_param: &mut Option<ParamKindOrd>,
+                               param_ord: ParamKindOrd) {
+            match max_param {
+                Some(max_param) if *max_param > param_ord => {
+                    parser.span_err(parser.prev_span,
+                        &format!("{} parameters must be declared prior to {} parameters",
+                            param_ord, max_param));
+                }
+                Some(_) | None => *max_param = Some(param_ord),
+            }
+        }
         loop {
             let attrs = self.parse_outer_attributes()?;
             if self.check_lifetime() {
-                let lifetime = self.expect_lifetime();
                 // Parse lifetime parameter.
+                let lifetime = self.expect_lifetime();
                 let bounds = if self.eat(&token::Colon) {
                     self.parse_lt_param_bounds()
                 } else {
-                    Vec::new()
+                    vec![]
                 };
                 params.push(ast::GenericParam {
                     ident: lifetime.ident,
@@ -5083,28 +5096,25 @@ impl<'a> Parser<'a> {
                     bounds,
                     kind: ast::GenericParamKind::Lifetime,
                 });
-                if seen_non_lifetime_param {
-                    self.span_err(self.prev_span,
-                        "lifetime parameters must be declared prior to type and const parameters");
-                }
-            } else if self.check_const_param() {
-                // TODO(const_generics): should const parameters be forced to be after type parameters?
-                params.push(self.parse_const_param(attrs)?);
-                seen_non_lifetime_param = true;
+                enforce_param_order(&self, &mut max_param, ParamKindOrd::Lifetime);
             } else if self.check_ident() {
                 // Parse type parameter.
                 params.push(self.parse_ty_param(attrs)?);
-                seen_non_lifetime_param = true;
+                enforce_param_order(&self, &mut max_param, ParamKindOrd::Type);
+            } else if self.check_const_param() {
+                // Parse const parameter.
+                params.push(self.parse_const_param(attrs)?);
+                enforce_param_order(&self, &mut max_param, ParamKindOrd::Const);
             } else {
                 // Check for trailing attributes and stop parsing.
                 if !attrs.is_empty() {
-                    let param_kind = if seen_non_lifetime_param {
-                        "type and const"
+                    if let Some(max_param) = max_param {
+                        self.span_err(attrs[0].span,
+                            &format!("trailing attribute after {} parameters", max_param));
                     } else {
-                        "lifetime"
-                    };
-                    self.span_err(attrs[0].span,
-                        &format!("trailing attribute after {} parameters", param_kind));
+                        self.span_err(attrs[0].span,
+                            "leading attribute before generic parameters");
+                    }
                 }
                 break
             }