Delete C++ move operations for non-copyable Clone types

cc_bindings_from_rs/generate_bindings/database/adt_core_bindings.rs

@@ -100,7 +100,4 @@ pub enum MoveCtorStyle {
     Default,
     // The type is Default (and Unpin) and can be moved using MemSwap and std::move.
     MemSwap,
-    // The type cannot be moved but has a copy constructor and assignment operator that are used in
-    // lieu of the move constructor and assignment operator.
-    Copy,
 }

cc_bindings_from_rs/generate_bindings/generate_bindings_test.rs

@@ -1747,10 +1747,9 @@ fn test_format_item_struct_with_custom_drop_and_no_default_and_clone(test_src: &
             }
         );
 
-        // Implicit, but not `=default`-ed move constructor and move assignment
-        // operator.
-        assert_cc_not_matches!(main_api.tokens, quote! { TypeUnderTest(TypeUnderTest&&) });
-        assert_cc_not_matches!(main_api.tokens, quote! { operator=(TypeUnderTest&&) });
+        // Explicitly deleted move constructor and move assignment operator.
+        assert_cc_matches!(main_api.tokens, quote! { TypeUnderTest(TypeUnderTest&&) = delete; });
+        assert_cc_matches!(main_api.tokens, quote! { operator=(TypeUnderTest&&) = delete; });
         // No definition of a custom move constructor nor move assignment operator.
         assert_cc_not_matches!(result.cc_details.tokens, quote! { TypeUnderTest(TypeUnderTest&&) },);
         assert_cc_not_matches!(result.cc_details.tokens, quote! { operator=(TypeUnderTest&&) },);

cc_bindings_from_rs/generate_bindings/lib.rs

@@ -1214,8 +1214,6 @@ fn has_move_ctor_and_assignment_operator<'tcx>(
     let is_unpin = self_ty.is_unpin(tcx, typing_env);
     if has_default_ctor && is_unpin {
         Some(MoveCtorStyle::MemSwap)
-    } else if db.has_copy_ctor_and_assignment_operator(def_id, self_ty).is_some() {
-        Some(MoveCtorStyle::Copy)
     } else {
         None
     }
@@ -1234,9 +1232,6 @@ fn generate_move_ctor_and_assignment_operator<'tcx>(
         let adt_cc_name = &core.cc_short_name;
         let qualified_adt_name = &core.cc_fully_qualified_name;
         match db.has_move_ctor_and_assignment_operator(core.def_id, core.self_ty) {
-            // We rely on the copy constructor and assignment operator to handle the move
-            // operations.
-            Some(MoveCtorStyle::Copy) => Ok(ApiSnippets::default()),
             None => {
                 bail!(
                     "C++ move operations are unavailable for this type. See \

cc_bindings_from_rs/test/known_traits/drop/drop.rs

@@ -165,9 +165,6 @@ pub mod drop_impl_with_clone {
             self.field = i;
         }
     }
-    impl DropImplWithClone {
-        pub fn take_by_value(_: Self) {}
-    }
 }
 
 /// Test for `Drop` support when:

cc_bindings_from_rs/test/known_traits/drop/drop_test.cc

@@ -120,9 +120,9 @@ TYPED_TEST(CustomDropWithDefaultTest, ByValue) {
 TEST(DropTest, DropImplWithClone) {
   using TypeUnderTest = drop::drop_impl_with_clone::DropImplWithClone;
   static_assert(!std::is_default_constructible_v<TypeUnderTest>);
-  static_assert(std::is_move_constructible_v<TypeUnderTest>);
+  static_assert(!std::is_move_constructible_v<TypeUnderTest>);
   static_assert(!std::is_trivially_move_constructible_v<TypeUnderTest>);
-  static_assert(std::is_move_assignable_v<TypeUnderTest>);
+  static_assert(!std::is_move_assignable_v<TypeUnderTest>);
   static_assert(!std::is_trivially_move_assignable_v<TypeUnderTest>);
   static_assert(std::is_destructible_v<TypeUnderTest>);
   static_assert(!std::is_trivially_destructible_v<TypeUnderTest>);
@@ -151,9 +151,10 @@ TEST(DropTest, DropImplWithClone) {
 
     // We expect the move to be implemented in terms of copy, so we expect
     // a corresponding increase in clone counters.
-    TypeUnderTest s2(std::move(s));
+    TypeUnderTest s2(s);
     EXPECT_EQ(123, s2.get_int());
-    EXPECT_EQ(123, s.get_int());  // NOLINT(bugprone-use-after-move)
+    EXPECT_EQ(123, s.get_int());  // Since it's no longer movable, we must
+                                  // invoke copy explicitly.
     EXPECT_EQ(1, drop::counters::get_clone_count());
     EXPECT_EQ(0, drop::counters::get_drop_count());
   }
@@ -169,10 +170,10 @@ TEST(DropTest, DropImplWithClone) {
     EXPECT_EQ(0, drop::counters::get_clone_from_count());
     EXPECT_EQ(0, drop::counters::get_drop_count());
 
-    // We expect the move to be implemented in terms of copy, so we expect
-    // a corresponding increase in clone counters.
-    s2 = std::move(s1);
-    EXPECT_EQ(123, s1.get_int());  // NOLINT(bugprone-use-after-move)
+    // We expect the explicit move to trigger a compiler error, so we verify
+    // copy assignment works instead.
+    s2 = s1;
+    EXPECT_EQ(123, s1.get_int());
     EXPECT_EQ(123, s2.get_int());
     EXPECT_EQ(0, drop::counters::get_clone_count());
     EXPECT_EQ(1, drop::counters::get_clone_from_count());
@@ -182,8 +183,8 @@ TEST(DropTest, DropImplWithClone) {
     // (it would lead to aliasing-related UB in Rust when `self` and `source`
     // point to the same object).
 #pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wself-move"
-    s2 = std::move(s2);
+#pragma clang diagnostic ignored "-Wself-assign-overloaded"
+    s2 = s2;
 #pragma clang diagnostic pop
     EXPECT_EQ(123, s1.get_int());
     EXPECT_EQ(123, s2.get_int());
@@ -194,19 +195,6 @@ TEST(DropTest, DropImplWithClone) {
   EXPECT_EQ(0, drop::counters::get_clone_count());
   EXPECT_EQ(1, drop::counters::get_clone_from_count());
   EXPECT_EQ(2, drop::counters::get_drop_count());
-
-  // Testing pass by value.
-  drop::counters::reset_counts();
-  {
-    TypeUnderTest s = TypeUnderTest::create_from_int(123);
-    EXPECT_EQ(0, drop::counters::get_clone_count());
-    // Clones twice: once into the parameter, once again from the thunk to Rust.
-    // Both are destroyed.
-    TypeUnderTest::take_by_value(std::move(s));
-    EXPECT_EQ(2, drop::counters::get_clone_count());
-    EXPECT_EQ(2, drop::counters::get_drop_count());
-  }
-  EXPECT_EQ(3, drop::counters::get_drop_count());
 }
 
 TEST(DropTest, DropImplWithNothingElse) {

cc_bindings_from_rs/test/structs/tuple_structs/tuple_structs_cc_api.h

@@ -41,6 +41,11 @@ struct CRUBIT_INTERNAL_RUST_TYPE(
   // Drop::drop
   ~CloneNoDefault();
 
+  // C++ move operations are unavailable for this type. See
+  // http://crubit.rs/rust/movable_types for an explanation of Rust types that
+  // are C++ movable.
+  CloneNoDefault(CloneNoDefault&&) = delete;
+  ::tuple_structs::CloneNoDefault& operator=(CloneNoDefault&&) = delete;
   // Clone::clone
   CloneNoDefault(const CloneNoDefault&);