Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Inadequate recognition of inline namespaces #2950

Closed
dtolnay opened this issue Oct 15, 2024 · 1 comment · Fixed by #2951
Closed

Inadequate recognition of inline namespaces #2950

dtolnay opened this issue Oct 15, 2024 · 1 comment · Fixed by #2951

Comments

@dtolnay
Copy link
Member

dtolnay commented Oct 15, 2024

The following is minimized from Xcode's C++ standard library implementation.

// namespace.h

#pragma once
#define BEGIN_NAMESPACE namespace repro { inline namespace __1 {
#define END_NAMESPACE } }
// main.cc

#include "namespace.h"

BEGIN_NAMESPACE

class duration {};

END_NAMESPACE
$ bindgen main.cc --enable-cxx-namespaces
/* automatically generated by rust-bindgen 0.70.1 */

#[allow(non_snake_case, non_camel_case_types, non_upper_case_globals)]
pub mod root {
    #[allow(unused_imports)]
    use self::super::root;
    pub mod repro {
        #[allow(unused_imports)]
        use self::super::super::root;
        pub mod __1 {
            #[allow(unused_imports)]
            use self::super::super::super::root;
            #[repr(C)]
            #[derive(Debug, Copy, Clone)]
            pub struct duration {
                pub _address: u8,
            }
            #[allow(clippy::unnecessary_operation, clippy::identity_op)]
            const _: () = {
                ["Size of duration"][::std::mem::size_of::<duration>() - 1usize];
                ["Alignment of duration"][::std::mem::align_of::<duration>() - 1usize];
            };
        }
    }
}

Bindgen is incorrectly generating root::repro::__1::duration instead of root::repro::duration. If the 2 defines are placed into main.cc instead of namespace.h, the behavior is different.

#[allow(non_snake_case, non_camel_case_types, non_upper_case_globals)]
pub mod root {
    #[allow(unused_imports)]
    use self::super::root;
    pub mod repro {
        #[allow(unused_imports)]
        use self::super::super::root;
        #[repr(C)]
        #[derive(Debug, Copy, Clone)]
        pub struct duration {
            pub _address: u8,
        }
        #[allow(clippy::unnecessary_operation, clippy::identity_op)]
        const _: () = {
            ["Size of duration"][::std::mem::size_of::<duration>() - 1usize];
            ["Alignment of duration"][::std::mem::align_of::<duration>() - 1usize];
        };
    }
}

As of current master, the logic by which bindgen decides whether a namespace is inline is here:

let mut module_name = None;
let spelling = cursor.spelling();
if !spelling.is_empty() {
module_name = Some(spelling)
}
let mut kind = ModuleKind::Normal;
let mut looking_for_name = false;
for token in cursor.tokens().iter() {
match token.spelling() {
b"inline" => {
debug_assert!(
kind != ModuleKind::Inline,
"Multiple inline keywords?"
);
kind = ModuleKind::Inline;
// When hitting a nested inline namespace we get a spelling
// that looks like ["inline", "foo"]. Deal with it properly.
looking_for_name = true;
}

It should be using https://docs.rs/clang-sys/1.8.1/clang_sys/fn.clang_Cursor_isInlineNamespace.html instead, which I confirmed fixes this bug.

@dtolnay
Copy link
Member Author

dtolnay commented Oct 16, 2024

As a workaround, adding a non-macro-generated namespace repro::inline __1 {} to the top of any affected files (before includes) avoids the bug.

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

Successfully merging a pull request may close this issue.

1 participant