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

Elide lifetimes in derived functions #226

Merged
merged 1 commit into from Nov 21, 2022
Merged

Conversation

drivasperez
Copy link
Contributor

@drivasperez drivasperez commented Nov 19, 2022

Closes #211.

The bug was caused by user structs and enums using the same lifetime ('a) that was used by some of the derived functions (help, url, code) in their signatures. So for the following example:

use miette::Diagnostic;
use thiserror::Error;

#[derive(Debug, Diagnostic, Error)]
#[error("foo")]
#[diagnostic(help("foo"))]
pub enum Foo<'a> {
    Bar { bar: &'a str },
}

you would get derived code that looks like this:

use miette::Diagnostic;
use thiserror::Error;
#[error("foo")]
#[diagnostic(help("foo"))]
pub enum Foo<'a> {
    Bar { bar: &'a str },
}

impl<'a> miette::Diagnostic for Foo<'a> {
    fn help<'a>(
        &'a self,
    ) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + 'a>> {
        #[allow(unused_variables, deprecated)]
        match self {
            Self::Bar { bar } => {
                std::option::Option::Some(
                    std::boxed::Box::new({
                        let res = ::alloc::fmt::format(
                            ::core::fmt::Arguments::new_v1(&["foo"], &[]),
                        );
                        res
                    }),
                )
            }
            _ => std::option::Option::None,
        }
    }
}

Which is a compile error because 'a is defined twice. However, it seems like you can just elide the explicit lifetime in the generated functions, which gives you code that looks like:

impl<'a> miette::Diagnostic for Foo<'a> {
    fn help(&self) -> std::option::Option<std::boxed::Box<dyn std::fmt::Display + '_>> {
        #[allow(unused_variables, deprecated)]
        match self {
            Self::Bar { bar } => {
                std::option::Option::Some(
                    std::boxed::Box::new({
                        let res = ::alloc::fmt::format(
                            ::core::fmt::Arguments::new_v1(&["foo"], &[]),
                        );
                        res
                    }),
                )
            }
            _ => std::option::Option::None,
        }
    }
}

So all appears to be well. I changed most of the derived tests to use a 'a lifetime while trying to work out where it was breaking. It seems like this is a case that should be covered but maybe in its own specialised unit test rather than scattered through the others?

@zkat zkat merged commit c88f0b5 into zkat:main Nov 21, 2022
@drivasperez drivasperez deleted the issue-211 branch November 22, 2022 14:14
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.

Derive macro breaks on types with 'a lifetime parameters
2 participants