Merge pull request #364 from diffblue/verilog-fix-symbols

Verilog: clean up base_name vs identifier in declarators
diffblue · Feb 5, 2024 · 9b1c261 · 9b1c261
2 parents 2ba20d2 + cbae8e1
commit 9b1c261
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 28 deletions.
diff --git a/regression/verilog/typedef/typedef_name_collision1.desc b/regression/verilog/typedef/typedef_name_collision1.desc
@@ -1,8 +1,7 @@
-KNOWNBUG
+CORE
 typedef_name_collision1.sv
 
+^file .* line 6: definition of symbol `some_identifier' conflicts with earlier definition at line 3$
 ^EXIT=2$
 ^SIGNAL=0$
 --
---
-The name collision should be errored.
diff --git a/src/verilog/parser.y b/src/verilog/parser.y
@@ -122,7 +122,9 @@ Function: new_symbol
 inline static void new_symbol(YYSTYPE &dest, YYSTYPE &src)
 {
   init(dest, ID_symbol);
-  addswap(dest, ID_identifier, src);
+  const auto base_name = stack_expr(src).id();
+  stack_expr(dest).set(ID_identifier, base_name);
+  stack_expr(dest).set(ID_base_name, base_name);
 }
 
 /*******************************************************************\
@@ -1538,7 +1540,9 @@ list_of_param_assignments:
 
 param_assignment: param_identifier '=' const_expression
 		{ init($$, ID_parameter);
-		  addswap($$, ID_identifier, $1);
+		  auto base_name = stack_expr($1).id();
+		  stack_expr($$).set(ID_identifier, base_name);
+		  stack_expr($$).set(ID_base_name, base_name);
 		  addswap($$, ID_value, $3); }
         ;
 
@@ -2498,9 +2502,7 @@ statement_item:
 	;
 
 system_task_name: TOK_SYSIDENT
-                { init($$, ID_symbol);
-                  stack_expr($$).set(ID_identifier, stack_expr($1).id());
-                }
+                { new_symbol($$, $1); }
         ;
 
 // System Verilog standard 1800-2017
@@ -3182,7 +3184,7 @@ type_identifier: TOK_TYPE_IDENTIFIER
 		{
 		  init($$, ID_typedef_type);
 		  auto base_name = stack_expr($1).id();
-		  stack_expr($$).set(ID_C_base_name, base_name);
+		  stack_expr($$).set(ID_base_name, base_name);
 		  stack_expr($$).set(ID_identifier, PARSER.current_scope->prefix+id2string(base_name));
 		}
 	;

diff --git a/src/verilog/verilog_expr.h b/src/verilog/verilog_expr.h
@@ -424,7 +424,7 @@ class verilog_parameter_declt : public verilog_module_itemt
 
     const irep_idt &base_name() const
     {
-      return get(ID_identifier);
+      return get(ID_base_name);
     }
 
     const exprt &value() const

diff --git a/src/verilog/verilog_interfaces.cpp b/src/verilog/verilog_interfaces.cpp
@@ -65,53 +65,48 @@ void verilog_typecheckt::check_module_ports(
 
     const auto &declarator = decl.declarators().front();
 
-    const irep_idt &name = declarator.identifier();
+    const irep_idt &base_name = declarator.base_name();
 
-    if(name.empty())
+    if(base_name.empty())
     {
       throw errort().with_location(decl.source_location())
         << "empty port name (module " << module_symbol.base_name << ')';
     }
 
-    if(port_names.find(name)!=
-       port_names.end())
+    if(port_names.find(base_name) != port_names.end())
     {
-      error().source_location = declarator.source_location();
-      error() << "duplicate port name: `" << name << '\'' << eom;
-      throw 0;
+      throw errort().with_location(declarator.source_location())
+        << "duplicate port name: `" << base_name << '\'';
     }
 
-    irep_idt identifier=
-      id2string(module_identifier)+"."+id2string(name);
-
+    irep_idt identifier = hierarchical_identifier(base_name);
+
     const symbolt *port_symbol=0;
 
     // find the symbol
 
     if(ns.lookup(identifier, port_symbol))
     {
       throw errort().with_location(declarator.source_location())
-        << "port `" << name << "' not declared";
+        << "port `" << base_name << "' not declared";
     }
 
     if(!port_symbol->is_input && !port_symbol->is_output)
     {
-      error().source_location = declarator.source_location();
-      error() << "port `" << name 
-              << "' not declared as input or output" << eom;
-      throw 0;
+      throw errort().with_location(declarator.source_location())
+        << "port `" << base_name << "' not declared as input or output";
     }
 
-    ports[nr].set("#name", name);
+    ports[nr].set("#name", base_name);
     ports[nr].id(ID_symbol);
     ports[nr].set(ID_identifier, identifier);
     ports[nr].set(ID_C_source_location, declarator.source_location());
     ports[nr].set(ID_type, port_symbol->type);
     ports[nr].set(ID_input, port_symbol->is_input);
     ports[nr].set(ID_output, port_symbol->is_output);
 
-    port_names[name]=nr;
-    
+    port_names[base_name] = nr;
+
     nr++;
   }