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

magic conversion fails to compile under no_std #282

Closed
rnbguy opened this issue Oct 29, 2024 · 6 comments · Fixed by #283
Closed

magic conversion fails to compile under no_std #282

rnbguy opened this issue Oct 29, 2024 · 6 comments · Fixed by #283

Comments

@rnbguy
Copy link
Contributor

rnbguy commented Oct 29, 2024

The following tests fails to compile

// lib.rs
#![no_std]

pub fn add(left: u64, right: u64) -> u64 {
    left + right
}

#[cfg(test)]
mod tests {
    use super::*;
    use rstest::rstest;

    #[rstest]
    #[case("2", "2", "4")]
    fn it_works(#[case] left: u64, #[case] right: u64, #[case] expected: u64) {
        assert_eq!(add(left, right), expected);
    }
}

because this is how it is expanded,

// Recursive expansion of rstest macro
// ====================================

#[cfg(test)]
fn it_works(left: u64, right: u64, expected: u64) {
    {
        match (&(add(left, right)), &expected) {
            (left_val, right_val) => {
                if !(*left_val == *right_val) {
                    let kind = core::panicking::AssertKind::Eq;
                    core::panicking::assert_failed(
                        kind,
                        &*left_val,
                        &*right_val,
                        core::option::Option::None,
                    );
                }
            }
        };
    }
}
#[cfg(test)]
mod it_works {
    use super::*;
    #[test]
    fn case_1() {
        let left = {
            use rstest::magic_conversion::*;
            (&&&Magic::<u64>(std::marker::PhantomData)).magic_conversion("2")
        };
        let right = {
            use rstest::magic_conversion::*;
            (&&&Magic::<u64>(std::marker::PhantomData)).magic_conversion("2")
        };
        let expected = {
            use rstest::magic_conversion::*;
            (&&&Magic::<u64>(std::marker::PhantomData)).magic_conversion("4")
        };
        it_works(left, right, expected)
    }
}

rstest could have used core::marker::PhantomData.

@rnbguy
Copy link
Contributor Author

rnbguy commented Oct 29, 2024

Probably need a pass through clippy::std_instead_of_core and clippy::std_instead_of_alloc lints 🙂

@la10736
Copy link
Owner

la10736 commented Nov 13, 2024

Unfortunately I have no time to look on it.

@rnbguy
Copy link
Contributor Author

rnbguy commented Nov 13, 2024

I am happy to make a PR. 🙂 but, can I expect a quick release with it ? 🙏

@la10736
Copy link
Owner

la10736 commented Nov 13, 2024

If you provide a PR I'll review and add it ASAP

@la10736
Copy link
Owner

la10736 commented Nov 14, 2024

Just a note: you have a workaround here:

// lib.rs
#![no_std]

pub fn add(left: u64, right: u64) -> u64 {
    left + right
}

#[cfg(test)]
mod tests {
    use super::*;
    use rstest::rstest;
    use ::core as std;

    #[rstest]
    #[case("2", "2", "4")]
    fn it_works(#[case] left: u64, #[case] right: u64, #[case] expected: u64) {
        assert_eq!(add(left, right), expected);
    }
}

Or better

// lib.rs
#![no_std]
#![cfg(test)]
extern crate std;

pub fn add(left: u64, right: u64) -> u64 {
    left + right
}

#[cfg(test)]
mod tests {
    use super::*;
    use rstest::rstest;

    #[rstest]
    #[case("2", "2", "4")]
    fn it_works(#[case] left: u64, #[case] right: u64, #[case] expected: u64) {
        assert_eq!(add(left, right), expected);
    }
}

@la10736
Copy link
Owner

la10736 commented Nov 14, 2024

Even if your crate is a no_std one the tests are compiled with std support (they run in multi threading environment and is sure that they have std support). That means in your library your code cannot use std directly because the #![no_std] attribute remove this crate from the resolution but when you compile test the std binary is present.

So, we can just just take care that the rendered code use core instead std if it's possible, but the rstest library can still remain in std mode because it will be compiled with the std support and executed in a std environment.

I'll add these notes in the PR also.

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 a pull request may close this issue.

2 participants