Simple but tricky logic refactoring, macros vs closures + futures + lifetimes. Rust in HARD mode

Here is a fun one for the expert Rust heads to get their head around!

Essentially I want something like this pseudo macro code:

macro_rules! legacy_compat_retrieval {
    ($hash: ident, $fut: expr ) => {{
        let processed = full_path_processed(hash);
        // this match scrutinee shouldn't be hardcoded,
        // but it depends on the path generated in the line above.
        match self.retrieve(&processed).await {
            Ok(file) => Ok(file),
            Err(BlobStorageError::FileNotFound) => {
                let original = full_path_original(hash);
                // this match scrutinee shouldn't be hardcoded,
                // but it depends on the path generated in the line above.
                match self.retrieve(&original).await {
                    Ok(file) => Ok(file),
                    Err(BlobStorageError::FileNotFound) => {
                        let legacy = self.legacy_file_path(hash);
                        // the expr inside `Ok` shouldn't be hardcoded,
                        // but it depends on the path generated in the line above.
                        Ok(self.retrieve(&legacy).await?)
                    }
                    Err(e) => Err(e),
                }
            }
            Err(e) => Err(e),
        }
    }};
}

where Instead of self.retrieve(&processed).await I can plug in any async function that takes at least a path, but maybe more parameters.

So I got a version working with closures:

async fn legacy_compat_retrieval<T, F1, F2, F3, Fut1, Fut2, Fut3>(
    &self,
    hash: &Hash,
    f1: F1,
    f2: F2,
    f3: F3,
) -> Result<T, BlobStorageError>
where
    F1: FnOnce(String) -> Fut1,
    F2: FnOnce(String) -> Fut2,
    F3: FnOnce(String) -> Fut3,
    Fut1: Future<Output = Result<T, BlobStorageError>>,
    Fut2: Future<Output = Result<T, BlobStorageError>>,
    Fut3: Future<Output = Result<T, BlobStorageError>>,
{
    let processed = full_path_processed(hash);
    match f1(processed).await {
        Ok(file) => Ok(file),
        Err(BlobStorageError::FileNotFound) => {
            let original = full_path_original(hash);
            match f2(original).await {
                Ok(file) => Ok(file),
                Err(BlobStorageError::FileNotFound) => {
                    let legacy = self.legacy_file_path(hash);
                    f3(legacy).await
                }
                Err(e) => Err(e),
            }
        }
        Err(e) => Err(e),
    }
}

And this works, when called like so:

pub async fn retrieve_file(&self, hash: &Hash) -> Result<Vec<u8>, BlobStorageError> {
    self.legacy_compat_retrieval(
        hash,
        |path| async move { self.retrieve(&path).await },
        |path| async move { self.retrieve(&path).await },
        |path| async move { self.retrieve(&path).await },
    )
    .await
}

The problem is, I cant pass &mut into the closures. So this doesn't work

pub async fn retrieve_file_streaming<W>(
    &self,
    hash: &Hash,
    writer: &mut W,
) -> Result<(), BlobStorageError>
where
    W: AsyncWrite + Send + Unpin,
{
    let w1 = writer;
    let w2 = writer;
    let w3 = writer;
    self.legacy_compat_retrieval(
        hash,
        |path| async move { self.retrieve_streaming(&path, w1).await },
        |path| async move { self.retrieve_streaming(&path, w2).await },
        |path| async move { self.retrieve_streaming(&path, w3).await },
    )
    .await
}

Can anyone think of a way where I can refactor out the matching logic so i wouldn't have to WET the legacy compat logic everywhere?

Make the closures take the writer parameter and pass it in explicitly along with the path.

are you saying to modifty the sig to be FnOnce(String, W) -> Fut1? the problem is some fns will have a writer, others may not.

You can't have multiple &mut to the same writer at the same time, and lifetimes passed to another function (including hidden in your closure and future types) must be valid for the entire call. A macro can do it by removing the function barrier and relying on function body reborrow/lifetime analysis, but there's no way to import analysis across the function barrier.

So I feel anything that makes this work without changing the shape is going to be a hack.

Moreover, even if changing the shape is acceptable, passing something or some set of things to a function to allow it to generate multiple futures with some exclusively shared resources is going to involve either some sort of synchronization overhead, or some higher-ranked bound to express unnameable, borrow-capturing futures, which is still generally a mess in Rust.

I don't think there's a simple solution. If you have a macro that works, the juice probably isn't worth the squeeze.

I thought about suggesting that the matches should be unnested. That way the found-or-else progress could be made sequential which lends itself to chaining. It doesn't quite work out as well as I had hoped in practice...

You can fake it with a much simpler macro that only implements the early bail like ? or try!, but with the domain-specific behavior of letting the retry logic occur sequentially.

Here's the macro, assuming $crate::BlobStorageError is a good reference to use:

macro_rules! try_found {
	($expr:expr) => {
		match $expr {
			::std::result::Result::Ok(file) => return ::std::result::Result::Ok(file),
			::std::result::Result::Err($crate::BlobStorageError::FileNotFound) => (),
			::std::result::Result::Err(err) => return ::std::result::Result::Err(err),
		}
	}
}

The two call sites are just three lines each (six if you split up the get-path/retrieve calls like you had been). I don't see anything particularly WET about this:

	pub async fn retrieve_file(&self, hash: &Hash) -> Result<Vec<u8>, BlobStorageError> {
		try_found!(self.retrieve(&full_path_processed(hash)).await);
		try_found!(self.retrieve(&full_path_original(hash)).await);
		self.retrieve(&self.legacy_file_path(hash)).await
	}

	pub async fn retrieve_file_streaming<W>(
		&self,
		hash: &Hash,
		writer: &mut W,
	) -> Result<(), BlobStorageError>
	where
		W: AsyncWrite + Send + Unpin,
	{
		try_found!(self.retrieve_streaming(&full_path_processed(hash), writer).await);
		try_found!(self.retrieve_streaming(&full_path_original(hash), writer).await);
		self.retrieve_streaming(&self.legacy_file_path(hash), writer).await
	}
2 Likes

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.