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

help
1

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?

2

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

3

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.

4

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.

5

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