Pointer: make copy_from/copy_to/move_from/move_to work well with unio…

…ns of pointers. Fixes #3775
crystal-lang · Dec 26, 2016 · ab4a68e · ab4a68e
1 parent 4cc34b6
commit ab4a68e
Showing 3 changed files with 79 additions and 28 deletions.
diff --git a/spec/std/array_spec.cr b/spec/std/array_spec.cr
@@ -396,6 +396,12 @@ describe "Array" do
       a.concat(1..4)
       a.@capacity.should eq(6)
     end
+
+    it "concats a union of arrays" do
+      a = [1, '2']
+      a.concat([3] || ['4'])
+      a.should eq([1, '2', 3])
+    end
   end
 
   describe "delete" do

diff --git a/spec/std/pointer_spec.cr b/spec/std/pointer_spec.cr
@@ -32,7 +32,7 @@ describe "Pointer" do
       p2 = Pointer.malloc(4) { 0 }
       p2.copy_from(p1, 4)
       4.times do |i|
-        p2[0].should eq(p1[0])
+        p2[i].should eq(p1[i])
       end
     end
 
@@ -42,6 +42,14 @@ describe "Pointer" do
         p1.copy_from(p1, -1)
       end
     end
+
+    it "copies from union of pointers" do
+      p1 = Pointer.malloc(4, 1)
+      p2 = Pointer.malloc(4, 1.5)
+      p3 = Pointer.malloc(4, 0 || 0.0)
+      p3.copy_from(p1 || p2, 4)
+      4.times { |i| p3[i].should eq(p1[i]) }
+    end
   end
 
   describe "copy_to" do
@@ -50,7 +58,7 @@ describe "Pointer" do
       p2 = Pointer.malloc(4) { 0 }
       p1.copy_to(p2, 4)
       4.times do |i|
-        p2[0].should eq(p1[0])
+        p2[i].should eq(p1[i])
       end
     end
 
@@ -60,6 +68,14 @@ describe "Pointer" do
         p1.copy_to(p1, -1)
       end
     end
+
+    it "copies to union of pointers" do
+      p1 = Pointer.malloc(4, 1)
+      p2 = Pointer.malloc(4, 0 || 1.5)
+      p3 = Pointer.malloc(4, 0 || 'a')
+      p1.copy_to(p2 || p3, 4)
+      4.times { |i| p2[i].should eq(p1[i]) }
+    end
   end
 
   describe "move_from" do
@@ -87,6 +103,14 @@ describe "Pointer" do
         p1.move_from(p1, -1)
       end
     end
+
+    it "moves from union of pointers" do
+      p1 = Pointer.malloc(4, 1)
+      p2 = Pointer.malloc(4, 1.5)
+      p3 = Pointer.malloc(4, 0 || 0.0)
+      p3.move_from(p1 || p2, 4)
+      4.times { |i| p3[i].should eq(p1[i]) }
+    end
   end
 
   describe "move_to" do
@@ -114,6 +138,14 @@ describe "Pointer" do
         p1.move_to(p1, -1)
       end
     end
+
+    it "moves to union of pointers" do
+      p1 = Pointer.malloc(4, 1)
+      p2 = Pointer.malloc(4, 0 || 1.5)
+      p3 = Pointer.malloc(4, 0 || 'a')
+      p1.move_to(p2 || p3, 4)
+      4.times { |i| p2[i].should eq(p1[i]) }
+    end
   end
 
   describe "memcmp" do

diff --git a/src/pointer.cr b/src/pointer.cr
@@ -146,16 +146,7 @@ struct Pointer(T)
   # ptr1[3] # => 4
   # ```
   def copy_from(source : Pointer(T), count : Int)
-    raise ArgumentError.new("negative count") if count < 0
-
-    if self.class == source.class
-      Intrinsics.memcpy(self.as(Void*), source.as(Void*), (count * sizeof(T)).to_u32, 0_u32, false)
-    else
-      while (count -= 1) >= 0
-        self[count] = source[count]
-      end
-    end
-    self
+    source.copy_to(self, count)
   end
 
   # :nodoc:
@@ -186,7 +177,7 @@ struct Pointer(T)
   # ptr2[3] # => 14
   # ```
   def copy_to(target : Pointer, count : Int)
-    target.copy_from(self, count)
+    target.copy_from_impl(self, count)
   end
 
   # Copies *count* elements from *source* into *self*.
@@ -207,20 +198,7 @@ struct Pointer(T)
   # ptr1[3] # => 3
   # ```
   def move_from(source : Pointer(T), count : Int)
-    raise ArgumentError.new("negative count") if count < 0
-
-    if self.class == source.class
-      Intrinsics.memmove(self.as(Void*), source.as(Void*), (count * sizeof(T)).to_u32, 0_u32, false)
-    else
-      if source.address < address
-        copy_from source, count
-      else
-        count.times do |i|
-          self[i] = source[i]
-        end
-      end
-    end
-    self
+    source.move_to(self, count)
   end
 
   # :nodoc:
@@ -250,7 +228,42 @@ struct Pointer(T)
   # ptr1[3] # => 3
   # ```
   def move_to(target : Pointer, count : Int)
-    target.move_from(self, count)
+    target.move_from_impl(self, count)
+  end
+
+  # We use separate method in which we make sure that `source`
+  # is never a union of pointers. This is guaranteed because both
+  # copy_from/move_from/copy_to/move_to reverse self and caller,
+  # and so if either self or the arguments are unions a dispatch
+  # will happen and unions will disappear.
+  protected def copy_from_impl(source : Pointer(T), count : Int)
+    raise ArgumentError.new("negative count") if count < 0
+
+    if self.class == source.class
+      Intrinsics.memcpy(self.as(Void*), source.as(Void*), (count * sizeof(T)).to_u32, 0_u32, false)
+    else
+      while (count -= 1) >= 0
+        self[count] = source[count]
+      end
+    end
+    self
+  end
+
+  protected def move_from_impl(source : Pointer(T), count : Int)
+    raise ArgumentError.new("negative count") if count < 0
+
+    if self.class == source.class
+      Intrinsics.memmove(self.as(Void*), source.as(Void*), (count * sizeof(T)).to_u32, 0_u32, false)
+    else
+      if source.address < address
+        copy_from source, count
+      else
+        count.times do |i|
+          self[i] = source[i]
+        end
+      end
+    end
+    self
   end
 
   # Compares *count* elements from this pointer and *other*, byte by byte.