Skip to content

Use async closure #6

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

Merged
merged 3 commits into from
Mar 15, 2025
Merged

Use async closure #6

merged 3 commits into from
Mar 15, 2025

Conversation

touilleMan
Copy link
Contributor

@touilleMan touilleMan commented Mar 15, 2025

close #5

Note this constitutes a breaking change, and means only Rust>=1.85 is supported

Regarding the Rust>=1.85, another solution to avoid this would be to put async closure support under a feature.
This is a bit trick though: since Faction::open & TransactionBuilder::run would have two flavors of signature.

To do that there is basically three ways:

  1. Duplicate Faction::open & TransactionBuilder::run bodies
  2. Put Faction::open & TransactionBuilder::run body inside a macro which is called in each flavor of the functions
  3. Have the async-closure flavor of Faction::open & TransactionBuilder::run internally calles the sync-closure -returning-future
    #[cfg(not(feature = "async-closure"))]
    pub async fn open<Fun, RetFut>(
        &self,
        name: &str,
        version: u32,
        on_upgrade_needed: Fun,
    ) -> crate::Result<Database<Err>, Err>
    where
        Fun: 'static + FnOnce(VersionChangeEvent<Err>) -> RetFut,
        RetFut: 'static + Future<Output = crate::Result<(), Err>> {
        self.open_internal(name, version, on_upgrade_needed).await
}

    #[cfg(feature = "async-closure")]
    pub async fn open(
        &self,
        name: &str,
        version: u32,
        on_upgrade_needed: impl AsyncFnOnce(VersionChangeEvent<Err>) -> crate::Result<(), Err> + 'static,
    ) -> crate::Result<Database<Err>, Err> {
        self.open_internal(name, version, |evt| on_upgrade_needed(evt)).await
    }

    async fn open_internal<Fun, RetFut>(
        &self,
        name: &str,
        version: u32,
        on_upgrade_needed: Fun,
    ) -> crate::Result<Database<Err>, Err>
    where
        Fun: 'static + FnOnce(VersionChangeEvent<Err>) -> RetFut,
        RetFut: 'static + Future<Output = crate::Result<(), Err>> {
        ...
    }

Solution 3 seems the most elegant, however it only works because the async closure is 'static.

If that wasn't the case we would in theory need to use a unsafe std::mem::transmute to overwrite the lifetime of the async closure before passing it to open_internal... but even this doesn't work in practice:

  • In Rust each closure has it own type (i.e. two closures with the same signature are considered of different types), so turning a impl AsyncFnOnce() into a impl AsyncFnOnce() + 'static cannot be expressed as far as I'm aware (as there is no way in telling the resulting impl AsyncFnOnce() + 'static have the same size than the impl AsyncFnOnce() provided as input)
  • On top of that unsafe_jar::run relies on wasm_bindgen::Closure::once which itself requires the passed closure to be 'static

Copy link
Owner

@Ekleog Ekleog left a comment

Choose a reason for hiding this comment

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

LGTM! Just two small style nits, I'm committing them to your branch and merging 😄

@Ekleog Ekleog merged commit 0aec463 into Ekleog:main Mar 15, 2025
1 check passed
@Ekleog
Copy link
Owner

Ekleog commented Mar 15, 2025

Thank you for the PR! I'll probably release 0.5 after coming to a decision on #4 ; and having made enough progress on sakuhiki to be sure the new API works fine for me 😄

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.

Use AsyncFnOnce instead of FnOnce -> RetFut
2 participants