Merge #1312

1312: Refactor generic parameter parsing and report order errors r=CohenArthur a=CohenArthur

Closes #1311

This allows us to parse lifetimes and types (and later const generics)
in any order without necessarily erroring out for the wrong reason. It also simplifies the
code greatly and makes it easier to modify.

This also removes an unused duplicate `parse_generic_params` function

Co-authored-by: Arthur Cohen <arthur.cohen@embecosm.com>

5 files changed, 106 insertions, 315 deletions

diff --git a/gcc/rust/ast/rust-ast.h b/gcc/rust/ast/rust-ast.h
index 8f5657f..2d7d31a 100644
--- a/gcc/rust/ast/rust-ast.h
+++ b/gcc/rust/ast/rust-ast.h
@@ -1255,6 +1255,13 @@ protected:
 class GenericParam
 {
 public:
+  enum class Kind
+  {
+    Lifetime,
+    Type,
+    Const,
+  };
+
   virtual ~GenericParam () {}
 
   // Unique pointer custom clone function
@@ -1269,6 +1276,8 @@ public:
 
   virtual Location get_locus () const = 0;
 
+  virtual Kind get_kind () const = 0;
+
   NodeId get_node_id () { return node_id; }
 
 protected:
@@ -1322,6 +1331,8 @@ public:
 
   Location get_locus () const override final { return locus; }
 
+  Kind get_kind () const override final { return Kind::Lifetime; }
+
 protected:
   /* Use covariance to implement clone function as returning this object rather
    * than base */
diff --git a/gcc/rust/ast/rust-item.h b/gcc/rust/ast/rust-item.h
index 6d953fb..94ffffb 100644
--- a/gcc/rust/ast/rust-item.h
+++ b/gcc/rust/ast/rust-item.h
@@ -132,6 +132,8 @@ public:
 
   Location get_locus () const override final { return locus; }
 
+  Kind get_kind () const override final { return Kind::Type; }
+
   void accept_vis (ASTVisitor &vis) override;
 
   // TODO: is this better? Or is a "vis_block" better?
diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h
index 0fc1c1f..ce68c16 100644
--- a/gcc/rust/parse/rust-parse-impl.h
+++ b/gcc/rust/parse/rust-parse-impl.h
@@ -2793,197 +2793,87 @@ Parser<ManagedTokenSource>::parse_generic_params_in_angles ()
   return generic_params;
 }
 
-/* Parse generic (lifetime or type) params NOT INSIDE ANGLE BRACKETS!!! Almost
- * always parse_generic_params_in_angles is what is wanted. */
 template <typename ManagedTokenSource>
-std::vector<std::unique_ptr<AST::GenericParam>>
-Parser<ManagedTokenSource>::parse_generic_params ()
+template <typename EndTokenPred>
+std::unique_ptr<AST::GenericParam>
+Parser<ManagedTokenSource>::parse_generic_param (EndTokenPred is_end_token)
 {
-  std::vector<std::unique_ptr<AST::GenericParam>> generic_params;
-
-  // can't parse lifetime and type params separately due to lookahead issues
-  // thus, parse them all here
+  auto token = lexer.peek_token ();
+  auto outer_attrs = parse_outer_attribute ();
+  std::unique_ptr<AST::GenericParam> param;
 
-  // DEBUG
-  rust_debug ("starting to parse generic params (inside angle brackets)");
-
-  /* HACK: used to retain attribute data if a lifetime param is tentatively
-   * parsed but it turns out to be type param */
-  AST::Attribute parsed_outer_attr = AST::Attribute::create_empty ();
-
-  /* HACK: generic params always in angle brackets with current syntax, so have
-   * that as end char */
-  const_TokenPtr t = lexer.peek_token ();
-  // parse lifetime params
-  while (!is_right_angle_tok (t->get_id ()))
-    {
-      // HACK: reimpl of lifetime param parsing
-      AST::Attribute outer_attr = parse_outer_attribute ();
-
-      // move attribute outward if type param
-      if (lexer.peek_token ()->get_id () != LIFETIME)
-	{
-	  parsed_outer_attr = std::move (outer_attr);
-
-	  // DEBUG
-	  rust_debug (
-	    "broke from parsing lifetime params as next token isn't lifetime - "
-	    "saved attribute");
-
-	  break;
-	}
-
-      Location locus = lexer.peek_token ()->get_locus ();
-      AST::Lifetime lifetime = parse_lifetime ();
-
-      // DEBUG
-      rust_debug ("parsed lifetime in lifetime params");
-
-      // parse optional bounds
-      std::vector<AST::Lifetime> lifetime_bounds;
-      if (lexer.peek_token ()->get_id () == COLON)
-	{
-	  lexer.skip_token ();
-	  // parse required bounds
-	  lifetime_bounds = parse_lifetime_bounds (
-	    [] (TokenId id) { return is_right_angle_tok (id) || id == COMMA; });
-	}
-
-      std::unique_ptr<AST::LifetimeParam> param (
-	new AST::LifetimeParam (std::move (lifetime),
-				std::move (lifetime_bounds),
-				std::move (outer_attr), locus));
-      generic_params.push_back (std::move (param));
-
-      if (lexer.peek_token ()->get_id () != COMMA)
-	{
-	  break;
-	}
-      lexer.skip_token ();
-
-      t = lexer.peek_token ();
-    }
-
-  // parse type params (reimpl required for first one but not others)
-  if (!is_right_angle_tok (lexer.peek_token ()->get_id ())
-      && !parsed_outer_attr.is_empty ())
+  switch (token->get_id ())
     {
-      // DEBUG
-      rust_debug ("as parsed outer attr isn't empty, started parsing type "
-		  "param reimpl");
-
-      // reimpl as type param definitely exists
-      const_TokenPtr ident_tok = expect_token (IDENTIFIER);
-      if (ident_tok == nullptr)
-	{
-	  Error error (
-	    lexer.peek_token ()->get_locus (),
-	    "failed to parse identifier in type param in generic params");
-	  add_error (std::move (error));
-
-	  return std::vector<std::unique_ptr<AST::GenericParam>> ();
-	}
-      Identifier ident = ident_tok->get_str ();
-
-      // parse optional bounds
-      std::vector<std::unique_ptr<AST::TypeParamBound>> type_param_bounds;
-      if (lexer.peek_token ()->get_id () == COLON)
-	{
-	  lexer.skip_token ();
-
-	  // parse optional type param bounds
-	  type_param_bounds = parse_type_param_bounds ();
-	}
-
-      // parse optional type
-      std::unique_ptr<AST::Type> type = nullptr;
-      if (lexer.peek_token ()->get_id () == EQUAL)
-	{
-	  lexer.skip_token ();
-
-	  // parse required type
-	  type = parse_type ();
-	  if (type == nullptr)
-	    {
-	      Error error (
-		lexer.peek_token ()->get_locus (),
-		"failed to parse type in type param in generic params");
-	      add_error (std::move (error));
-
-	      return std::vector<std::unique_ptr<AST::GenericParam>> ();
-	    }
-	}
-
-      std::unique_ptr<AST::TypeParam> param (
-	new AST::TypeParam (std::move (ident), ident_tok->get_locus (),
-			    std::move (type_param_bounds), std::move (type),
-			    std::move (parsed_outer_attr)));
-      generic_params.push_back (std::move (param));
-
-      // handle comma
-      if (lexer.peek_token ()->get_id () == COMMA)
-	{
-	  lexer.skip_token ();
-	}
-    }
-
-  // DEBUG
-  rust_debug (
-    "about to start parsing normally-parsed type params in generic params");
+      case LIFETIME: {
+	auto lifetime = parse_lifetime ();
+	if (lifetime.is_error ())
+	  {
+	    rust_error_at (
+	      token->get_locus (),
+	      "failed to parse lifetime in generic parameter list");
+	    return nullptr;
+	  }
 
-  // parse rest of type params - reimpl due to right angle tokens
-  t = lexer.peek_token ();
-  while (!is_right_angle_tok (t->get_id ()))
-    {
-      std::unique_ptr<AST::TypeParam> type_param = parse_type_param ();
+	std::vector<AST::Lifetime> lifetime_bounds;
+	if (lexer.peek_token ()->get_id () == COLON)
+	  {
+	    lexer.skip_token ();
+	    // parse required bounds
+	    lifetime_bounds
+	      = parse_lifetime_bounds ([is_end_token] (TokenId id) {
+		  return is_end_token (id) || id == COMMA;
+		});
+	  }
 
-      if (type_param == nullptr)
-	{
-	  Error error (lexer.peek_token ()->get_locus (),
-		       "failed to parse type param in generic params");
-	  add_error (std::move (error));
+	param = std::unique_ptr<AST::LifetimeParam> (new AST::LifetimeParam (
+	  std::move (lifetime), std::move (lifetime_bounds),
+	  std::move (outer_attrs), token->get_locus ()));
+	break;
+      }
+      case IDENTIFIER: {
+	auto type_ident = token->get_str ();
+	lexer.skip_token ();
 
-	  return std::vector<std::unique_ptr<AST::GenericParam>> ();
-	}
+	std::vector<std::unique_ptr<AST::TypeParamBound>> type_param_bounds;
+	if (lexer.peek_token ()->get_id () == COLON)
+	  {
+	    lexer.skip_token ();
 
-      // DEBUG
-      rust_debug ("successfully parsed type param");
+	    // parse optional type param bounds
+	    type_param_bounds = parse_type_param_bounds ();
+	  }
 
-      generic_params.push_back (std::move (type_param));
+	std::unique_ptr<AST::Type> type = nullptr;
+	if (lexer.peek_token ()->get_id () == EQUAL)
+	  {
+	    lexer.skip_token ();
 
-      if (lexer.peek_token ()->get_id () != COMMA)
-	{
-	  break;
-	}
-      // skip commas, including trailing commas
-      lexer.skip_token ();
+	    // parse required type
+	    type = parse_type ();
+	    if (!type)
+	      {
+		rust_error_at (
+		  lexer.peek_token ()->get_locus (),
+		  "failed to parse type in type param in generic params");
+		return nullptr;
+	      }
+	  }
 
-      t = lexer.peek_token ();
+	param = std::unique_ptr<AST::TypeParam> (
+	  new AST::TypeParam (std::move (type_ident), token->get_locus (),
+			      std::move (type_param_bounds), std::move (type),
+			      std::move (outer_attrs)));
+	break;
+      }
+    default:
+      // FIXME: Can we clean this last call with a method call?
+      rust_error_at (token->get_locus (),
+		     "unexpected token when parsing generic parameters: %qs",
+		     token->get_str ().c_str ());
+      return nullptr;
     }
 
-  // old code
-  /*
-  // parse lifetime params (optional), allowed to end with a trailing comma
-  std::vector<std::unique_ptr<AST::LifetimeParam>> lifetime_params
-    = parse_lifetime_params();
-  if (!lifetime_params.empty()) {
-      // C++11 code:
-      generic_params.insert(generic_params.end(),
-	std::make_move_iterator(lifetime_params.begin()),
-	std::make_move_iterator(lifetime_params.end()));
-  }
-
-  // parse type params (optional)
-  std::vector<std::unique_ptr<AST::TypeParam>> type_params =
-  parse_type_params(); if (!type_params.empty()) {
-      // C++11 code:
-      generic_params.insert(generic_params.end(),
-	std::make_move_iterator(type_params.begin()),
-	std::make_move_iterator(type_params.end()));
-  }*/
-
-  generic_params.shrink_to_fit ();
-  return generic_params;
+  return param;
 }
 
 /* Parse generic (lifetime or type) params NOT INSIDE ANGLE BRACKETS!!! Almost
@@ -3002,149 +2892,34 @@ Parser<ManagedTokenSource>::parse_generic_params (EndTokenPred is_end_token)
    * parsed but it turns out to be type param */
   AST::Attribute parsed_outer_attr = AST::Attribute::create_empty ();
 
-  const_TokenPtr t = lexer.peek_token ();
-  // parse lifetime params
-  while (!is_end_token (t->get_id ()))
-    {
-      // HACK: reimpl of lifetime param parsing
-      AST::Attribute outer_attr = parse_outer_attribute ();
-
-      // move attribute outward if type param
-      if (lexer.peek_token ()->get_id () != LIFETIME)
-	{
-	  parsed_outer_attr = std::move (outer_attr);
-	  break;
-	}
-
-      Location locus = lexer.peek_token ()->get_locus ();
-      AST::Lifetime lifetime = parse_lifetime ();
-
-      // parse optional bounds
-      std::vector<AST::Lifetime> lifetime_bounds;
-      if (lexer.peek_token ()->get_id () == COLON)
-	{
-	  lexer.skip_token ();
-	  // parse required bounds
-	  lifetime_bounds = parse_lifetime_bounds ([is_end_token] (TokenId id) {
-	    return is_end_token (id) || id == COMMA;
-	  });
-	}
-
-      std::unique_ptr<AST::LifetimeParam> param (
-	new AST::LifetimeParam (std::move (lifetime),
-				std::move (lifetime_bounds),
-				std::move (outer_attr), locus));
-      generic_params.push_back (std::move (param));
-
-      if (lexer.peek_token ()->get_id () != COMMA)
-	break;
-
-      lexer.skip_token ();
-      t = lexer.peek_token ();
-    }
+  // Did we parse a generic type param yet
+  auto type_seen = false;
+  // Did the user write a lifetime parameter after a type one
+  auto order_error = false;
 
-  // parse type params (reimpl required for first one but not others)
-  if (!is_end_token (lexer.peek_token ()->get_id ())
-      && !parsed_outer_attr.is_empty ())
-    {
-      // reimpl as type param definitely exists
-      const_TokenPtr ident_tok = expect_token (IDENTIFIER);
-      if (ident_tok == nullptr)
-	{
-	  Error error (
-	    lexer.peek_token ()->get_locus (),
-	    "failed to parse identifier in type param in generic params");
-	  add_error (std::move (error));
-
-	  return {};
-	}
-      Identifier ident = ident_tok->get_str ();
-
-      // parse optional bounds
-      std::vector<std::unique_ptr<AST::TypeParamBound>> type_param_bounds;
-      if (lexer.peek_token ()->get_id () == COLON)
-	{
-	  lexer.skip_token ();
-
-	  // parse optional type param bounds
-	  type_param_bounds = parse_type_param_bounds ();
-	}
-
-      // parse optional type
-      std::unique_ptr<AST::Type> type = nullptr;
-      if (lexer.peek_token ()->get_id () == EQUAL)
-	{
-	  lexer.skip_token ();
-
-	  // parse required type
-	  type = parse_type ();
-	  if (type == nullptr)
-	    {
-	      Error error (
-		lexer.peek_token ()->get_locus (),
-		"failed to parse type in type param in generic params");
-	      add_error (std::move (error));
-
-	      return {};
-	    }
-	}
-
-      std::unique_ptr<AST::TypeParam> param (
-	new AST::TypeParam (std::move (ident), ident_tok->get_locus (),
-			    std::move (type_param_bounds), std::move (type),
-			    std::move (parsed_outer_attr)));
-      generic_params.push_back (std::move (param));
-
-      // handle comma
-      if (lexer.peek_token ()->get_id () == COMMA)
-	lexer.skip_token ();
-    }
-
-  // parse rest of type params - reimpl due to right angle tokens
-  t = lexer.peek_token ();
-  while (!is_end_token (t->get_id ()))
+  // parse lifetime params
+  while (!is_end_token (lexer.peek_token ()->get_id ()))
     {
-      std::unique_ptr<AST::TypeParam> type_param = parse_type_param ();
-
-      if (type_param == nullptr)
+      auto param = parse_generic_param (is_end_token);
+      if (param)
 	{
-	  Error error (lexer.peek_token ()->get_locus (),
-		       "failed to parse type param in generic params");
-	  add_error (std::move (error));
+	  // TODO: Handle `Const` here as well if necessary
+	  if (param->get_kind () == AST::GenericParam::Kind::Type)
+	    type_seen = true;
+	  else if (param->get_kind () == AST::GenericParam::Kind::Lifetime
+		   && type_seen)
+	    order_error = true;
 
-	  return {};
+	  generic_params.emplace_back (std::move (param));
+	  maybe_skip_token (COMMA);
 	}
-
-      generic_params.push_back (std::move (type_param));
-
-      if (lexer.peek_token ()->get_id () != COMMA)
-	break;
-
-      // skip commas, including trailing commas
-      lexer.skip_token ();
-      t = lexer.peek_token ();
     }
 
-  // old code
-  /*
-  // parse lifetime params (optional), allowed to end with a trailing comma
-  std::vector<std::unique_ptr<AST::LifetimeParam>> lifetime_params
-    = parse_lifetime_params();
-  if (!lifetime_params.empty()) {
-      // C++11 code:
-      generic_params.insert(generic_params.end(),
-	std::make_move_iterator(lifetime_params.begin()),
-	std::make_move_iterator(lifetime_params.end()));
-  }
-
-  // parse type params (optional)
-  std::vector<std::unique_ptr<AST::TypeParam>> type_params =
-  parse_type_params(); if (!type_params.empty()) {
-      // C++11 code:
-      generic_params.insert(generic_params.end(),
-	std::make_move_iterator(type_params.begin()),
-	std::make_move_iterator(type_params.end()));
-  }*/
+  // FIXME: Add reordering hint
+  if (order_error)
+    rust_error_at (generic_params.front ()->get_locus (),
+		   "invalid order for generic parameters: lifetimes should "
+		   "always come before types");
 
   generic_params.shrink_to_fit ();
   return generic_params;
diff --git a/gcc/rust/parse/rust-parse.h b/gcc/rust/parse/rust-parse.h
index fcfcf18..d19bc71 100644
--- a/gcc/rust/parse/rust-parse.h
+++ b/gcc/rust/parse/rust-parse.h
@@ -223,10 +223,12 @@ private:
   AST::FunctionQualifiers parse_function_qualifiers ();
   std::vector<std::unique_ptr<AST::GenericParam> >
   parse_generic_params_in_angles ();
-  std::vector<std::unique_ptr<AST::GenericParam> > parse_generic_params ();
   template <typename EndTokenPred>
   std::vector<std::unique_ptr<AST::GenericParam> >
   parse_generic_params (EndTokenPred is_end_token);
+  template <typename EndTokenPred>
+  std::unique_ptr<AST::GenericParam>
+  parse_generic_param (EndTokenPred is_end_token);
 
   template <typename EndTokenPred>
   std::vector<std::unique_ptr<AST::LifetimeParam> >
diff --git a/gcc/testsuite/rust/compile/generics13.rs b/gcc/testsuite/rust/compile/generics13.rs
new file mode 100644
index 0000000..05c75c5
--- /dev/null
+++ b/gcc/testsuite/rust/compile/generics13.rs
@@ -0,0 +1 @@
+struct Foo<A, 'a>; // { dg-error "invalid order for generic parameters: lifetimes should always come before types" }