update clone_wire so that name is required when part of the same block

Author: mdko

pyrtl/transform.py

@@ -19,9 +19,10 @@
 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, next_tempvar_name
+from .wire import Const, Input, Output, WireVector, Register
 
 
 def net_transform(transform_func, block=None, **kwargs):
@@ -240,16 +241,21 @@ 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)
 
-    Naming the newly cloned wire the same as another existing wire in the same
-    block will cause the other wire to be given a new internally created name. This
-    function is mainly intended to be used when the two wires are from different blocks.
+    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.
     """
-    name = old_wire.name if name is None else name
-    if name in working_block().wirevector_by_name and (working_block() is old_wire._block):
-        w = working_block().wirevector_by_name[name]
-        w.name = next_tempvar_name()
+    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):
         return Const(old_wire.val, old_wire.bitwidth, name=name)

tests/test_transform.py

@@ -51,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
@@ -263,26 +264,47 @@ class TestCloning(unittest.TestCase):
     def setUp(self):
         pyrtl.reset_working_block()
 
-    def test_clone_wire_no_or_same_name_same_block(self):
-        for clone_name in (None, 'a'):
-            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)
+    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)
 
-            w = pyrtl.clone_wire(a, name=clone_name)
-            self.assertTrue(a.name.startswith("tmp"))
-            self.assertEqual(w.name, 'a')
-            self.assertIs(pyrtl.working_block().wirevector_by_name['a'], w)
-            self.assertEqual(pyrtl.working_block().wirevector_set, {a, w})
+    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.name = 'w'
-            self.assertEqual(w.name, 'w')
-            self.assertIs(pyrtl.working_block().wirevector_by_name['w'], w)
+        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})
-            pyrtl.reset_working_block()
+        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'):
@@ -300,18 +322,7 @@ def test_clone_wire_no_or_same_name_different_block(self):
             self.assertEqual(b.wirevector_set, {w})
             pyrtl.reset_working_block()
 
-    def test_clone_wire_with_different_name_same_block(self):
-        a = pyrtl.WireVector(1, 'a')
-        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, w})
-
-        pyrtl.working_block().remove_wirevector(a)
-        self.assertEqual(pyrtl.working_block().wirevector_set, {w})
-
-    def test_clone_wire_with_different_name_different_block(self):
+    def test_clone_wire_different_name_different_block(self):
         a = pyrtl.WireVector(1, 'a')
         b = pyrtl.Block()
         with set_working_block(b):