genrtlil: improve name conflict error messaging

diff --git a/frontends/ast/genrtlil.cc b/frontends/ast/genrtlil.cc
index 449f8c38e9db967c1ba81a1ea9bd2c61cc4592a8..d4299bf69c64b35a5b7cc93fdab9baad7035df7c 100644 (file)
--- a/frontends/ast/genrtlil.cc
+++ b/frontends/ast/genrtlil.cc
@@ -1002,6 +1002,29 @@ void AstNode::detectSignWidth(int &width_hint, bool &sign_hint, bool *found_real
        detectSignWidthWorker(width_hint, sign_hint, found_real);
 }
 
+static void check_unique_id(RTLIL::Module *module, RTLIL::IdString id,
+               const AstNode *node, const char *to_add_kind)
+{
+       auto already_exists = [&](const RTLIL::AttrObject *existing, const char *existing_kind) {
+               std::string src = existing->get_string_attribute(ID::src);
+               std::string location_str = "earlier";
+               if (!src.empty())
+                       location_str = "at " + src;
+               log_file_error(node->filename, node->location.first_line,
+                       "Cannot add %s `%s' because a %s with the same name was already created %s!\n",
+                       to_add_kind, id.c_str(), existing_kind, location_str.c_str());
+       };
+
+       if (const RTLIL::Wire *wire = module->wire(id))
+               already_exists(wire, "signal");
+       if (const RTLIL::Cell *cell = module->cell(id))
+               already_exists(cell, "cell");
+       if (module->processes.count(id))
+               already_exists(module->processes.at(id), "process");
+       if (module->memories.count(id))
+               already_exists(module->memories.at(id), "memory");
+}
+
 // create RTLIL from an AST node
 // all generated cells, wires and processes are added to the module pointed to by 'current_module'
 // when the AST node is an expression (AST_ADD, AST_BIT_XOR, etc.), the result signal is returned.
@@ -1047,7 +1070,9 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
                // If a port in a module with unknown type is found, mark it with the attribute 'is_interface'
                // This is used by the hierarchy pass to know when it can replace interface connection with the individual
                // signals.
-               RTLIL::Wire *wire = current_module->addWire(str, 1);
+               RTLIL::IdString id = str;
+               check_unique_id(current_module, id, this, "interface port");
+               RTLIL::Wire *wire = current_module->addWire(id, 1);
                set_src_attr(wire, this);
                wire->start_offset = 0;
                wire->port_id = port_id;
@@ -1085,7 +1110,9 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
                                log_file_error(filename, location.first_line, "Parameter `%s' with non-constant value!\n", str.c_str());
 
                        RTLIL::Const val = children[0]->bitsAsConst();
-                       RTLIL::Wire *wire = current_module->addWire(str, GetSize(val));
+                       RTLIL::IdString id = str;
+                       check_unique_id(current_module, id, this, "pwire");
+                       RTLIL::Wire *wire = current_module->addWire(id, GetSize(val));
                        current_module->connect(wire, val);
                        wire->is_signed = children[0]->is_signed;
 
@@ -1102,15 +1129,15 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
 
        // create an RTLIL::Wire for an AST_WIRE node
        case AST_WIRE: {
-                       if (current_module->wires_.count(str) != 0)
-                               log_file_error(filename, location.first_line, "Re-definition of signal `%s'!\n", str.c_str());
                        if (!range_valid)
                                log_file_error(filename, location.first_line, "Signal `%s' with non-constant width!\n", str.c_str());
 
                        if (!(range_left + 1 >= range_right))
                                log_file_error(filename, location.first_line, "Signal `%s' with invalid width range %d!\n", str.c_str(), range_left - range_right + 1);
 
-                       RTLIL::Wire *wire = current_module->addWire(str, range_left - range_right + 1);
+                       RTLIL::IdString id = str;
+                       check_unique_id(current_module, id, this, "signal");
+                       RTLIL::Wire *wire = current_module->addWire(id, range_left - range_right + 1);
                        set_src_attr(wire, this);
                        wire->start_offset = range_right;
                        wire->port_id = port_id;
@@ -1132,9 +1159,6 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
 
        // create an RTLIL::Memory for an AST_MEMORY node
        case AST_MEMORY: {
-                       if (current_module->memories.count(str) != 0)
-                               log_file_error(filename, location.first_line, "Re-definition of memory `%s'!\n", str.c_str());
-
                        log_assert(children.size() >= 2);
                        log_assert(children[0]->type == AST_RANGE);
                        log_assert(children[1]->type == AST_RANGE);
@@ -1153,6 +1177,7 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
                                memory->start_offset = children[1]->range_left;
                                memory->size = children[1]->range_right - children[1]->range_left + 1;
                        }
+                       check_unique_id(current_module, memory->name, this, "memory");
                        current_module->memories[memory->name] = memory;
 
                        for (auto &attr : attributes) {
@@ -1684,6 +1709,7 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
                        else
                                cellname = str;
 
+                       check_unique_id(current_module, cellname, this, "procedural assertion");
                        RTLIL::Cell *cell = current_module->addCell(cellname, celltype);
                        set_src_attr(cell, this);
 
@@ -1726,10 +1752,9 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint)
                {
                        int port_counter = 0, para_counter = 0;
 
-                       if (current_module->count_id(str) != 0)
-                               log_file_error(filename, location.first_line, "Re-definition of cell `%s'!\n", str.c_str());
-
-                       RTLIL::Cell *cell = current_module->addCell(str, "");
+                       RTLIL::IdString id = str;
+                       check_unique_id(current_module, id, this, "cell");
+                       RTLIL::Cell *cell = current_module->addCell(id, "");
                        set_src_attr(cell, this);
                        // Set attribute 'module_not_derived' which will be cleared again after the hierarchy pass
                        cell->set_bool_attribute(ID::module_not_derived);

diff --git a/tests/verilog/conflict_assert.ys b/tests/verilog/conflict_assert.ys
new file mode 100644 (file)
index 0000000..121a0cf
--- /dev/null
+++ b/tests/verilog/conflict_assert.ys
@@ -0,0 +1,8 @@
+logger -expect error "Cannot add procedural assertion `\\x' because a signal with the same name was already created" 1
+read_verilog -sv <<EOT
+module top;
+    wire x, y;
+    always @*
+        x: assert(y == 1);
+endmodule
+EOT

diff --git a/tests/verilog/conflict_cell_memory.ys b/tests/verilog/conflict_cell_memory.ys
new file mode 100644 (file)
index 0000000..ddc6759
--- /dev/null
+++ b/tests/verilog/conflict_cell_memory.ys
@@ -0,0 +1,9 @@
+logger -expect error "Cannot add cell `\\x' because a memory with the same name was already created" 1
+read_verilog <<EOT
+module mod;
+endmodule
+module top;
+    reg [2:0] x [0:0];
+    mod x();
+endmodule
+EOT

diff --git a/tests/verilog/conflict_interface_port.ys b/tests/verilog/conflict_interface_port.ys
new file mode 100644 (file)
index 0000000..b97ec02
--- /dev/null
+++ b/tests/verilog/conflict_interface_port.ys
@@ -0,0 +1,17 @@
+logger -expect error "Cannot add interface port `\\i' because a signal with the same name was already created" 1
+read_verilog -sv <<EOT
+interface intf;
+    logic x;
+    assign x = 1;
+    modport m(input x);
+endinterface
+module mod(intf.m i);
+    wire x;
+    assign x = i.x;
+    wire i;
+endmodule
+module top;
+    intf i();
+    mod m(i);
+endmodule
+EOT

diff --git a/tests/verilog/conflict_memory_wire.ys b/tests/verilog/conflict_memory_wire.ys
new file mode 100644 (file)
index 0000000..5c29607
--- /dev/null
+++ b/tests/verilog/conflict_memory_wire.ys
@@ -0,0 +1,7 @@
+logger -expect error "Cannot add memory `\\x' because a signal with the same name was already created" 1
+read_verilog <<EOT
+module top;
+    reg [2:0] x;
+    reg [2:0] x [0:0];
+endmodule
+EOT

diff --git a/tests/verilog/conflict_pwire.ys b/tests/verilog/conflict_pwire.ys
new file mode 100644 (file)
index 0000000..ecc30d3
--- /dev/null
+++ b/tests/verilog/conflict_pwire.ys
@@ -0,0 +1,8 @@
+logger -expect error "Cannot add pwire `\\x' because a signal with the same name was already created" 1
+read_verilog -pwires <<EOT
+module top;
+    wire x;
+    assign x = 1;
+    localparam x = 2;
+endmodule
+EOT

diff --git a/tests/verilog/conflict_wire_memory.ys b/tests/verilog/conflict_wire_memory.ys
new file mode 100644 (file)
index 0000000..77520fe
--- /dev/null
+++ b/tests/verilog/conflict_wire_memory.ys
@@ -0,0 +1,7 @@
+logger -expect error "Cannot add signal `\\x' because a memory with the same name was already created" 1
+read_verilog <<EOT
+module top;
+    reg [2:0] x [0:0];
+    reg [2:0] x;
+endmodule
+EOT