From: Philipp Gesang <[email protected]>
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 <[email protected]>
---
gcc/rust/parse/rust-parse-impl.hxx | 218 ++++++++++++++----
gcc/rust/parse/rust-parse.h | 5 +-
.../rust/compile/func-qualifier-default.rs | 5 +-
...ier-order.rs => func-qualifier-order-1.rs} | 0
.../rust/compile/func-qualifier-order-2.rs | 6 +
.../rust/compile/func-qualifier-order-3.rs | 14 ++
6 files changed, 195 insertions(+), 53 deletions(-)
rename gcc/testsuite/rust/compile/{func-qualifier-order.rs =>
func-qualifier-order-1.rs} (100%)
create mode 100644 gcc/testsuite/rust/compile/func-qualifier-order-2.rs
create mode 100644 gcc/testsuite/rust/compile/func-qualifier-order-3.rs
diff --git a/gcc/rust/parse/rust-parse-impl.hxx
b/gcc/rust/parse/rust-parse-impl.hxx
index 8820e3555e0..fc9f45a8e5c 100644
--- a/gcc/rust/parse/rust-parse-impl.hxx
+++ b/gcc/rust/parse/rust-parse-impl.hxx
@@ -1559,7 +1559,9 @@ 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 ();
+ auto qualifiers = parse_function_qualifiers ();
+ if (!qualifiers)
+ return nullptr;
skip_token (FN_KW);
@@ -1635,17 +1637,16 @@ Parser<ManagedTokenSource>::parse_function
(AST::Visibility vis,
body = std::move (block_expr.value ());
}
- return std::unique_ptr<AST::Function> (
- 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),
- std::move (outer_attrs), locus, is_external));
+ return std::unique_ptr<AST::Function> (new AST::Function (
+ std::move (function_name), std::move (*qualifiers.value ()),
+ std::move (generic_params), std::move (function_params),
+ std::move (return_type), std::move (where_clause), std::move (body),
+ std::move (vis), std::move (outer_attrs), locus, is_external));
}
// Parses function or method qualifiers (i.e. const, unsafe, and extern).
template <typename ManagedTokenSource>
-AST::FunctionQualifiers
+tl::expected<std::unique_ptr<AST::FunctionQualifiers>, Parse::Error::Node>
Parser<ManagedTokenSource>::parse_function_qualifiers ()
{
Default default_status = Default::No;
@@ -1655,51 +1656,165 @@ 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)
- {
+ return tl::unexpected<Parse::Error::Node> (
+ Parse::Error::Node::MALFORMED);
+ }
+
+ switch (token_id)
+ {
+ 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;
+ default:
+ goto done;
+ }
+ found_order.push_back (token_id);
lexer.skip_token ();
- unsafe_status = Unsafety::Unsafe;
}
+done:
- if (lexer.peek_token ()->get_id () == EXTERN_KW)
- {
- lexer.skip_token ();
- has_extern = true;
+ if (!ensure_function_qualifier_order (locus, std::move (found_order)))
+ return tl::unexpected<Parse::Error::Node> (Parse::Error::Node::MALFORMED);
- // detect optional abi name
- const_TokenPtr next_tok = lexer.peek_token ();
- if (next_tok->get_id () == STRING_LITERAL)
+ return std::unique_ptr<AST::FunctionQualifiers> (
+ new AST::FunctionQualifiers (locus, default_status, async_status,
+ const_status, unsafe_status, has_extern,
+ std::move (abi)));
+}
+
+template <typename ManagedTokenSource>
+bool
+Parser<ManagedTokenSource>::ensure_function_qualifier_order (
+ location_t locus, std::vector<TokenId> found_order)
+{
+ // Check in order of default, const, async, unsafe, extern
+ auto token_priority = [] (const TokenId id) {
+ switch (id)
+ {
+ 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;
+ default:
+ rust_unreachable ();
+ };
+ };
+
+ 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::ostringstream ss;
+
+ for (auto id : token_ids)
+ {
+ if (ss.tellp () != 0)
+ ss << ' ';
+
+ if (id == IDENTIFIER)
+ ss << Values::WeakKeywords::DEFAULT;
+ else
+ ss << token_id_keyword_string (id);
+ }
+
+ return ss.str ();
+ };
+
+ 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 false;
}
+
+ last_priority = priority;
}
- return AST::FunctionQualifiers (locus, default_status, async_status,
- const_status, unsafe_status, has_extern,
- std::move (abi));
+ return true;
}
// Parses generic (lifetime or type) params inside angle brackets (optional).
@@ -4276,7 +4391,9 @@
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 ();
+ auto qualifiers = parse_function_qualifiers ();
+ if (!qualifiers)
+ return nullptr;
skip_token (FN_KW);
@@ -4362,7 +4479,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.value ()),
std::move (generic_params), std::move (function_params),
std::move (return_type), std::move (where_clause),
std::move (body), std::move (vis),
@@ -4459,7 +4576,9 @@
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 ();
+ auto qualifiers = parse_function_qualifiers ();
+ if (!qualifiers)
+ return nullptr;
skip_token (FN_KW);
@@ -4590,7 +4709,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.value ()),
std::move (generic_params), std::move (function_params),
std::move (return_type), std::move (where_clause),
std::move (body), std::move (vis),
@@ -6202,7 +6321,9 @@ 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 ();
+ auto qualifiers = parse_function_qualifiers ();
+ if (!qualifiers)
+ return nullptr;
if (!skip_token (FN_KW))
return nullptr;
@@ -6285,11 +6406,10 @@ 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),
- is_variadic, std::move (variadic_attrs),
- std::move (return_type), best_try_locus));
+ return std::unique_ptr<AST::BareFunctionType> (new AST::BareFunctionType (
+ std::move (for_lifetimes), std::move (*qualifiers.value ()),
+ std::move (params), is_variadic, std::move (variadic_attrs),
+ std::move (return_type), best_try_locus));
}
template <typename ManagedTokenSource>
diff --git a/gcc/rust/parse/rust-parse.h b/gcc/rust/parse/rust-parse.h
index 96b2d091fc7..8df9904766a 100644
--- a/gcc/rust/parse/rust-parse.h
+++ b/gcc/rust/parse/rust-parse.h
@@ -364,7 +364,10 @@ private:
std::unique_ptr<AST::Function> parse_function (AST::Visibility vis,
AST::AttrVec outer_attrs,
bool is_external = false);
- AST::FunctionQualifiers parse_function_qualifiers ();
+ tl::expected<std::unique_ptr<AST::FunctionQualifiers>, Parse::Error::Node>
+ parse_function_qualifiers ();
+ bool ensure_function_qualifier_order (location_t locus,
+ std::vector<TokenId> found_order);
std::vector<std::unique_ptr<AST::GenericParam>>
parse_generic_params_in_angles ();
template <typename EndTokenPred>
diff --git a/gcc/testsuite/rust/compile/func-qualifier-default.rs
b/gcc/testsuite/rust/compile/func-qualifier-default.rs
index e2e600c4ba3..68be6b5aaa7 100644
--- a/gcc/testsuite/rust/compile/func-qualifier-default.rs
+++ b/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 }
}
diff --git a/gcc/testsuite/rust/compile/func-qualifier-order.rs
b/gcc/testsuite/rust/compile/func-qualifier-order-1.rs
similarity index 100%
rename from gcc/testsuite/rust/compile/func-qualifier-order.rs
rename to gcc/testsuite/rust/compile/func-qualifier-order-1.rs
diff --git a/gcc/testsuite/rust/compile/func-qualifier-order-2.rs
b/gcc/testsuite/rust/compile/func-qualifier-order-2.rs
new file mode 100644
index 00000000000..7eebe7df930
--- /dev/null
+++ b/gcc/testsuite/rust/compile/func-qualifier-order-2.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." }
+
diff --git a/gcc/testsuite/rust/compile/func-qualifier-order-3.rs
b/gcc/testsuite/rust/compile/func-qualifier-order-3.rs
new file mode 100644
index 00000000000..6e7e69c02c1
--- /dev/null
+++ b/gcc/testsuite/rust/compile/func-qualifier-order-3.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 }
+}
--
2.50.1
