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

Support async return #58

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Support async return #58

wants to merge 6 commits into from

Conversation

waynexia
Copy link

Support for defining fail points with async blocks. E.g.:

  async fn async_fn() {
      fail_point!("async_return", |s| async {
          (async {}).await;
          s.map_or(2, |s| s.parse().unwrap())
      });
  }

Signed-off-by: Ruihang Xia <[email protected]>
@BusyJay
Copy link
Member

BusyJay commented Sep 29, 2021

Interesting. What if user wants async move {}?

Signed-off-by: Ruihang Xia <[email protected]>
@waynexia
Copy link
Author

waynexia commented Oct 8, 2021

Sorry for the late reply :P

For move before the closure param list, we can match it with ident. But I can't figure out how to match those after async. Here I use another rule to handle this. Please let me know if there is a better approach!

@BusyJay
Copy link
Member

BusyJay commented Oct 9, 2021

How about:

    ($name:expr, await $e:expr) => {{
        if let Some(res) = $crate::eval($name, $e) {
            return res.await;
        }
    }};

The rule is simple. And it matches all cases without depending on the appearance of any keywords in the expression. One drawback is it moves await ahead, which is opposite to the syntax of await.

@BusyJay
Copy link
Member

BusyJay commented Oct 9, 2021

/cc @brson @lucab @sticnarf I also would like to hear your advice.

@waynexia
Copy link
Author

It is a good point to make the rules simple by not following the formal grammar. Wondering what others think.

@lucab
Copy link
Member

lucab commented Oct 11, 2021

Speaking of async, I do wonder whether it makes sense to have a dedicated async_fail_point!() instead.
I currently see a mismatch with actions like sleep and yield in an async context, which will block the executor instead of behaving more gracefully.
I'm not really a macro expert, but perhaps the implementation could end up being easier if we clearly split the sync-vs-async domains through different macros?

Cargo.toml Outdated Show resolved Hide resolved
@waynexia
Copy link
Author

I defined a new macro async_fail_point as @lucab said. It does make things more straightforward 👍 PTAL @BusyJay @lucab

Signed-off-by: Ruihang Xia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants