Removed unsafe optimizations in makeBlock

Author: gershnik

CHANGELOG.md

@@ -9,6 +9,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Boxed values now actually formally conform to relevant protocols (as reported by `confromsToProtocol:`)
 
 ### Fixed
+- Removed some optimizations in `makeBlock` that could cause unexpected block move
+  instead of copy when reusing block pointers
 - Added `<exception>` header include to XCTestUtil.h that is now required on newer Xcodes
 - Making `BoxUtil.h` not rely on `std::filesystem` which is not available on older Apple
   platforms.

include/objc-helpers/BlockUtil.h

@@ -224,20 +224,11 @@ namespace BlockUtil
             operator BlockType() const & noexcept
                 { return castToBlockType();}
 
-            operator BlockType() && noexcept {
-                this->copyCanBeMove = true;
-                return castToBlockType();
-            }
 
             auto get() const & noexcept -> BlockType {
                 return castToBlockType();
             }
 
-            auto get() && noexcept -> BlockType {
-                this->copyCanBeMove = true;
-                return castToBlockType();
-            }
-            
             auto operator()(Args... args) -> Ret {
                 auto & callable = *reinterpret_cast<TypeToCall *>(this->callableBuf);
                 return callable(std::forward<Args>(args)...);
@@ -249,7 +240,8 @@ namespace BlockUtil
                 return dummy;
             }
             friend auto copy(BlockWithCallable && obj) -> BlockType {
-                auto dummy = (BlockType)std::move(obj);
+                obj.copyCanBeMove = true;
+                auto dummy = (BlockType)obj;
                 return dummy;
             }
 #else

test/BlockUtilTest.mm

@@ -95,13 +95,14 @@ int operator()(int i) const {
         })) == 3);
     }
 
+    //TODO: these would ideally have two moves rather than movae+copy but doing so seems impossible
     {
         foo::record.clear();
         int (^block)(int) = makeBlock(foo{});
 
         CHECK(block(6) == 6);
     }
-    CHECK(foo::record == "dmm~~o~");
+    CHECK(foo::record == "dmc~~o~");
 
     {
         foo::record.clear();
@@ -110,7 +111,8 @@ int operator()(int i) const {
 
         CHECK(block2(6) == 6);
     }
-    CHECK(foo::record == "dmm~~o~");
+    CHECK(foo::record == "dmc~~o~");
+    //TODO: end
 
     {
         foo::record.clear();
@@ -153,6 +155,29 @@ int operator()(int i) const {
     CHECK(foo::record == "dc~co~~");
 }
 
+
+static void inner(void (^bl1)(), void (^bl2)()) {
+    bl1();
+    bl2();
+}
+
+
+static void outer(void (^bl)()) {
+    inner(makeBlock([bl] {
+        bl();
+    }), makeBlock([bl] {
+        bl();
+    }));
+}
+
+TEST_CASE("makeBlock nested") {
+    
+    auto p = std::make_shared<int>(1);
+    outer(makeBlock([p] {
+        CHECK(p);
+    }));
+}
+
 @interface MakeStrongCheck : NSObject {
 @public int i;
 }