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.

Sign up for GitHub

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

Jump to bottom

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 •
edited
Loading

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:

rust-bindgen/bindgen/ir/context.rs

Lines 2267 to 2286 in af26991

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 dtolnay mentioned this issue Oct 16, 2024
Merged
@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Recognize inline namespaces using clang's dedicated API for that dtolnay-contrib/bindgen
1 participant
@dtolnay