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

Question about "#[pymethods] cannot be used with lifetime parameters or generics" #1088

Closed
tomplex opened this issue Aug 7, 2020 · 3 comments
Labels

Comments

@tomplex
Copy link

tomplex commented Aug 7, 2020

Hello,

I'm a newbie to Rust in general and especially PyO3, so if there's a better place to ask this please let me know.

I'm trying to wrap / expose to Python a Rust struct which requires lifetime parameters. In #436 a comment suggests:

As far as I know there's no way to make lifetimes sound with python objects, i.e. we can't enforce that the referenced objects actually outlive our object after we moved it into a python object.

How do I reconcile this with my need to specify a lifetime for this struct?

This is an essentially equivalent example:

use std::io::{BufReader, Read};
use std::fs::File;

use pyo3::prelude::*;
use pyo3::exceptions;

// This is library code, owned by someone else 
// which I'm trying to wrap 
struct FancyReader<'a> {
    reader: &'a mut dyn Read,
}

impl <'a> FancyReader<'a> {
    pub fn open<R: 'a + Read>(reader: &'a mut R) -> Result<Self, String> {
        Ok(FancyReader { reader, })
    }
}

// My code
#[pyclass(name=Reader, unsendable)]
struct PyReader<'a> {
    reader: FancyReader<'a>,
}

#[pymethods]
impl <'a> PyReader<'a> {
    #[new]
    fn new(filepath: String) -> PyResult<Self> {
        let mut file_reader = BufReader::new(File::open(filepath)?);

        let fancy_reader = FancyReader::open(&mut file_reader);
        match fancy_reader {
            Ok(reader) => PyResult::Ok(PyReader{reader,}),
            Err(_) => {
                let error = PyErr::new::<exceptions::OSError, _>("Could not open or read file.");
                PyResult::Err(error)
            }
        }
    }
}

I get a bit further trying the'static lifetime in my pyclass, but I don't know if that is best practice. Also, I then get an error about the lifetimes not matching between the new function and FancyReader::open:

error[E0597]: `file_reader` does not live long enough
  --> src/lib.rs:30:46
   |
30 |         let fancy_reader = FancyReader::open(&mut file_reader);
   |                            ------------------^^^^^^^^^^^^^^^^-
   |                            |                 |
   |                            |                 borrowed value does not live long enough
   |                            argument requires that `file_reader` is borrowed for `'static`
...
38 |     }
   |     - `file_reader` dropped here while still borrowed

I'm not sure where to go from here. Many thanks for any help you can provide.

@davidhewitt
Copy link
Member

Hi, thanks for the great question.

As you noted, it's very demanding for Rust's lifetime semantics to be reconciled with Python, where the reference counting strategy means objects can live arbitrarily long. (I think in the future we might be able to allow borrowing other data that's specifically managed by Python, but that's a hard question that's not enjoyed any design time yet).

These are a few of the strategies I might consider when facing a lifetime in a struct. Hopefully one of them will be right for you:

Change the field lifetime to 'static

As you mention above, this is one way to remove a lifetime. However, this places a very strong restriction on the fields: by denoting 'static you're pretty much saying that field will own all of its data (or is borrowed from data hard-coded into the program).

Because this restriction is so demanding, it only works in a handful of cases - and won't apply here for you, as you've seen.

Consider taking ownership instead of borrowing

One way to remove a lifetime requirement from a trait object reference &'a mut dyn Read is to instead modify it to be an owned trait object: Box<dyn Read>.

This requires that the object which is being converted to a trait object itself is 'static. std::fs::File doesn't have a lifetime parameter, so this is satisfied.

However, in your case this still wouldn't work, because the modification needed would be to the FancyReader type, which is in a third-party library.

Consider changing your API to work around this

At this point, I'm afraid I'm out of solutions which don't require an algorithm redesign. Probably the best solution left to you is to not expose the PyReader type to Python. Maybe you can instead make a function which takes the filepath, and then does the complete read with FancyReader before then returning the result of that read to Python?

@tomplex
Copy link
Author

tomplex commented Aug 7, 2020

@davidhewitt, thanks so much for the quick and thoughtful reply. I really appreciate you taking the time to respond.

It does seem as though your third option is the best one available. I think I would be able to work it in such a way that I can write a class in Python-land which provides a similar functionality, accessed through functions from the Rust side.

I have an echo of an idea in my head that involves creating a wrapper type for FancyReader which has ownership of the trait object like you suggested as Box<dyn Read> and can safely allow it be borrowed by the FancyReader, but I suspect that would just push the problem down a level and the lifetimes would still bubble up to the pyclass. My understanding of these details is still fuzzy at best, but I suppose there's no better way to learn then trying and seeing it not work. =)

Thanks again for your time and for linking a couple of related issues I can look through!

@davidhewitt
Copy link
Member

As there has been no response on this question for some time, I'm going to close it. Thank you for the discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants