gccrs: parse all function qualifiers before generating error

Instead of erroring out on the first misplaced qualifier, parse
as many qualifiers as possible to allow for more helpful error
message giving the correct order the qualifiers should be in.
Duplicate qualifiers are a kind of a special case and generate a
separate error message.

gcc/rust/ChangeLog
	* parse/rust-parse-impl.hxx (parse_function_qualifiers)
	Collect qualifiers into vector before generating the
	error message
	* parse/rust-parse.h: (parse_function_qualifiers) Make
	function fallible

gcc/testsuite/ChangeLog:
	* rust/compile/func-qualifier-default.rs:
	Adapt to change in compiler messages
	* rust/compile/func-qualifier-order.rs: Renamed existing
	test file for clarity (from)
	* rust/compile/func-qualifier-order-1.rs: Renamed existing
	test file (to)
	* rust/compile/func-qualifier-order-2.rs: New test file
	* rust/compile/func-qualifier-order-3.rs: New test file

Signed-off-by: Philipp Gesang <phg@phi-gamma.net>

Author: phi-gamma

gcc/rust/parse/rust-parse-impl.hxx

@@ -1559,7 +1559,10 @@ Parser<ManagedTokenSource>::parse_function (AST::Visibility vis,
 {
   location_t locus = lexer.peek_token ()->get_locus ();
   // Get qualifiers for function if they exist
-  AST::FunctionQualifiers qualifiers = parse_function_qualifiers ();
+  std::unique_ptr<AST::FunctionQualifiers> qualifiers
+    = parse_function_qualifiers ();
+  if (qualifiers == nullptr)
+    return nullptr;
 
   skip_token (FN_KW);
 
@@ -1636,7 +1639,7 @@ Parser<ManagedTokenSource>::parse_function (AST::Visibility vis,
     }
 
   return std::unique_ptr<AST::Function> (
-    new AST::Function (std::move (function_name), std::move (qualifiers),
+    new AST::Function (std::move (function_name), std::move (*qualifiers),
 		       std::move (generic_params), std::move (function_params),
 		       std::move (return_type), std::move (where_clause),
 		       std::move (body), std::move (vis),
@@ -1645,7 +1648,7 @@ Parser<ManagedTokenSource>::parse_function (AST::Visibility vis,
 
 // Parses function or method qualifiers (i.e. const, unsafe, and extern).
 template <typename ManagedTokenSource>
-AST::FunctionQualifiers
+std::unique_ptr<AST::FunctionQualifiers>
 Parser<ManagedTokenSource>::parse_function_qualifiers ()
 {
   Default default_status = Default::No;
@@ -1655,51 +1658,151 @@ Parser<ManagedTokenSource>::parse_function_qualifiers ()
   bool has_extern = false;
   std::string abi;
 
+  // collect all qualifiers before checking the order to allow for a better
+  // error message
+  std::vector<TokenId> found_order;
+
   const_TokenPtr t;
   location_t locus = lexer.peek_token ()->get_locus ();
-  // Check in order of default, const, async, unsafe, extern
-  if (lexer.peek_token ()->get_id () == IDENTIFIER
-       && lexer.peek_token ()->get_str () == Values::WeakKeywords::DEFAULT)
-    {
-      default_status = Default::Yes;
-      lexer.skip_token();
-    }
 
-  if (lexer.peek_token ()->get_id () == CONST)
+  // this will terminate on duplicates or the first non-qualifier token
+  while (true)
     {
-      lexer.skip_token ();
-      const_status = Const::Yes;
-    }
+      const TokenId token_id = lexer.peek_token ()->get_id ();
 
-  if (lexer.peek_token ()->get_id () == ASYNC)
-    {
-      lexer.skip_token ();
-      async_status = Async::Yes;
-     }
+      if (std::find (found_order.cbegin (), found_order.cend (), token_id)
+	  != found_order.cend ())
+	{
+	  // qualifiers mustn't appear twice
+	  Error error (lexer.peek_token ()->get_locus (),
+		       "encountered duplicate function qualifier %qs",
+		       lexer.peek_token ()->get_token_description ());
+	  add_error (std::move (error));
 
-  if (lexer.peek_token ()->get_id () == UNSAFE)
-    {
-      lexer.skip_token ();
-      unsafe_status = Unsafety::Unsafe;
-    }
+	  return nullptr;
+	}
 
-  if (lexer.peek_token ()->get_id () == EXTERN_KW)
-    {
+      switch (token_id)
+	{
+	default:
+	  goto done;
+	case IDENTIFIER:
+	  if (lexer.peek_token ()->get_str () != Values::WeakKeywords::DEFAULT)
+	    {
+	      // only "default" is valid in this context
+	      goto done;
+	    }
+	  default_status = Default::Yes;
+	  break;
+	case CONST:
+	  const_status = Const::Yes;
+	  break;
+	case ASYNC:
+	  async_status = Async::Yes;
+	  break;
+	case UNSAFE:
+	  unsafe_status = Unsafety::Unsafe;
+	  break;
+	case EXTERN_KW:
+	  has_extern = true;
+	  // detect optional abi name
+	  lexer.skip_token ();
+	  const_TokenPtr next_tok = lexer.peek_token ();
+	  if (next_tok->get_id () == STRING_LITERAL)
+	    {
+	      abi = next_tok->get_str ();
+	    }
+	  break;
+	}
+      found_order.push_back (token_id);
       lexer.skip_token ();
-      has_extern = true;
+    }
+done:
 
-      // detect optional abi name
-      const_TokenPtr next_tok = lexer.peek_token ();
-      if (next_tok->get_id () == STRING_LITERAL)
+  // Check in order of default, const, async, unsafe, extern
+  auto token_priority = [] (const TokenId id) {
+    switch (id)
+      {
+      default: // unreachable, but will cause an error
+	return 0;
+      case IDENTIFIER: // "default"; the only "weak" keyword considered here
+	return 1;
+      case CONST:
+	return 2;
+      case ASYNC:
+	return 3;
+      case UNSAFE:
+	return 4;
+      case EXTERN_KW:
+	return 5;
+      };
+  };
+
+  size_t last_priority = 0;
+  for (auto token_id : found_order)
+    {
+      const size_t priority = token_priority (token_id);
+      if (priority <= last_priority)
 	{
-	  lexer.skip_token ();
-	  abi = next_tok->get_str ();
+	  auto qualifiers_to_str = [] (const std::vector<TokenId> &token_ids) {
+	    std::string acc;
+
+	    for (auto id : token_ids)
+	      {
+		if (!acc.empty ())
+		  acc += ' ';
+
+		if (id == IDENTIFIER)
+		  acc += "default";
+		else
+		  acc += token_id_keyword_string (id);
+	      }
+
+	    return acc;
+	  };
+
+	  std::vector<TokenId> expected_order
+	    = {IDENTIFIER, CONST, ASYNC, UNSAFE, EXTERN_KW};
+
+	  // we only keep the qualifiers actually used in the offending code
+	  std::vector<TokenId>::const_iterator token_id
+	    = expected_order.cbegin ();
+	  while (token_id != expected_order.cend ())
+	    {
+	      if (std::find (found_order.cbegin (), found_order.cend (),
+			     *token_id)
+		  == found_order.cend ())
+		{
+		  token_id = expected_order.erase (token_id);
+		}
+	      else
+		{
+		  ++token_id;
+		}
+	    }
+
+	  const std::string found_qualifiers = qualifiers_to_str (found_order);
+	  const std::string expected_qualifiers
+	    = qualifiers_to_str (expected_order);
+
+	  location_t error_locus
+	    = make_location (locus, locus, lexer.peek_token ()->get_locus ());
+	  Error error (
+	    error_locus,
+	    "invalid order of function qualifiers; found %qs, expected %qs",
+	    found_qualifiers.c_str (), expected_qualifiers.c_str ());
+	  add_error (std::move (error));
+
+	  return nullptr;
 	}
+
+      last_priority = priority;
     }
 
-  return AST::FunctionQualifiers (locus, default_status, async_status,
-                                  const_status, unsafe_status, has_extern,
-                                  std::move (abi));
+  return std::unique_ptr<AST::FunctionQualifiers> (
+    new AST::FunctionQualifiers (locus, default_status, async_status,
+				 const_status, unsafe_status, has_extern,
+				 std::move (abi)));
 }
 
 // Parses generic (lifetime or type) params inside angle brackets (optional).
@@ -4276,7 +4379,10 @@ Parser<ManagedTokenSource>::parse_inherent_impl_function_or_method (
 {
   location_t locus = lexer.peek_token ()->get_locus ();
   // parse function or method qualifiers
-  AST::FunctionQualifiers qualifiers = parse_function_qualifiers ();
+  std::unique_ptr<AST::FunctionQualifiers> qualifiers
+    = parse_function_qualifiers ();
+  if (qualifiers == nullptr)
+    return nullptr;
 
   skip_token (FN_KW);
 
@@ -4362,7 +4468,7 @@ Parser<ManagedTokenSource>::parse_inherent_impl_function_or_method (
     }
 
   return std::unique_ptr<AST::Function> (
-    new AST::Function (std::move (ident), std::move (qualifiers),
+    new AST::Function (std::move (ident), std::move (*qualifiers),
 		       std::move (generic_params), std::move (function_params),
 		       std::move (return_type), std::move (where_clause),
 		       std::move (body), std::move (vis),
@@ -4459,7 +4565,10 @@ Parser<ManagedTokenSource>::parse_trait_impl_function_or_method (
   location_t locus = lexer.peek_token ()->get_locus ();
 
   // parse function or method qualifiers
-  AST::FunctionQualifiers qualifiers = parse_function_qualifiers ();
+  std::unique_ptr<AST::FunctionQualifiers> qualifiers
+    = parse_function_qualifiers ();
+  if (qualifiers == nullptr)
+    return nullptr;
 
   skip_token (FN_KW);
 
@@ -4590,7 +4699,7 @@ Parser<ManagedTokenSource>::parse_trait_impl_function_or_method (
     }
 
   return std::unique_ptr<AST::Function> (
-    new AST::Function (std::move (ident), std::move (qualifiers),
+    new AST::Function (std::move (ident), std::move (*qualifiers),
 		       std::move (generic_params), std::move (function_params),
 		       std::move (return_type), std::move (where_clause),
 		       std::move (body), std::move (vis),
@@ -6202,7 +6311,10 @@ Parser<ManagedTokenSource>::parse_bare_function_type (
   // TODO: pass in for lifetime location as param
   location_t best_try_locus = lexer.peek_token ()->get_locus ();
 
-  AST::FunctionQualifiers qualifiers = parse_function_qualifiers ();
+  std::unique_ptr<AST::FunctionQualifiers> qualifiers
+    = parse_function_qualifiers ();
+  if (qualifiers == nullptr)
+    return nullptr;
 
   if (!skip_token (FN_KW))
     return nullptr;
@@ -6287,7 +6399,7 @@ Parser<ManagedTokenSource>::parse_bare_function_type (
 
   return std::unique_ptr<AST::BareFunctionType> (
     new AST::BareFunctionType (std::move (for_lifetimes),
-			       std::move (qualifiers), std::move (params),
+			       std::move (*qualifiers), std::move (params),
 			       is_variadic, std::move (variadic_attrs),
 			       std::move (return_type), best_try_locus));
 }

gcc/rust/parse/rust-parse.h

@@ -364,7 +364,7 @@ template <typename ManagedTokenSource> class Parser
   std::unique_ptr<AST::Function> parse_function (AST::Visibility vis,
 						 AST::AttrVec outer_attrs,
 						 bool is_external = false);
-  AST::FunctionQualifiers parse_function_qualifiers ();
+  std::unique_ptr<AST::FunctionQualifiers> parse_function_qualifiers ();
   std::vector<std::unique_ptr<AST::GenericParam>>
   parse_generic_params_in_angles ();
   template <typename EndTokenPred>

gcc/testsuite/rust/compile/func-qualifier-default.rs

@@ -12,7 +12,6 @@ impl<T> Trait for T {
 
     // `default` precedes all other qualifiers
     unsafe default fn unsafe_fn() {}
-    // { dg-error "expecting .fn. but .identifier. found" "" { target *-*-* } .-1 }
-    // { dg-error "expecting ... but .fn. found" "" { target *-*-* } .-2 }
-    // { dg-error "failed to parse trait impl item in trait impl" "" { target *-*-* } .-3 }
+    // { dg-error "invalid order of function qualifiers; found .unsafe default., expected .default unsafe." "" { target *-*-* } .-1 }
+    // { dg-error "failed to parse trait impl item in trait impl" "" { target *-*-* } .-2 }
 }

…ite/rust/compile/func-qualifier-order.rs …e/rust/compile/func-qualifier-order-1.rsgcc/testsuite/rust/compile/func-qualifier-order.rs renamed to gcc/testsuite/rust/compile/func-qualifier-order-1.rs gcc/testsuite/rust/compile/func-qualifier-order.rs renamed to gcc/testsuite/rust/compile/func-qualifier-order-1.rs

@@ -0,0 +1,6 @@
+// { dg-additional-options "-frust-edition=2018" }
+#![feature(no_core)]
+#![no_core]
+
+async unsafe async fn duplicate_qualifier() {} // { dg-error "encountered duplicate function qualifier .async." }
+

gcc/testsuite/rust/compile/func-qualifier-order-2.rs

@@ -0,0 +1,14 @@
+#![feature(min_specialization)]
+#![feature(no_core)]
+#![no_core]
+
+trait Dummy {
+}
+
+// the purpose of this is to trigger the compiler message
+// regarding the order of qualifiers
+impl<T> Dummy for T {
+    default async unsafe extern "C" const fn all_the_qualifiers() {}
+    // { dg-error "invalid order of function qualifiers; found .default async unsafe extern const., expected .default const async unsafe extern." "" { target *-*-* } .-1 }
+    // { dg-error "failed to parse trait impl item in trait impl" "" { target *-*-* } .-2 }
+}