Merge pull request #349 from pllab/clone_update

Updating `clone_wire` to be safe when used on same block.

Author: mdko

pyrtl/transform.py

@@ -19,6 +19,7 @@
 structures (through Block.logic, block.Wirevector_set, etc).
 """
 import functools
+from pyrtl.pyrtlexceptions import PyrtlError
 
 from .core import set_working_block, LogicNet, working_block
 from .wire import Const, Input, Output, WireVector, Register
@@ -240,19 +241,25 @@ def clone_wire(old_wire, name=None):
     """ Makes a copy of any existing wire.
 
     :param old_wire: The wire to clone
-    :param name: A name for the new wire
+    :param name: A name for the new wire (required if the old wire
+        and newly cloned wire are part of the same block)
 
-    Note that this function is mainly intended to be used when the
-    two wires are from different blocks. Making two wires with the
-    same name in the same block is not allowed.
+    This function is mainly intended to be used when the two wires are from different
+    blocks. Making two wires with the same name in the same block is not allowed.
     """
+    if name is None:
+        if working_block() is old_wire._block:
+            raise PyrtlError("Must provide a name for the newly cloned wire "
+                             "when cloning within the same block.")
+        name = old_wire.name
+
+    if name in working_block().wirevector_by_name:
+        raise PyrtlError("Cannot give a newly cloned wire the same name "
+                         "as an existing wire.")
+
     if isinstance(old_wire, Const):
-        if name is None:
-            return Const(old_wire.val, old_wire.bitwidth, name=old_wire.name)
         return Const(old_wire.val, old_wire.bitwidth, name=name)
     else:
-        if name is None:
-            return old_wire.__class__(old_wire.bitwidth, name=old_wire.name)
         return old_wire.__class__(old_wire.bitwidth, name=name)
 
 

tests/test_transform.py

@@ -1,3 +1,4 @@
+from pyrtl.core import set_working_block
 import unittest
 import pyrtl
 from pyrtl import transform
@@ -50,7 +51,8 @@ def insert_random_inversions(rate=0.5):
 
     def randomly_replace(wire):
         if random.random() < rate:
-            new_src, new_dst = transform.clone_wire(wire), transform.clone_wire(wire)
+            new_src = transform.clone_wire(wire, pyrtl.wire.next_tempvar_name())
+            new_dst = transform.clone_wire(wire, pyrtl.wire.next_tempvar_name())
             new_dst <<= ~new_src
             return new_src, new_dst
         return wire, wire
@@ -258,6 +260,81 @@ def test_replace_only_dst_wire(self):
         self.assertEqual(w4_dst_net.dests, w1_dst_net.dests)
 
 
+class TestCloning(unittest.TestCase):
+    def setUp(self):
+        pyrtl.reset_working_block()
+
+    def test_same_type(self):
+        for ix, cls in enumerate([pyrtl.WireVector, pyrtl.Register, pyrtl.Input, pyrtl.Output]):
+            w1 = cls(4, 'w%d' % ix)
+            w2 = pyrtl.clone_wire(w1, 'y%d' % ix)
+            self.assertIsInstance(w2, cls)
+            self.assertEqual(w1.bitwidth, w2.bitwidth)
+
+    def test_clone_wire_no_name_same_block(self):
+        a = pyrtl.WireVector(1, 'a')
+        with self.assertRaises(pyrtl.PyrtlError) as error:
+            pyrtl.clone_wire(a)
+        self.assertEqual(
+            str(error.exception),
+            "Must provide a name for the newly cloned wire "
+            "when cloning within the same block."
+        )
+
+    def test_clone_wire_same_name_same_block(self):
+        a = pyrtl.WireVector(1, 'a')
+        with self.assertRaises(pyrtl.PyrtlError) as error:
+            pyrtl.clone_wire(a, 'a')
+        self.assertEqual(
+            str(error.exception),
+            "Cannot give a newly cloned wire the same name as an existing wire."
+        )
+
+    def test_clone_wire_different_name_same_block(self):
+        a = pyrtl.WireVector(1, 'a')
+        self.assertEqual(a.name, 'a')
+        self.assertEqual(pyrtl.working_block().wirevector_set, {a})
+        self.assertIs(pyrtl.working_block().wirevector_by_name['a'], a)
+
+        w = pyrtl.clone_wire(a, name='w')
+        self.assertEqual(w.name, 'w')
+        self.assertEqual(a.name, 'a')
+        self.assertIs(pyrtl.working_block().wirevector_by_name['w'], w)
+        self.assertIs(pyrtl.working_block().wirevector_by_name['a'], a)
+        self.assertEqual(pyrtl.working_block().wirevector_set, {a, w})
+
+        pyrtl.working_block().remove_wirevector(a)
+        self.assertEqual(pyrtl.working_block().wirevector_set, {w})
+
+    def test_clone_wire_no_or_same_name_different_block(self):
+        for clone_name in (None, 'a'):
+            a = pyrtl.WireVector(1, 'a')
+            b = pyrtl.Block()
+            with pyrtl.set_working_block(b):
+                w = pyrtl.clone_wire(a, name=clone_name)
+
+            self.assertEqual(a.name, 'a')
+            self.assertIs(pyrtl.working_block().wirevector_by_name['a'], a)
+            self.assertEqual(pyrtl.working_block().wirevector_set, {a})
+
+            self.assertEqual(w.name, 'a')
+            self.assertIs(b.wirevector_by_name['a'], w)
+            self.assertEqual(b.wirevector_set, {w})
+            pyrtl.reset_working_block()
+
+    def test_clone_wire_different_name_different_block(self):
+        a = pyrtl.WireVector(1, 'a')
+        b = pyrtl.Block()
+        with set_working_block(b):
+            w = pyrtl.clone_wire(a, 'w')
+        self.assertEqual(a.name, 'a')
+        self.assertEqual(w.name, 'w')
+        self.assertIs(pyrtl.working_block().wirevector_by_name['a'], a)
+        self.assertEqual(pyrtl.working_block().wirevector_set, {a})
+        self.assertIs(b.wirevector_by_name['w'], w)
+        self.assertEqual(b.wirevector_set, {w})
+
+
 # this code needs mocking from python 3's unittests to work
 """
 @mock.patch('transform_examples.pyrtl.probe')