Auto merge of #395 - emilio:inline-namespace, r=fitzgen

ir: Handle inline namespaces.

Here's my proposal to handle them,

r? @fitzgen

Author: bors-servo

bindgen/src/options.rs

@@ -121,6 +121,10 @@ pub fn builder_from_flags<I>(args: I)
             Arg::with_name("use-core")
                 .long("use-core")
                 .help("Use types from Rust core instead of std."),
+            Arg::with_name("conservative-inline-namespaces")
+                .long("conservative-inline-namespaces")
+                .help("Conservatively generate inline namespaces to avoid name \
+                       conflicts."),
             Arg::with_name("use-msvc-mangling")
                 .long("use-msvc-mangling")
                 .help("MSVC C++ ABI mangling. DEPRECATED: Has no effect."),
@@ -268,6 +272,10 @@ pub fn builder_from_flags<I>(args: I)
         builder = builder.use_core();
     }
 
+    if matches.is_present("conservative-inline-namespaces") {
+        builder = builder.conservative_inline_namespaces();
+    }
+
     if let Some(whitelist) = matches.values_of("whitelist-function") {
         for regex in whitelist {
             builder = builder.whitelisted_function(regex);

libbindgen/src/codegen/mod.rs

@@ -329,7 +329,8 @@ impl CodeGenerator for Module {
             }
         };
 
-        if !ctx.options().enable_cxx_namespaces {
+        if !ctx.options().enable_cxx_namespaces ||
+            (self.is_inline() && !ctx.options().conservative_inline_namespaces) {
             codegen_self(result, &mut false);
             return;
         }

libbindgen/src/ir/context.rs

@@ -14,7 +14,7 @@ use super::derive::{CanDeriveCopy, CanDeriveDebug};
 use super::int::IntKind;
 use super::item::{Item, ItemCanonicalPath};
 use super::item_kind::ItemKind;
-use super::module::Module;
+use super::module::{Module, ModuleKind};
 use super::ty::{FloatKind, Type, TypeKind};
 use super::type_collector::{ItemSet, TypeCollector};
 use syntax::ast::Ident;
@@ -545,7 +545,7 @@ impl<'ctx> BindgenContext<'ctx> {
     }
 
     fn build_root_module(id: ItemId) -> Item {
-        let module = Module::new(Some("root".into()));
+        let module = Module::new(Some("root".into()), ModuleKind::Normal);
         Item::new(id, None, None, id, ItemKind::Module(module))
     }
 
@@ -955,31 +955,64 @@ impl<'ctx> BindgenContext<'ctx> {
         &self.options
     }
 
+    /// Tokenizes a namespace cursor in order to get the name and kind of the
+    /// namespace,
+    fn tokenize_namespace(&self,
+                          cursor: &clang::Cursor)
+                          -> (Option<String>, ModuleKind) {
+        assert_eq!(cursor.kind(), ::clang_sys::CXCursor_Namespace,
+                   "Be a nice person");
+        let tokens = match self.translation_unit.tokens(&cursor) {
+            Some(tokens) => tokens,
+            None => return (None, ModuleKind::Normal),
+        };
+
+        let mut iter = tokens.iter();
+        let mut kind = ModuleKind::Normal;
+        let mut found_namespace_keyword = false;
+        let mut module_name = None;
+        while let Some(token) = iter.next() {
+            match &*token.spelling {
+                "inline" => {
+                    assert!(!found_namespace_keyword);
+                    assert!(kind != ModuleKind::Inline);
+                    kind = ModuleKind::Inline;
+                }
+                "namespace" => {
+                    found_namespace_keyword = true;
+                }
+                "{" => {
+                    assert!(found_namespace_keyword);
+                    break;
+                }
+                name if found_namespace_keyword => {
+                    module_name = Some(name.to_owned());
+                    break;
+                }
+                _ => {
+                    panic!("Unknown token while processing namespace: {:?}",
+                           token);
+                }
+            }
+        };
+
+        (module_name, kind)
+    }
+
     /// Given a CXCursor_Namespace cursor, return the item id of the
     /// corresponding module, or create one on the fly.
     pub fn module(&mut self, cursor: clang::Cursor) -> ItemId {
         use clang_sys::*;
-        assert!(cursor.kind() == CXCursor_Namespace, "Be a nice person");
+        assert_eq!(cursor.kind(), CXCursor_Namespace, "Be a nice person");
         let cursor = cursor.canonical();
         if let Some(id) = self.modules.get(&cursor) {
             return *id;
         }
 
-        let module_id = self.next_item_id();
-        let module_name = self.translation_unit
-            .tokens(&cursor)
-            .and_then(|tokens| {
-                if tokens.len() <= 1 {
-                    None
-                } else {
-                    match &*tokens[1].spelling {
-                        "{" => None,
-                        s => Some(s.to_owned()),
-                    }
-                }
-            });
+        let (module_name, kind) = self.tokenize_namespace(&cursor);
 
-        let module = Module::new(module_name);
+        let module_id = self.next_item_id();
+        let module = Module::new(module_name, kind);
         let module = Item::new(module_id,
                                None,
                                None,

libbindgen/src/ir/item.rs

@@ -853,6 +853,15 @@ impl Item {
         format!("id_{}", self.id().as_usize())
     }
 
+    /// Get a reference to this item's `Module`, or `None` if this is not a
+    /// `Module` item.
+    pub fn as_module(&self) -> Option<&Module> {
+        match self.kind {
+            ItemKind::Module(ref module) => Some(module),
+            _ => None,
+        }
+    }
+
     /// Get a mutable reference to this item's `Module`, or `None` if this is
     /// not a `Module` item.
     pub fn as_module_mut(&mut self) -> Option<&mut Module> {
@@ -1304,7 +1313,13 @@ impl ItemCanonicalPath for Item {
         let mut path: Vec<_> = target.ancestors(ctx)
             .chain(iter::once(ctx.root_module()))
             .map(|id| ctx.resolve_item(id))
-            .filter(|item| item.is_module() || item.id() == target.id())
+            .filter(|item| {
+                item.id() == target.id() ||
+                item.as_module().map_or(false, |module| {
+                    !module.is_inline() ||
+                        ctx.options().conservative_inline_namespaces
+                })
+            })
             .map(|item| {
                 ctx.resolve_item(item.name_target(ctx))
                     .name(ctx)

libbindgen/src/ir/module.rs

@@ -5,20 +5,32 @@ use parse::{ClangSubItemParser, ParseError, ParseResult};
 use parse_one;
 use super::context::{BindgenContext, ItemId};
 
+/// Whether this module is inline or not.
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub enum ModuleKind {
+    /// This module is not inline.
+    Normal,
+    /// This module is inline, as in `inline namespace foo {}`.
+    Inline,
+}
+
 /// A module, as in, a C++ namespace.
 #[derive(Clone, Debug)]
 pub struct Module {
     /// The name of the module, or none if it's anonymous.
     name: Option<String>,
+    /// The kind of module this is.
+    kind: ModuleKind,
     /// The children of this module, just here for convenience.
     children_ids: Vec<ItemId>,
 }
 
 impl Module {
     /// Construct a new `Module`.
-    pub fn new(name: Option<String>) -> Self {
+    pub fn new(name: Option<String>, kind: ModuleKind) -> Self {
         Module {
             name: name,
+            kind: kind,
             children_ids: vec![],
         }
     }
@@ -37,6 +49,11 @@ impl Module {
     pub fn children(&self) -> &[ItemId] {
         &self.children_ids
     }
+
+    /// Whether this namespace is inline.
+    pub fn is_inline(&self) -> bool {
+        self.kind == ModuleKind::Inline
+    }
 }
 
 impl ClangSubItemParser for Module {

libbindgen/src/lib.rs

@@ -305,6 +305,39 @@ impl Builder {
         self
     }
 
+    /// Treat inline namespaces conservatively.
+    ///
+    /// This is tricky, because in C++ is technically legal to override an item
+    /// defined in an inline namespace:
+    ///
+    /// ```cpp
+    /// inline namespace foo {
+    ///     using Bar = int;
+    /// }
+    /// using Bar = long;
+    /// ```
+    ///
+    /// Even though referencing `Bar` is a compiler error.
+    ///
+    /// We want to support this (arguably esoteric) use case, but we don't want
+    /// to make the rest of bindgen users pay an usability penalty for that.
+    ///
+    /// To support this, we need to keep all the inline namespaces around, but
+    /// then bindgen usage is a bit more difficult, because you cannot
+    /// reference, e.g., `std::string` (you'd need to use the proper inline
+    /// namespace).
+    ///
+    /// We could complicate a lot of the logic to detect name collisions, and if
+    /// not detected generate a `pub use inline_ns::*` or something like that.
+    ///
+    /// That's probably something we can do if we see this option is needed in a
+    /// lot of cases, to improve it's usability, but my guess is that this is
+    /// not going to be too useful.
+    pub fn conservative_inline_namespaces(mut self) -> Builder {
+        self.options.conservative_inline_namespaces = true;
+        self
+    }
+
     /// Ignore functions.
     pub fn ignore_functions(mut self) -> Builder {
         self.options.codegen_config.functions = false;
@@ -448,6 +481,11 @@ pub struct BindgenOptions {
     /// Which kind of items should we generate? By default, we'll generate all
     /// of them.
     pub codegen_config: CodegenConfig,
+
+    /// Whether to treat inline namespaces conservatively.
+    ///
+    /// See the builder method description for more details.
+    pub conservative_inline_namespaces: bool,
 }
 
 impl BindgenOptions {
@@ -489,6 +527,7 @@ impl Default for BindgenOptions {
             dummy_uses: None,
             type_chooser: None,
             codegen_config: CodegenConfig::all(),
+            conservative_inline_namespaces: false,
         }
     }
 }

libbindgen/tests/expectations/tests/inline_namespace.rs

@@ -0,0 +1,28 @@
+/* automatically generated by rust-bindgen */
+
+
+#![allow(non_snake_case)]
+
+
+pub mod root {
+    #[allow(unused_imports)]
+    use self::super::root;
+    pub mod foo {
+        #[allow(unused_imports)]
+        use self::super::super::root;
+        pub type Ty = ::std::os::raw::c_int;
+    }
+    #[repr(C)]
+    #[derive(Debug, Copy)]
+    pub struct Bar {
+        pub baz: root::foo::Ty,
+    }
+    #[test]
+    fn bindgen_test_layout_Bar() {
+        assert_eq!(::std::mem::size_of::<Bar>() , 4usize);
+        assert_eq!(::std::mem::align_of::<Bar>() , 4usize);
+    }
+    impl Clone for Bar {
+        fn clone(&self) -> Self { *self }
+    }
+}

libbindgen/tests/expectations/tests/inline_namespace_conservative.rs

@@ -0,0 +1,33 @@
+/* automatically generated by rust-bindgen */
+
+
+#![allow(non_snake_case)]
+
+
+pub mod root {
+    #[allow(unused_imports)]
+    use self::super::root;
+    pub mod foo {
+        #[allow(unused_imports)]
+        use self::super::super::root;
+        pub mod bar {
+            #[allow(unused_imports)]
+            use self::super::super::super::root;
+            pub type Ty = ::std::os::raw::c_int;
+        }
+        pub type Ty = ::std::os::raw::c_longlong;
+    }
+    #[repr(C)]
+    #[derive(Debug, Copy)]
+    pub struct Bar {
+        pub baz: root::foo::bar::Ty,
+    }
+    #[test]
+    fn bindgen_test_layout_Bar() {
+        assert_eq!(::std::mem::size_of::<Bar>() , 4usize);
+        assert_eq!(::std::mem::align_of::<Bar>() , 4usize);
+    }
+    impl Clone for Bar {
+        fn clone(&self) -> Self { *self }
+    }
+}

libbindgen/tests/expectations/tests/inline_namespace_whitelist.rs

@@ -0,0 +1,15 @@
+/* automatically generated by rust-bindgen */
+
+
+#![allow(non_snake_case)]
+
+
+pub mod root {
+    #[allow(unused_imports)]
+    use self::super::root;
+    pub mod std {
+        #[allow(unused_imports)]
+        use self::super::super::root;
+        pub type string = *const ::std::os::raw::c_char;
+    }
+}

libbindgen/tests/headers/inline_namespace.hpp

@@ -0,0 +1,11 @@
+// bindgen-flags: --enable-cxx-namespaces -- -std=c++11
+
+namespace foo {
+  inline namespace bar {
+    using Ty = int;
+  };
+};
+
+class Bar {
+  foo::Ty baz;
+};

libbindgen/tests/headers/inline_namespace_conservative.hpp

@@ -0,0 +1,12 @@
+// bindgen-flags: --enable-cxx-namespaces --conservative-inline-namespaces -- -std=c++11
+
+namespace foo {
+  inline namespace bar {
+    using Ty = int;
+  };
+  using Ty = long long;
+};
+
+class Bar {
+  foo::bar::Ty baz;
+};

libbindgen/tests/headers/inline_namespace_whitelist.hpp

@@ -0,0 +1,7 @@
+// bindgen-flags: --enable-cxx-namespaces --whitelist-type=std::string -- -std=c++11
+
+namespace std {
+  inline namespace bar {
+    using string = const char*;
+  };
+};