gen: Move has_destructor logic to types.rs

3 files changed, 49 insertions, 36 deletions

diff --git a/src/gen.rs b/src/gen.rs
index 3d1202d2..24933abb 100644
--- a/src/gen.rs
+++ b/src/gen.rs
@@ -820,7 +820,7 @@ fn comp_to_rs(ctx: &mut GenCtx, name: &str, ci: CompInfo)
     }
 }
 
-fn comp_attrs(ctx: &GenCtx, ci: &CompInfo, name: &str, has_destructor: bool, extra: &mut Vec<P<ast::Item>>) -> Vec<ast::Attribute> {
+fn comp_attrs(ctx: &GenCtx, ci: &CompInfo, name: &str, extra: &mut Vec<P<ast::Item>>) -> Vec<ast::Attribute> {
     let mut attrs = mk_doc_attr(ctx, &ci.comment);
     attrs.push(mk_repr_attr(ctx, &ci.layout));
     let mut derives = vec![];
@@ -829,7 +829,7 @@ fn comp_attrs(ctx: &GenCtx, ci: &CompInfo, name: &str, has_destructor: bool, ext
         derives.push("Debug");
     }
 
-    if has_destructor {
+    if ci.has_destructor() {
         for attr in ctx.options.dtor_attrs.iter() {
             let attr = ctx.ext_cx.ident_of(attr);
             attrs.push(quote_attr!(&ctx.ext_cx, #[$attr]));
@@ -963,7 +963,6 @@ fn cstruct_to_rs(ctx: &mut GenCtx, name: &str, ci: CompInfo) -> Vec<P<ast::Item>
 
     let mut anon_enum_count = 0;
     let mut setters = vec!();
-    let mut has_destructor = ci.has_destructor;
     let mut template_args_used = vec![false; template_args.len()];
 
     for m in members.iter() {
@@ -992,10 +991,6 @@ fn cstruct_to_rs(ctx: &mut GenCtx, name: &str, ci: CompInfo) -> Vec<P<ast::Item>
         let (opt_rc_c, opt_f) = comp_fields(m);
 
         if let Some(f) = opt_f {
-            if cty_has_destructor(&f.ty) {
-                has_destructor = true;
-            }
-
             let (f_name, f_ty) = match f.bitfields {
                 Some(ref v) => {
                     bitfields += 1;
@@ -1022,9 +1017,6 @@ fn cstruct_to_rs(ctx: &mut GenCtx, name: &str, ci: CompInfo) -> Vec<P<ast::Item>
 
             let is_translatable = cty_is_translatable(&f_ty);
             if !is_translatable || f_ty.is_opaque() {
-                // Be conservative here and assume it might have a
-                // destructor or some other serious constraint.
-                has_destructor = true;
                 if !is_translatable {
                     println!("{}::{} not translatable, void: {}", ci.name, f.name, f_ty == TVoid);
                 }
@@ -1180,7 +1172,7 @@ fn cstruct_to_rs(ctx: &mut GenCtx, name: &str, ci: CompInfo) -> Vec<P<ast::Item>
         }
     );
 
-    let attrs = comp_attrs(&ctx, &ci, name, has_destructor, &mut extra);
+    let attrs = comp_attrs(&ctx, &ci, name, &mut extra);
 
     let struct_def = ast::Item {
         ident: ctx.ext_cx.ident_of(&id),
@@ -1342,7 +1334,7 @@ fn cunion_to_rs(ctx: &mut GenCtx, name: &str, ci: CompInfo) -> Vec<P<ast::Item>>
 
     let union_id = rust_type_id(ctx, name);
 
-    let union_attrs = comp_attrs(&ctx, &ci, name, ci.has_destructor, &mut extra);
+    let union_attrs = comp_attrs(&ctx, &ci, name, &mut extra);
 
     extra.push(mk_test_fn(ctx, &name, &layout));
 
@@ -2067,30 +2059,6 @@ fn cty_is_translatable(ty: &Type) -> bool {
     }
 }
 
-fn cty_has_destructor(ty: &Type) -> bool {
-    match ty {
-        &TArray(ref t, _, _) => {
-            cty_has_destructor(&**t)
-        }
-        &TComp(ref ci) => {
-            let c = ci.borrow();
-            if c.has_destructor || c.members.iter().any(|f| match f {
-                &CompMember::Field(ref f) |
-                &CompMember::CompField(_, ref f) =>
-                    cty_has_destructor(&f.ty),
-                _ => false,
-            }) {
-                return true;
-            }
-            c.ref_template.is_some()
-        },
-        &TNamed(ref ti) => {
-            cty_has_destructor(&ti.borrow().ty)
-        },
-        _ => false,
-    }
-}
-
 fn mk_ty(ctx: &GenCtx, global: bool, segments: &[String]) -> ast::Ty {
     mk_ty_args(ctx, global, segments, vec!())
 }
diff --git a/src/parser.rs b/src/parser.rs
index eace81dd..7a1f43d2 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -123,6 +123,7 @@ fn decl_name(ctx: &mut ClangParserCtx, cursor: &Cursor) -> Global {
                 };
 
                 let mut module_id = ctx.current_module_id;
+                let mut has_dtor = false;
 
                 // If it's an instantiation of another template,
                 // find the canonical declaration to find the module
@@ -133,6 +134,7 @@ fn decl_name(ctx: &mut ClangParserCtx, cursor: &Cursor) -> Global {
                         GComp(ref ci) |
                         GCompDecl(ref ci) => {
                             opaque |= ci.borrow().opaque;
+                            has_dtor |= ci.borrow().has_destructor;
                             module_id = ci.borrow().module_id;
                         }
                         _ => {}
@@ -143,6 +145,7 @@ fn decl_name(ctx: &mut ClangParserCtx, cursor: &Cursor) -> Global {
                 ci.opaque = opaque;
                 ci.hide = hide;
                 ci.args = args;
+                ci.has_destructor = has_dtor;
                 ci.has_non_type_template_params = has_non_type_template_params;
 
                 let ci = Rc::new(RefCell::new(ci));
@@ -820,6 +823,12 @@ fn visit_composite(cursor: &Cursor, parent: &Cursor,
         }
         CXCursor_Destructor => {
             ci.has_destructor = true;
+            if let Some(ref t) = ci.ref_template {
+                match *t {
+                    TComp(ref parent_ci) => parent_ci.borrow_mut().has_destructor = true,
+                    _ => {}
+                }
+            }
         }
         CXCursor_NonTypeTemplateParameter => {
             log_err_warn(ctx, &format!("warning: Non-type template parameter in composite member could affect layout: `{}` (kind {}) in `{}` ({})",
diff --git a/src/types.rs b/src/types.rs
index c01f5b3e..96b03c17 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -220,6 +220,27 @@ impl Type {
             _ => false,
         }
     }
+
+    // If a type is opaque we conservatively
+    // assume it has destructor
+    pub fn has_destructor(&self) -> bool {
+        self.is_opaque() || match *self {
+            TArray(ref t, _, _) => t.has_destructor(),
+            TNamed(ref ti) => ti.borrow().ty.has_destructor(),
+            TComp(ref ci) => ci.borrow().has_destructor(),
+            _ => false,
+        }
+    }
+
+    pub fn is_translatable(&self) -> bool {
+        match *self {
+            TVoid => false,
+            TArray(ref t, _, _) => t.is_translatable(),
+            TNamed(ref ti) => ti.borrow().ty.is_translatable(),
+            TComp(ref ci) => ci.borrow().is_translatable(),
+            _ => true,
+        }
+    }
 }
 
 #[derive(Copy, Clone, Debug, PartialEq)]
@@ -411,6 +432,21 @@ impl CompInfo {
         }
         self.opaque
     }
+
+    pub fn has_destructor(&self) -> bool {
+        self.has_destructor ||
+        self.ref_template.as_ref().map(|t| t.has_destructor()).unwrap_or(false) ||
+        self.members.iter().any(|m| match *m {
+            CompMember::Field(ref f) |
+            CompMember::CompField(_, ref f)
+                => f.ty.is_opaque() || f.ty.has_destructor() || !f.ty.is_translatable(),
+            _ => false,
+        })
+    }
+
+    pub fn is_translatable(&self) -> bool {
+        self.args.iter().all(|t| t.is_translatable()) && !self.has_non_type_template_params
+    }
 }
 
 impl fmt::Debug for CompInfo {