This macro generates tests which cause clippy to panic

[lopopolo]

I am writing a macro to generate impls for some traits (in the manner of TryFrom) and test code. clippy panics when running cargo clippy --all-targets.

I invoke the macro in the same crate that defines it like this:

use crate::convert::fixnum::Int;

mrb_nilable_impl!(bool as nilable_bool);
mrb_nilable_impl!(Vec<u8> as nilable_byte_string);
// TODO: nilable float
mrb_nilable_impl!(Int as nilable_fixnum);
mrb_nilable_impl!(String as nilable_string);

This is the macro:

#[macro_export]
macro_rules! mrb_nilable_impl {
    ($type:ty as $wrapper_module:ident) => {
        #[allow(clippy::use_self)]
        impl $crate::TryFromMrb<std::option::Option<$type>> for $crate::Value {
            type From = $crate::Rust;
            type To = $crate::Ruby;

            unsafe fn try_from_mrb(
                mrb: *mut $crate::sys::mrb_state,
                value: std::option::Option<$type>,
            ) -> std::result::Result<Self, $crate::convert::Error<Self::From, Self::To>> {
                match value {
                    std::option::Option::Some(value) => Self::try_from_mrb(mrb, value),
                    std::option::Option::None => {
                        std::result::Result::Ok(Self::new($crate::sys::mrb_sys_nil_value()))
                    }
                }
            }
        }

        #[allow(clippy::use_self)]
        impl $crate::TryFromMrb<$crate::Value> for std::option::Option<$type> {
            type From = $crate::Ruby;
            type To = $crate::Rust;

            unsafe fn try_from_mrb(
                mrb: *mut $crate::sys::mrb_state,
                value: $crate::Value,
            ) -> std::result::Result<Self, $crate::convert::Error<Self::From, Self::To>> {
                match value.ruby_type() {
                    $crate::Ruby::Nil => std::result::Result::Ok(None),
                    _ => <$type>::try_from_mrb(mrb, value).map(std::option::Option::Some),
                }
            }
        }

        #[cfg(test)]
        mod $wrapper_module {
            mod tests {
                use quickcheck_macros::quickcheck;

                use $crate::convert::*;
                use $crate::value::*;

                #[test]
                fn fail_convert() {
                    unsafe {
                        let mrb = mrb_open();
                        let context = mrbc_context_new(mrb);
                        // get a mrb_value that can't be converted to a
                        // primitive type.
                        let code = "Object.new";
                        let value = mrb_load_nstring_cxt(
                            mrb,
                            code.as_ptr() as *const i8,
                            code.len(),
                            context,
                        );
                        let value = Value::new(value);
                        let result =
                            <std::option::Option<$type>>::try_from_mrb(mrb, value).map(|_| ());
                        mrb_close(mrb);
                        assert_eq!(
                            result.map_err(|e| e.from),
                            std::result::Result::Err(Ruby::Object)
                        );
                    }
                }

                #[quickcheck]
                fn convert_to_value(v: std::option::Option<$type>) -> bool
                where
                    $type: std::clone::Clone
                        + std::cmp::PartialEq
                        + TryFromMrb<Value, From = Ruby, To = Rust>,
                    Value: TryFromMrb<std::option::Option<$type>, From = Rust, To = Ruby>,
                    std::option::Option<$type>:
                        std::clone::Clone + TryFromMrb<Value, From = Ruby, To = Rust>,
                {
                    unsafe {
                        let mrb = mrb_open();
                        let value = match Value::try_from_mrb(mrb, v.clone()) {
                            std::result::Result::Ok(value) => value,
                            // we don't care about inner conversion failures for `T`
                            std::result::Result::Err(_) => return true,
                        };
                        let good = if let std::option::Option::Some(v) = v {
                            <$type>::try_from_mrb(mrb, value).expect("convert") == v
                        } else {
                            let inner = value.inner();
                            mrb_sys_value_is_nil(inner)
                        };
                        mrb_close(mrb);
                        good
                    }
                }

                #[quickcheck]
                fn roundtrip(v: std::option::Option<$type>) -> bool
                where
                    $type: std::clone::Clone
                        + std::cmp::PartialEq
                        + TryFromMrb<Value, From = Ruby, To = Rust>,
                    Value: TryFromMrb<std::option::Option<$type>, From = Rust, To = Ruby>,
                    std::option::Option<$type>:
                        std::clone::Clone + TryFromMrb<Value, From = Ruby, To = Rust>,
                {
                    unsafe {
                        let mrb = mrb_open();
                        let value = match Value::try_from_mrb(mrb, v.clone()) {
                            std::result::Result::Ok(value) => value,
                            // we don't care about inner conversion failures for `T`
                            std::result::Result::Err(_) => return true,
                        };
                        let good = <std::option::Option<$type>>::try_from_mrb(mrb, value)
                            .expect("convert")
                            == v;
                        mrb_close(mrb);
                        good
                    }
                }
            }
        }
    };
}

I have a similar macro which impls the same traits on Vec which does not crash clippy:

#[macro_export]
macro_rules! mrb_array_impl {
    ($type:ty as $wrapper_module:ident) => {
        #[allow(clippy::use_self)]
        impl $crate::TryFromMrb<std::vec::Vec<$type>> for $crate::Value {
            type From = $crate::Rust;
            type To = $crate::Ruby;

            unsafe fn try_from_mrb(
                mrb: *mut $crate::sys::mrb_state,
                value: std::vec::Vec<$type>,
            ) -> std::result::Result<Self, $crate::convert::Error<Self::From, Self::To>> {
                use std::convert::TryFrom;
                let size = i64::try_from(value.len()).map_err(|_| $crate::convert::Error {
                    from: $crate::Rust::Vec,
                    to: $crate::Ruby::Array,
                })?;
                let array = $crate::sys::mrb_ary_new_capa(mrb, size);
                for (i, item) in value.into_iter().enumerate() {
                    let idx = i64::try_from(i).map_err(|_| $crate::convert::Error {
                        from: $crate::Rust::Vec,
                        to: $crate::Ruby::Array,
                    })?;
                    let ary_item = Self::try_from_mrb(mrb, item)?;
                    let inner = ary_item.inner();
                    $crate::sys::mrb_ary_set(mrb, array, idx, inner);
                }
                std::result::Result::Ok(Self::new(array))
            }
        }

        #[allow(clippy::use_self)]
        impl $crate::TryFromMrb<$crate::Value> for std::vec::Vec<$type> {
            type From = $crate::Ruby;
            type To = $crate::Rust;

            unsafe fn try_from_mrb(
                mrb: *mut $crate::sys::mrb_state,
                value: $crate::Value,
            ) -> std::result::Result<Self, $crate::convert::Error<Self::From, Self::To>> {
                use std::convert::TryFrom;
                match value.ruby_type() {
                    $crate::Ruby::Array => {
                        let inner = value.inner();
                        let len = $crate::sys::mrb_sys_ary_len(inner);
                        let cap = usize::try_from(len).map_err(|_| $crate::convert::Error {
                            from: $crate::Ruby::Array,
                            to: $crate::Rust::Vec,
                        })?;
                        let mut vec = Self::with_capacity(cap);
                        for i in 0..cap {
                            let idx = i64::try_from(i).map_err(|_| $crate::convert::Error {
                                from: $crate::Ruby::Array,
                                to: $crate::Rust::Vec,
                            })?;
                            let item =
                                $crate::Value::new($crate::sys::mrb_ary_ref(mrb, inner, idx));
                            vec.push(<$type>::try_from_mrb(mrb, item)?);
                        }
                        std::result::Result::Ok(vec)
                    }
                    type_tag => std::result::Result::Err($crate::convert::Error {
                        from: type_tag,
                        to: $crate::Rust::Vec,
                    }),
                }
            }
        }

        #[cfg(test)]
        mod $wrapper_module {
            mod tests {
                use quickcheck_macros::quickcheck;
                use std::convert::TryFrom;

                use $crate::convert::*;
                use $crate::value::*;

                #[test]
                fn fail_convert() {
                    unsafe {
                        let mrb = mrb_open();
                        let value = Value::new(mrb_sys_true_value());
                        let expected = Error {
                            from: Ruby::Bool,
                            to: Rust::Vec,
                        };
                        let result = <std::vec::Vec<$type>>::try_from_mrb(mrb, value).map(|_| ());
                        mrb_close(mrb);
                        assert_eq!(result, Err(expected));
                    }
                }

                #[allow(clippy::needless_pass_by_value)]
                #[quickcheck]
                fn convert_to_value(v: std::vec::Vec<$type>) -> bool
                where
                    $type: std::clone::Clone
                        + std::cmp::PartialEq
                        + TryFromMrb<Value, From = Ruby, To = Rust>,
                    Value: TryFromMrb<std::vec::Vec<$type>, From = Rust, To = Ruby>,
                    std::vec::Vec<$type>:
                        std::clone::Clone + TryFromMrb<Value, From = Ruby, To = Rust>,
                {
                    unsafe {
                        let mrb = mrb_open();
                        let value = match Value::try_from_mrb(mrb, v.clone()) {
                            std::result::Result::Ok(value) => value,
                            // we don't care about inner conversion failures for `T`
                            std::result::Result::Err(_) => return true,
                        };
                        let inner = value.inner();
                        let size = i64::try_from(v.len()).expect("vec size");
                        let good = mrb_sys_ary_len(inner) == size;
                        mrb_close(mrb);
                        good
                    }
                }

                #[allow(clippy::needless_pass_by_value)]
                #[quickcheck]
                fn roundtrip(v: std::vec::Vec<$type>) -> bool
                where
                    $type: std::clone::Clone
                        + std::cmp::PartialEq
                        + TryFromMrb<Value, From = Ruby, To = Rust>,
                    Value: TryFromMrb<std::vec::Vec<$type>, From = Rust, To = Ruby>,
                    std::vec::Vec<$type>:
                        std::clone::Clone + TryFromMrb<Value, From = Ruby, To = Rust>,
                {
                    unsafe {
                        let mrb = mrb_open();
                        let value = match Value::try_from_mrb(mrb, v.clone()) {
                            std::result::Result::Ok(value) => value,
                            // we don't care about inner conversion failures for `T`
                            std::result::Result::Err(_) => return true,
                        };
                        let good =
                            <std::vec::Vec<$type>>::try_from_mrb(mrb, value).expect("convert") == v;
                        mrb_close(mrb);
                        good
                    }
                }
            }
        }
    };
}

Meta

Version

$ rustc --version --verbose
rustc 1.35.0-nightly (88f755f8a 2019-03-07)
binary: rustc
commit-hash: 88f755f8a84df1d9e6b17cf10c96ae8b93481b2e
commit-date: 2019-03-07
host: x86_64-apple-darwin
release: 1.35.0-nightly
LLVM version: 8.0

Backtrace:

$ RUST_BACKTRACE=1 cargo clippy --all-targets
    Checking mruby v0.1.0 (/Users/lopopolo/dev/repos/rube/mruby)
thread 'rustc' panicked at 'there must be a `else` here', src/libcore/option.rs:1038:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::_print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: rustc::util::common::panic_hook
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::continue_panic_fmt
   7: rust_begin_unwind
   8: core::panicking::panic_fmt
   9: core::option::expect_failed
  10: <clippy_lints::formatting::Formatting as rustc::lint::EarlyLintPass>::check_expr
  11: <rustc::lint::context::EarlyLintPassObjects<'_> as rustc::lint::EarlyLintPass>::check_expr
  12: <rustc::lint::context::EarlyContextAndPass<'a, T> as syntax::visit::Visitor<'a>>::visit_expr
  13: <rustc::lint::context::EarlyContextAndPass<'a, T> as syntax::visit::Visitor<'a>>::visit_local
  14: syntax::visit::walk_expr
  15: <rustc::lint::context::EarlyContextAndPass<'a, T> as syntax::visit::Visitor<'a>>::visit_expr
  16: syntax::visit::walk_fn
  17: syntax::visit::walk_item
  18: <rustc::lint::context::EarlyContextAndPass<'a, T> as syntax::visit::Visitor<'a>>::visit_item
  19: syntax::visit::walk_fn
  20: syntax::visit::walk_item
  21: <rustc::lint::context::EarlyContextAndPass<'a, T> as syntax::visit::Visitor<'a>>::visit_item
  22: syntax::visit::walk_item
  23: <rustc::lint::context::EarlyContextAndPass<'a, T> as syntax::visit::Visitor<'a>>::visit_item
  24: syntax::visit::walk_item
  25: <rustc::lint::context::EarlyContextAndPass<'a, T> as syntax::visit::Visitor<'a>>::visit_item
  26: syntax::visit::walk_item
  27: <rustc::lint::context::EarlyContextAndPass<'a, T> as syntax::visit::Visitor<'a>>::visit_item
  28: syntax::visit::walk_item
  29: syntax::visit::walk_crate
  30: rustc::lint::context::early_lint_crate
  31: rustc::lint::context::check_ast_crate
  32: rustc::util::common::time
  33: rustc_driver::driver::compile_input
  34: rustc_driver::run_compiler_with_pool
  35: <scoped_tls::ScopedKey<T>>::set
  36: rustc_driver::run_compiler
  37: <scoped_tls::ScopedKey<T>>::set
query stack during panic:
end of query stack

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.35.0-nightly (88f755f8a 2019-03-07) running on x86_64-apple-darwin

note: compiler flags: -C debuginfo=2 -C incremental

note: some of the compiler flags provided by cargo are hidden

error: Could not compile `mruby`.

To learn more, run the command again with --verbose.

[lopopolo]

Hmm I was running an out of date nightly. After pulling down the latest, this seems to no longer be an issue.

Version

$ rustc --version --verbose
rustc 1.36.0-nightly (3c3d3c177 2019-04-17)
binary: rustc
commit-hash: 3c3d3c1777041200bb7ed7a65b6562d62899778c
commit-date: 2019-04-17
host: x86_64-apple-darwin
release: 1.36.0-nightly
LLVM version: 8.0

[phansch]

(This has been fixed in Clippy here), and thanks for the report =) )

[lopopolo]

The one change that clippy suggested I make (twice) was

error: using `clone` on a `Copy` type