Skip to content

enforce function qualifier order#4463

Open
phi-gamma wants to merge 5 commits intoRust-GCC:masterfrom
phi-gamma:phg/keyword-order
Open

enforce function qualifier order#4463
phi-gamma wants to merge 5 commits intoRust-GCC:masterfrom
phi-gamma:phg/keyword-order

Conversation

@phi-gamma
Copy link

@phi-gamma phi-gamma commented Mar 5, 2026 •
edited
Loading

Fixes #2819 "enforce rust keyword order", building on top of PR #2872
which got closed due to inactivity.

I made the function qualifier parsing in Parser<>::parse_function_qualifiers
a little more involved to provide a little more constructive feedback
for the user. It will now try and read as many qualifier tokens as
possible (or until it hits a duplicate) so as to respond with them in
the correct order.

Example output:

/home/phg/src/rust/gccrs/gcc/testsuite/rust/compile/func-qualifier-order-3.rs:11:5: error: invalid order of function qualifiers; found ‘default async unsafe extern const’, expected ‘default const async unsafe extern’
   11 |     default async unsafe extern "C" const fn all_the_qualifiers() {}
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Special case for duplicate qualifiers:

/home/phg/src/rust/gccrs/gcc/testsuite/rust/compile/func-qualifier-order-2.rs:5:14: error: encountered duplicate function qualifier ‘async’
    5 | async unsafe async fn duplicate_qualifier() {}
      |              ^~~~~

Note that I preserved the two commits by @braw-lee in #2872 for their
work to get credited correctly; the commits needed some adapting to
the current state of gccrs.

  • [+] GCC development requires copyright assignment or the Developer's Certificate of Origin sign-off, see https://gcc.gnu.org/contribute.html or https://gcc.gnu.org/dco.html
  • [+] Read contributing guidelines
  • [+] make check-rust passes locally
  • [+] Run clang-format
  • [+] Added any relevant test cases to gcc/testsuite/rust/

@phi-gamma phi-gamma force-pushed the phg/keyword-order branch 10 times, most recently from 4fdee39 to b87cd54 Compare March 9, 2026 06:52
P-E-P
P-E-P reviewed Mar 11, 2026
View reviewed changes
gcc/rust/parse/rust-parse-impl.hxx Outdated
{
lexer.skip_token ();
abi = next_tok->get_str ();
auto qualifiers_to_str = [] (const std::vector<TokenId> &token_ids) {
Copy link
Member

@P-E-P P-E-P Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole found vs expected tokens list building code should be in a separate function.

Copy link
Author

@phi-gamma phi-gamma Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please have a look at the refactor commit that I put on top. It splits up that monster function into a number of smaller ones so parsing, validation, and extracting the qualifier flags are separate steps.

It's a little more invasive hence the extra commit. I'll squash that guy if it passes QA.

@phi-gamma phi-gamma force-pushed the phg/keyword-order branch 2 times, most recently from b0cf069 to 0b8e385 Compare March 12, 2026 09:01
braw-lee and others added 4 commits March 12, 2026 10:01
@braw-lee @phi-gamma
Add default as function qualifier.
c0e1924 
gcc/rust/ChangeLog:

	* ast/rust-ast-builder.cc (AstBuilder::fn_qualifiers):
	Add new parameter.
	* ast/rust-ast-builder.h: Likewise.
	* ast/rust-ast.cc (Function::Function):
	Remove current default parameter.
	(Function::operator=):
	Remove current default parameter.
	* ast/rust-item.h (class FunctionQualifiers):
	Add data member to represent defaultness of a function.
	(class Function):
	Remove current default parameter.
	* parse/rust-parse-impl.hxx (Parser::parse_function_qualifiers):
	Parse function qualifiers in order.
	* util/rust-common.h (enum class): Create enum to represent
	defaultness of a function.

Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
@braw-lee @phi-gamma
Reject default functions after parsing.
1e11f9d 
Functions with `default` qualifier outside of `impl` blocks should be
allowed to parse successfully and are later rejected during the
ASTValidation pass.

gcc/rust/ChangeLog:

	* checks/errors/rust-ast-validation.cc (ASTValidation::visit):
	Add error for functions with `default` qualifier outside of impl
	blocks.
	* parse/rust-parse-impl.hxx (Parser::parse_item):
	Allow parsing of functions with `default` qualifier.
	(Parser::parse_vis_item): Likewise.

Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
@phi-gamma
gccrs: add test cases for fn qualifiers, default keyword
ba25b99 
gcc/testsuite/ChangeLog:

	* rust/compile/func-qualifier-order.rs: New test file
	* rust/compile/func-qualifier-default.rs: New test file

Signed-off-by: Philipp Gesang <phg@phi-gamma.net>
@phi-gamma
gccrs: parse all function qualifiers before generating error
a4602f3 
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>
@phi-gamma phi-gamma force-pushed the phg/keyword-order branch 4 times, most recently from 8cf550a to 2677395 Compare March 13, 2026 15:47
@phi-gamma
gccrs: refactor function qualifier parsing
0c71d65 
gcc/rust/ChangeLog
	* parse/rust-parse-impl.hxx: Refactor qualifier parsing
	* parse/rust-parse.h: (parse_function_qualifiers) Likewise
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@P-E-P P-E-P P-E-P left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enforce rust keyword order

3 participants

@phi-gamma @P-E-P @braw-lee