Proc-macro interface for async-await export #975

chitoyuu · 2022-11-12T18:07:03Z

This is a two-part PR that accomplishes the following:

Fully proceduralize method exports

Converted the remaining macro_rules! used by the #[methods] macro into procedural macros, increasing flexibility for future expansion and reducing reliance on #[doc(hidden)] macro items.

This makes it easier to support more combinations of method signatures, without having to manually write out n^2 patterns.

Proc-macro interface for async-await export

Added support for exporting async fns and fns that return futures through the proc-macro interface.

Added #[method(async)], for methods that return futures but aren't async themselves. This is especially useful for working around lifetime elision (Rust putting the lifetime of &self into the return values of async fns).
Added the #[ctx] attribute that denotes the async context. Special arguments (#[base], #[ctx], and possibly #[self] in the future) can now be declared in any order, so long as they precede all regular arguments.
Methods without receivers can now be exported. However, they still currently use Map semantics.

Utterly double close #284.

Error messages

As a side effect to this refactoring, the macros are now capable of generating some fairly precisely spanned error messages:

derive_fail_methods_special_args.stderr

error: the method receiver cannot be optional (instead, remove the argument entirely)
  --> tests/ui/derive_fail_methods_special_args.rs:14:25
   |
14 |     async fn optional(#[opt] self, #[base] #[opt] _owner: &Node, #[ctx] #[opt] ctx: ()) {}
   |                         ^^^

error: the base/owner object cannot be optional (instead, remove the argument entirely)
  --> tests/ui/derive_fail_methods_special_args.rs:14:46
   |
14 |     async fn optional(#[opt] self, #[base] #[opt] _owner: &Node, #[ctx] #[opt] ctx: ()) {}
   |                                              ^^^

error: the async context cannot be optional (instead, remove the argument entirely)
  --> tests/ui/derive_fail_methods_special_args.rs:14:75
   |
14 |     async fn optional(#[opt] self, #[base] #[opt] _owner: &Node, #[ctx] #[opt] ctx: ()) {}
   |                                                                           ^^^

error: the method receiver cannot also be the base/owner object
  --> tests/ui/derive_fail_methods_special_args.rs:17:16
   |
17 |     fn based(#[base] self, #[base] _base: &Node, #[base] #[base] _basil: &Node, #[base] #[base] #[base] _basin: &Node) {}
   |                ^^^^

error: duplicate attribute
  --> tests/ui/derive_fail_methods_special_args.rs:17:60
   |
17 |     fn based(#[base] self, #[base] _base: &Node, #[base] #[base] _basil: &Node, #[base] #[base] #[base] _basin: &Node) {}
   |                                                            ^^^^

error: the special parameter base/owner object must only be declared once (the same parameter is already defined at #1)
  --> tests/ui/derive_fail_methods_special_args.rs:17:66
   |
17 |     fn based(#[base] self, #[base] _base: &Node, #[base] #[base] _basil: &Node, #[base] #[base] #[base] _basin: &Node) {}
   |                                                                  ^^^^^^

error: duplicate attribute
  --> tests/ui/derive_fail_methods_special_args.rs:17:91
   |
17 |     fn based(#[base] self, #[base] _base: &Node, #[base] #[base] _basil: &Node, #[base] #[base] #[base] _basin: &Node) {}
   |                                                                                           ^^^^

error: duplicate attribute
  --> tests/ui/derive_fail_methods_special_args.rs:17:99
   |
17 |     fn based(#[base] self, #[base] _base: &Node, #[base] #[base] _basil: &Node, #[base] #[base] #[base] _basin: &Node) {}
   |                                                                                                   ^^^^

error: the special parameter base/owner object must only be declared once (the same parameter is already defined at #1)
  --> tests/ui/derive_fail_methods_special_args.rs:17:105
   |
17 |     fn based(#[base] self, #[base] _base: &Node, #[base] #[base] _basil: &Node, #[base] #[base] #[base] _basin: &Node) {}
   |                                                                                                         ^^^^^^

error: the async context is only available to async methods
  --> tests/ui/derive_fail_methods_special_args.rs:20:26
   |
20 |     fn sync(self, #[ctx] ctx: ()) {}
   |                          ^^^

Close #439.

Compatibility

There aren't any breaking changes that I'm aware of. A shim for the legacy godot_wrap_method macro is kept for compatibility. It can be removed in the next breaking release. Async exports are unsupported by this shim.

Converted the remaining macro_rules! used by the `#[methods]` macro into procedural macros, increasing flexibility for future expansion and reducing reliance on `#[doc(hidden)]` macro items. This makes it easier to support more combinations of method signatures, without having to manually write out n^2 patterns. A shim for the legacy `godot_wrap_method` macro is kept for compatibility. It can be removed in the next breaking release.

chitoyuu · 2022-11-24T11:39:22Z

bors try

bors · 2022-11-24T12:11:15Z

try

Build failed:

full-ci

Bromeon

Just realized I didn't submit my review I started a few days ago. Not yet exhaustive, just a first few thoughts. Thanks a lot for this great addition! 🙂

Bromeon · 2022-11-12T21:20:47Z

gdnative-derive/src/lib.rs

+/// Convenience macro to wrap an object's method into a `Method` implementor
+/// that can be passed to the engine when registering a class.
+#[proc_macro]
+#[deprecated = "The legacy manual export macro is deprecated and will be removed in a future godot-rust version. \
+Either use the `#[methods]` attribute macro, or implement the `Method` trait manually instead."]
+pub fn godot_wrap_method(input: TokenStream) -> TokenStream {


Clever! I think for all practical purposes, turning a declarative macro into a procedural function-like one should not constitute a breaking change.

It shouldn't. There used to be a limitation where procedural function-like macros couldn't be used in expressions, but that was lifted quite a while ago (below our MSRV).

Bromeon · 2022-11-12T21:21:28Z

gdnative-derive/src/lib.rs

+/// Returns the (possibly renamed or imported as `gdnative`) identifier of the `gdnative_core` crate.
+fn crate_gdnative_core() -> proc_macro2::TokenStream {
+    let found_crate = proc_macro_crate::crate_name("gdnative-core")
+        .or_else(|_| proc_macro_crate::crate_name("gdnative"))
+        .expect("crate not found");
+
+    match found_crate {
+        proc_macro_crate::FoundCrate::Itself => quote!(crate),
+        proc_macro_crate::FoundCrate::Name(name) => {
+            let ident = proc_macro2::Ident::new(&name, proc_macro2::Span::call_site());
+            quote!( #ident )
+        }
+    }
+}


This is a really nice change, hardcoding the crate name was something that bothered me a bit in the past.

gdnative-derive/src/lib.rs

gdnative-derive/src/methods.rs

gdnative/tests/ui/derive_fail_methods_special_args.rs

Bromeon · 2022-11-25T12:14:09Z

gdnative-derive/src/lib.rs

+///   // This will NOT compile: Rust assumes that any futures returned by an `async fn` may only live as long as each of its
+///   // arguments, and there is no way to tell it otherwise. As a result, it will emit some cryptic complaints about lifetime.
+///   #[method]
+///   async fn answer(&self) -> i32 {
+///      42
+///   }


Is this not contradicting the code examples further above, where you have async fns that are apparently? Or is the difference the lack of return value?

Technically it isn't, but I see how it can be confusing. As far as the macro is concerned, this declaration is valid. The only thing preventing it from compiling is Rust's lifetime elision rules, and those rules can be relaxed down the line such that code like this compile.

I'll try to think of a way to word the examples better.

Yep, I think it would help to have a rule for users to know when they can use async fn and when not.

Maybe some of these rules could also be detected by the proc-macro itself, to give a better compiler message.

There's a problem with macro-based detection though: we still can't emit proper warnings. I don't think we want async fn (&self) to be a hard error here, for the reasons above, but any warnings we emit are still through #[deprecated], and as such a user cannot #[allow] them precisely without also allowing all the actually deprecated stuff.

Re-worded the examples.

Bromeon · 2022-11-25T12:15:41Z

gdnative-derive/src/lib.rs

+///     - `#[ctx]` - The [async context](gdnative::tasks::Context), for async methods. See the `async` argument
+///       below.


Typical parameter declaration is

#[ctx] ctx: Arc<Context>

Is that clear enough? Or should the type perhaps be named AsyncContext? The latter would make sure that it doesn't make sense in a non-async function signature.

Renaming the type would constitute a breaking change though, and I'm not exactly sure how much it's going to matter: currently the user have to import the Context type from gdnative::tasks::Context, which already indicates that it's related to async. The API is a dead giveaway too. I guess the attribute can be renamed #[async_ctx] if the extra clarity is worth the verbosity.

Note that the macro emits a rather informative error (imo) when it sees a #[ctx] in a non-async function:

gdnative/gdnative/tests/ui/derive_fail_methods_special_args.stderr

Lines 55 to 59 in ef25b42

error: the async context is only available to async methods

--> tests/ui/derive_fail_methods_special_args.rs:20:26

|

20 | fn sync(self, #[ctx] ctx: ()) {}

| ^^^

Ah true, this type was already there. #[async_ctx] was actually my first idea, but I thought in the type it might be clearer.

#[async_ctx] might be a bit clearer, but maybe we should ask some users. On the other hand, long signatures might be broken into multiple lines by rustfmt, and then having the same length as #[opt] would be a tiny readability bonus 😉probably not that much difference in practice.

Changed to #[async_ctx] for now.

chitoyuu · 2022-11-25T15:25:54Z

Thanks for the review! I'll implement most of the suggested changes tomorrow.

Added support for exporting `async fn`s and `fn`s that return futures through the proc-macro interface. - Added `#[method(async)]`, for methods that return futures but aren't async themselves. This is especially useful for working around lifetime elision (Rust putting the lifetime of `&self` into the return values of `async fn`s). - Added the `#[ctx]` attribute that denotes the async context. Special arguments (`#[base]`, `#[ctx]`, and possibly `#[self]` in the future) can now be declared in any order, so long as they precede all regular arguments. - Methods without receivers can now be exported. However, they still currently use `Map` semantics. Async exports are unsupported by the legacy `godot_wrap_method` shim.

chitoyuu · 2022-11-26T17:29:16Z

bors try

bors · 2022-11-26T17:43:55Z

try

Build succeeded:

full-ci

chitoyuu · 2022-11-30T04:21:58Z

Will merge a day later so related development can continue.

chitoyuu · 2022-12-01T07:28:31Z

bors r+

bors · 2022-12-01T07:48:08Z

Build succeeded:

full-ci

chitoyuu mentioned this pull request Nov 22, 2022

added Third-Person-Shooter example #977

Merged

bors bot added a commit that referenced this pull request Nov 24, 2022

Try #975:

f401c1d

Bromeon reviewed Nov 25, 2022

View reviewed changes

Bromeon added the feature Adds functionality to the library label Nov 25, 2022

chitoyuu force-pushed the feature/export-async branch from ef25b42 to 425cc3b Compare November 26, 2022 16:14

chitoyuu force-pushed the feature/export-async branch from 425cc3b to 6689a00 Compare November 26, 2022 17:29

bors bot added a commit that referenced this pull request Nov 26, 2022

Try #975:

fdc9395

Bromeon added the c: export Component: export (mod export, derive) label Nov 27, 2022

bors bot merged commit d0538f0 into godot-rust:master Dec 1, 2022

chitoyuu deleted the feature/export-async branch December 1, 2022 11:24

chitoyuu mentioned this pull request Mar 16, 2023

tokio recipe is still using a git dependency and the old method syntax godot-rust/gdnative-book#98

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proc-macro interface for async-await export #975

Proc-macro interface for async-await export #975

chitoyuu commented Nov 12, 2022

chitoyuu commented Nov 24, 2022

bors bot commented Nov 24, 2022

Bromeon left a comment

Bromeon Nov 12, 2022

chitoyuu Nov 25, 2022

Bromeon Nov 12, 2022

Bromeon Nov 25, 2022

chitoyuu Nov 25, 2022

Bromeon Nov 25, 2022

chitoyuu Nov 26, 2022

chitoyuu Nov 26, 2022

Bromeon Nov 25, 2022

chitoyuu Nov 25, 2022

Bromeon Nov 25, 2022

chitoyuu Nov 26, 2022

chitoyuu commented Nov 25, 2022

chitoyuu commented Nov 26, 2022

bors bot commented Nov 26, 2022

chitoyuu commented Nov 30, 2022 •

edited

Loading

chitoyuu commented Dec 1, 2022

bors bot commented Dec 1, 2022

		/// - `#[ctx]` - The [async context](gdnative::tasks::Context), for async methods. See the `async` argument
		/// below.

	error: the async context is only available to async methods
	--> tests/ui/derive_fail_methods_special_args.rs:20:26
	\|
	20 \| fn sync(self, #[ctx] ctx: ()) {}
	\| ^^^

Proc-macro interface for async-await export #975

Proc-macro interface for async-await export #975

Conversation

chitoyuu commented Nov 12, 2022

Fully proceduralize method exports

Proc-macro interface for async-await export

Error messages

Compatibility

chitoyuu commented Nov 24, 2022

bors bot commented Nov 24, 2022

try

Bromeon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chitoyuu commented Nov 25, 2022

chitoyuu commented Nov 26, 2022

bors bot commented Nov 26, 2022

try

chitoyuu commented Nov 30, 2022 • edited Loading

chitoyuu commented Dec 1, 2022

bors bot commented Dec 1, 2022

chitoyuu commented Nov 30, 2022 •

edited

Loading