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

clippy::redundant_closure warning on cxx::bridge macro in cxx 1.0.43+ #843

Closed
Minoru opened this issue Apr 20, 2021 · 3 comments · Fixed by #845
Closed

clippy::redundant_closure warning on cxx::bridge macro in cxx 1.0.43+ #843

Minoru opened this issue Apr 20, 2021 · 3 comments · Fixed by #845

Comments

@Minoru
Copy link

Minoru commented Apr 20, 2021

Hi! Starting with cxx 1.0.43, I'm getting this Clippy warning for my project (Rust 1.48 and Rust 1.51):

error: redundant closure found
 --> rust/libnewsboat-ffi/src/logger.rs:3:1
  |
3 | #[cxx::bridge(namespace = "newsboat::Logger")]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |

I couldn't narrow this down to anything in particular; the only way to make the warning go away is to make the bridge mod empty. It's also not related to the (namespace = …) thing, because removing it doesn't make the warning go away.

The warning advises to "run with -Z macro-backtrace for more info", but doing so makes the warning go away (I tried both cargo +nightly … and rustup override set nightly).

The result of macro expansion:

$ cargo +nightly rustc --package libnewsboat-ffi --profile=check -- -Zunstable-options --pretty=expanded
#![feature(prelude_import)]
// Each function in this crate is used in a single place: a corresponding C++ wrapper. It doesn't
// make sense to document these functions because they are basically an internal detail and don't
// stand on their own.
#![allow(clippy :: missing_safety_doc)]
// This lint is nitpicky, I don't think it's really important how the literals are written.
#![allow(clippy :: unreadable_literal)]
#[prelude_import]
use std::prelude::rust_2018::*;
#[macro_use]
extern crate std;

use libc::c_char;
use std::ffi::CString;
use std::panic::{catch_unwind, UnwindSafe};
use std::process::abort;

// SNIP: some unrelated modules

pub mod logger {
    use libnewsboat::logger;
    #[deny(improper_ctypes, improper_ctypes_definitions)]
    #[allow(non_camel_case_types, non_snake_case, clippy ::
            upper_case_acronyms)]
    mod ffi {
        #[repr(transparent)]
        pub struct Level {
            #[allow(missing_docs)]
            pub repr: u8,
        }
        impl ::core::marker::StructuralEq for Level { }
        #[automatically_derived]
        #[allow(unused_qualifications)]
        impl ::core::cmp::Eq for Level {
            #[inline]
            #[doc(hidden)]
            fn assert_receiver_is_total_eq(&self) -> () {
                { let _: ::core::cmp::AssertParamIsEq<u8>; }
            }
        }
        impl ::core::marker::StructuralPartialEq for Level { }
        #[automatically_derived]
        #[allow(unused_qualifications)]
        impl ::core::cmp::PartialEq for Level {
            #[inline]
            fn eq(&self, other: &Level) -> bool {
                match *other {
                    Level { repr: ref __self_1_0 } =>
                    match *self {
                        Level { repr: ref __self_0_0 } =>
                        (*__self_0_0) == (*__self_1_0),
                    },
                }
            }
            #[inline]
            fn ne(&self, other: &Level) -> bool {
                match *other {
                    Level { repr: ref __self_1_0 } =>
                    match *self {
                        Level { repr: ref __self_0_0 } =>
                        (*__self_0_0) != (*__self_1_0),
                    },
                }
            }
        }
        #[allow(non_upper_case_globals)]
        impl Level {
            pub const USERERROR: Self = Level{repr: 1,};
            pub const CRITICAL: Self = Level{repr: 2,};
            pub const ERROR: Self = Level{repr: 3,};
            pub const WARN: Self = Level{repr: 4,};
            pub const INFO: Self = Level{repr: 5,};
            pub const DEBUG: Self = Level{repr: 6,};
        }
        unsafe impl ::cxx::ExternType for Level {
            #[doc(hidden)]
            type Id =
             (::cxx::n, ::cxx::e, ::cxx::w, ::cxx::s, ::cxx::b, ::cxx::o,
              ::cxx::a, ::cxx::t, (), ::cxx::L, ::cxx::o, ::cxx::g, ::cxx::g,
              ::cxx::e, ::cxx::r, (), ::cxx::L, ::cxx::e, ::cxx::v, ::cxx::e,
              ::cxx::l);
            type Kind = ::cxx::kind::Trivial;
        }
        impl ::std::marker::Copy for Level { }
        impl ::std::clone::Clone for Level {
            fn clone(&self) -> Self { *self }
        }
        #[doc(hidden)]
        const _: () =
            {
                #[doc(hidden)]
                #[export_name = "newsboat$Logger$cxxbridge1$unset_loglevel"]
                unsafe extern "C" fn __unset_loglevel() {
                    let __fn = "newsboat::logger::ffi::unset_loglevel";
                    fn __unset_loglevel() { super::unset_loglevel() }
                    ::cxx::private::catch_unwind(__fn,
                                                 move || __unset_loglevel())
                }
                #[doc(hidden)]
                #[export_name = "newsboat$Logger$cxxbridge1$set_logfile"]
                unsafe extern "C" fn __set_logfile(logfile:
                                                       ::cxx::private::RustStr) {
                    let __fn = "newsboat::logger::ffi::set_logfile";
                    fn __set_logfile(logfile: &str) {
                        super::set_logfile(logfile)
                    }
                    ::cxx::private::catch_unwind(__fn,
                                                 move ||
                                                     __set_logfile(logfile.as_str()))
                }
                #[doc(hidden)]
                #[export_name = "newsboat$Logger$cxxbridge1$get_loglevel"]
                unsafe extern "C" fn __get_loglevel() -> i64 {
                    let __fn = "newsboat::logger::ffi::get_loglevel";
                    fn __get_loglevel() -> i64 { super::get_loglevel() }
                    ::cxx::private::catch_unwind(__fn,
                                                 move || __get_loglevel())
                }
                #[doc(hidden)]
                #[export_name = "newsboat$Logger$cxxbridge1$set_loglevel"]
                unsafe extern "C" fn __set_loglevel(level: Level) {
                    let __fn = "newsboat::logger::ffi::set_loglevel";
                    fn __set_loglevel(level: Level) {
                        super::set_loglevel(level)
                    }
                    ::cxx::private::catch_unwind(__fn,
                                                 move ||
                                                     __set_loglevel(level))
                }
                #[doc(hidden)]
                #[export_name = "newsboat$Logger$cxxbridge1$log_internal"]
                unsafe extern "C" fn __log_internal(level: Level,
                                                    message:
                                                        ::cxx::private::RustStr) {
                    let __fn = "newsboat::logger::ffi::log_internal";
                    fn __log_internal(level: Level, message: &str) {
                        super::log_internal(level, message)
                    }
                    ::cxx::private::catch_unwind(__fn,
                                                 move ||
                                                     __log_internal(level,
                                                                    message.as_str()))
                }
                #[doc(hidden)]
                #[export_name =
                  "newsboat$Logger$cxxbridge1$set_user_error_logfile"]
                unsafe extern "C" fn __set_user_error_logfile(user_error_logfile:
                                                                  ::cxx::private::RustStr) {
                    let __fn =
                        "newsboat::logger::ffi::set_user_error_logfile";
                    fn __set_user_error_logfile(user_error_logfile: &str) {
                        super::set_user_error_logfile(user_error_logfile)
                    }
                    ::cxx::private::catch_unwind(__fn,
                                                 move ||
                                                     __set_user_error_logfile(user_error_logfile.as_str()))
                }
            };
    }
    fn ffi_level_to_log_level(level: ffi::Level) -> logger::Level {
        match level {
            ffi::Level::USERERROR => logger::Level::UserError,
            ffi::Level::CRITICAL => logger::Level::Critical,
            ffi::Level::ERROR => logger::Level::Error,
            ffi::Level::WARN => logger::Level::Warn,
            ffi::Level::INFO => logger::Level::Info,
            ffi::Level::DEBUG => logger::Level::Debug,
            _ => { ::std::rt::begin_panic("Unknown log level") }
        }
    }
    fn unset_loglevel() { logger::get_instance().unset_loglevel(); }
    fn set_logfile(logfile: &str) {
        logger::get_instance().set_logfile(logfile);
    }
    fn get_loglevel() -> i64 { logger::get_instance().get_loglevel() as i64 }
    fn set_loglevel(level: ffi::Level) {
        let level = ffi_level_to_log_level(level);
        logger::get_instance().set_loglevel(level);
    }
    fn log_internal(level: ffi::Level, message: &str) {
        let level = ffi_level_to_log_level(level);
        logger::get_instance().log(level, message);
    }
    fn set_user_error_logfile(user_error_logfile: &str) {
        logger::get_instance().set_user_error_logfile(user_error_logfile);
    }
}

// SNIP: a few more modules

The only closures that I can see are the ones passed to ::cxx::private::catch_unwind, but these were present even in 1.0.41 (which didn't produce the warning).

I'm okay with suppressing this warning on my end, but I thought I'd let you know about this and see if you have some insights.

If you'd like to experiment locally:

@dtolnay
Copy link
Owner

dtolnay commented Apr 20, 2021

This looks like a Clippy bug in the 1.51.0 toolchain clippy. If you build with a newer clippy (cargo +beta clippy) then there is no warning.

@dtolnay
Copy link
Owner

dtolnay commented Apr 20, 2021

I published cxx 1.0.46 with a workaround.

@Minoru
Copy link
Author

Minoru commented Apr 21, 2021

Thank you!

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