Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit error on async trait functions #2767

Closed
P-E-P opened this issue Dec 1, 2023 · 8 comments
Closed

Emit error on async trait functions #2767

P-E-P opened this issue Dec 1, 2023 · 8 comments
Assignees

Comments

@P-E-P
Copy link
Member

P-E-P commented Dec 1, 2023

          you can add the same check for `async` functions, even if that is about to change soon.

Originally posted by @CohenArthur in #2753 (comment)

error[E0706]: functions in traits cannot be declared `async`
 --> src/lib.rs:2:5
  |
2 |     async fn titi();
  |     -----^^^^^^^^^^^
  |     |
  |     `async` because of this
  |
  = note: `async` trait functions are not currently supported
  = note: consider using the `async-trait` crate: https://crates.io/crates/async-trait
  = note: see issue #91611 <https://github.com/rust-lang/rust/issues/91611> for more information
@braw-lee
Copy link
Contributor

I want to fix this

@P-E-P P-E-P added this to the GCC 14.1 release milestone Dec 13, 2023
@braw-lee
Copy link
Contributor

I added a check and test for async function inside a trait but gccrs fails to recognize async as a token inside trait impl
test.rs:6:5: error: unrecognised token ‘async’ for item in trait impl

6 | async fn f() -> u32 {
| ^~~~~
test.rs:6:5: error: failed to parse trait impl item in trait impl
test.rs:6:5: error: failed to parse item in crate

@braw-lee
Copy link
Contributor

is async token not fully supported yet? I was trying to step through the lexer with gdb to find out but for some reason it skips over some code even with -O0 flag enabled

@P-E-P
Copy link
Member Author

P-E-P commented Dec 14, 2023

is async token not fully supported yet? I was trying to step through the lexer with gdb to find out but for some reason it skips over some code even with -O0 flag enabled

AFAIK async token is supported by the lexer, we already have multiple async nodes in our ast. For example the parse_async_item function make use of it indirectly.

@braw-lee
Copy link
Contributor

yes, looks like async is supported

this makes me think there is an unhandled case for async token when we are inside a trait
I will look into it in a few days

braw-lee added a commit to braw-lee/gccrs that referenced this issue Dec 16, 2023
Fixes Rust-GCC#2767

gcc/rust/ChangeLog:

	* checks/errors/rust-ast-validation.cc (ASTValidation::visit):
	Added check for `async` function inside trait.

gcc/testsuite/ChangeLog:

	* rust/compile/issue-2767.rs: New test.

Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
braw-lee added a commit to braw-lee/gccrs that referenced this issue Dec 27, 2023
Fixes Rust-GCC#2767

gcc/rust/ChangeLog:

	* checks/errors/rust-ast-validation.cc (ASTValidation::visit):
	Added check for `async` function inside trait.

gcc/testsuite/ChangeLog:

	* rust/compile/issue-2767.rs: New test.

Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
braw-lee added a commit to braw-lee/gccrs that referenced this issue Dec 27, 2023
Fixes Rust-GCC#2767

gcc/rust/ChangeLog:

	* checks/errors/rust-ast-validation.cc (ASTValidation::visit):
	Added check for `async` function inside trait.

gcc/testsuite/ChangeLog:

	* rust/compile/issue-2767.rs: New test.

Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
braw-lee added a commit to braw-lee/gccrs that referenced this issue Dec 27, 2023
Fixes Rust-GCC#2767

gcc/rust/ChangeLog:

	* checks/errors/rust-ast-validation.cc (ASTValidation::visit):
	Added check for `async` function inside trait.

gcc/testsuite/ChangeLog:

	* rust/compile/issue-2767.rs: New test.

Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
braw-lee added a commit to braw-lee/gccrs that referenced this issue Dec 27, 2023
Fixes Rust-GCC#2767

gcc/rust/ChangeLog:

	* checks/errors/rust-ast-validation.cc (ASTValidation::visit):
	Added check for `async` function inside trait.

gcc/testsuite/ChangeLog:

	* rust/compile/issue-2767.rs: New test.

Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
github-merge-queue bot pushed a commit that referenced this issue Dec 27, 2023
Fixes #2767

gcc/rust/ChangeLog:

	* checks/errors/rust-ast-validation.cc (ASTValidation::visit):
	Added check for `async` function inside trait.

gcc/testsuite/ChangeLog:

	* rust/compile/issue-2767.rs: New test.

Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
CohenArthur pushed a commit to CohenArthur/gccrs that referenced this issue Jan 17, 2024
Fixes Rust-GCC#2767

gcc/rust/ChangeLog:

	* checks/errors/rust-ast-validation.cc (ASTValidation::visit):
	Added check for `async` function inside trait.

gcc/testsuite/ChangeLog:

	* rust/compile/issue-2767.rs: New test.

Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
CohenArthur pushed a commit to CohenArthur/gccrs that referenced this issue Jan 26, 2024
Fixes Rust-GCC#2767

gcc/rust/ChangeLog:

	* checks/errors/rust-ast-validation.cc (ASTValidation::visit):
	Added check for `async` function inside trait.

gcc/testsuite/ChangeLog:

	* rust/compile/issue-2767.rs: New test.

Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
CohenArthur pushed a commit to CohenArthur/gccrs that referenced this issue Jan 26, 2024
Fixes Rust-GCC#2767

gcc/rust/ChangeLog:

	* checks/errors/rust-ast-validation.cc (ASTValidation::visit):
	Added check for `async` function inside trait.

gcc/testsuite/ChangeLog:

	* rust/compile/issue-2767.rs: New test.

Signed-off-by: Kushal Pal <kushalpal109@gmail.com>
@tschwinge
Copy link
Member

@P-E-P, @braw-lee, @CohenArthur, is it correct that this issue got closed via #2944 "Merge upstream, 2024-03-09" (why wasn't it then already closed via a GCC/Rust master branch commit?), or remains there work to be done here?

@braw-lee
Copy link
Contributor

#2779
the second commit of this PR fixed this issue

@braw-lee
Copy link
Contributor

I think it wasn't closed because I didn't mention the issue #2767 in the message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants