Auto merge of #1240 - emilio:virtual-dtor-fix, r=fitzgen

ir: Choose the right mangling for destructors on all codepaths.

Fixes #1133.

Author: bors-servo

bindgen-integration/cpp/Test.cc

@@ -3,6 +3,18 @@
 const int Test::COUNTDOWN[] = { 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 };
 const int* Test::COUNTDOWN_PTR = Test::COUNTDOWN;
 
+unsigned VirtualDestructor::sDestructorCount = 0;
+VirtualDestructor::~VirtualDestructor() {
+  sDestructorCount++;
+}
+
+unsigned InheritsFromVirtualDestructor::sDestructorCount = 0;
+InheritsFromVirtualDestructor::InheritsFromVirtualDestructor() = default;
+
+InheritsFromVirtualDestructor::~InheritsFromVirtualDestructor() {
+  sDestructorCount++;
+}
+
 const int* Test::countdown() {
   return COUNTDOWN;
 }

bindgen-integration/cpp/Test.h

@@ -19,6 +19,19 @@ class ITest {
   virtual void foo() = 0;
 };
 
+class VirtualDestructor {
+public:
+  static unsigned sDestructorCount;
+  virtual ~VirtualDestructor() = 0;
+};
+
+class InheritsFromVirtualDestructor final : public VirtualDestructor {
+public:
+  static unsigned sDestructorCount;
+  InheritsFromVirtualDestructor();
+  ~InheritsFromVirtualDestructor() final;
+};
+
 namespace testing {
 
 typedef Test TypeAlias;

bindgen-integration/src/lib.rs

@@ -260,3 +260,22 @@ fn test_destructors() {
 
     assert!(v, "Should've been restored when going out of scope");
 }
+
+impl Drop for bindings::InheritsFromVirtualDestructor {
+    fn drop(&mut self) {
+        unsafe { bindings::InheritsFromVirtualDestructor_InheritsFromVirtualDestructor_destructor(self) }
+    }
+}
+
+#[test]
+fn test_virtual_dtor() {
+    unsafe {
+        {
+            let b = bindings::InheritsFromVirtualDestructor::new();
+            // Let it go out of scope.
+        }
+
+        assert_eq!(bindings::InheritsFromVirtualDestructor_sDestructorCount, 1);
+        assert_eq!(bindings::VirtualDestructor_sDestructorCount, 1);
+    }
+}

src/codegen/mod.rs

@@ -1991,6 +1991,7 @@ impl MethodCodegen for Method {
             }
         });
 
+        // TODO(emilio): We could generate final stuff at least.
         if self.is_virtual() {
             return; // FIXME
         }

src/ir/function.rs

@@ -245,6 +245,7 @@ pub fn cursor_mangling(
     cursor: &clang::Cursor,
 ) -> Option<String> {
     use clang_sys;
+
     if !ctx.options().enable_mangling {
         return None;
     }
@@ -256,8 +257,14 @@ pub fn cursor_mangling(
         return None;
     }
 
+    let is_destructor = cursor.kind() == clang_sys::CXCursor_Destructor;
     if let Ok(mut manglings) = cursor.cxx_manglings() {
-        if let Some(m) = manglings.pop() {
+        while let Some(m) = manglings.pop() {
+            // Only generate the destructor group 1, see below.
+            if is_destructor && !m.ends_with("D1Ev") {
+                continue;
+            }
+
             return Some(m);
         }
     }
@@ -267,7 +274,7 @@ pub fn cursor_mangling(
         return None;
     }
 
-    if cursor.kind() == clang_sys::CXCursor_Destructor {
+    if is_destructor {
         // With old (3.8-) libclang versions, and the Itanium ABI, clang returns
         // the "destructor group 0" symbol, which means that it'll try to free
         // memory, which definitely isn't what we want.

tests/expectations/tests/virtual_dtor.rs

@@ -1,9 +1,7 @@
 /* automatically generated by rust-bindgen */
 
-
 #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
 
-
 #[repr(C)]
 pub struct nsSlots__bindgen_vtable(::std::os::raw::c_void);
 #[repr(C)]
@@ -30,6 +28,6 @@ impl Default for nsSlots {
     }
 }
 extern "C" {
-    #[link_name = "\u{1}_ZN7nsSlotsD0Ev"]
+    #[link_name = "\u{1}_ZN7nsSlotsD1Ev"]
     pub fn nsSlots_nsSlots_destructor(this: *mut nsSlots);
 }