Merge pull request #254 from diffblue/hierarchical_identifiers2

Verilog: fix nested hierarchical identifiers
diffblue · Dec 6, 2023 · 409db4b · 409db4b
2 parents b83e940 + 0961b8d
commit 409db4b
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 19 deletions.
diff --git a/regression/verilog/hierarchical_identifiers/hierarchical_identifiers1.desc b/regression/verilog/hierarchical_identifiers/hierarchical_identifiers1.desc
@@ -0,0 +1,7 @@
+CORE
+hierarchical_identifiers1.v
+--module main --bound 0
+^EXIT=0$
+^SIGNAL=0$
+--
+^warning: ignoring
diff --git a/.../verilog/hierarchical_identifiers1/main.v → ...l_identifiers/hierarchical_identifiers1.v b/.../verilog/hierarchical_identifiers1/main.v → ...l_identifiers/hierarchical_identifiers1.v
diff --git a/regression/verilog/hierarchical_identifiers1/test.desc b/regression/verilog/hierarchical_identifiers1/test.desc
diff --git a/src/verilog/verilog_expr.h b/src/verilog/verilog_expr.h
@@ -33,6 +33,16 @@ class hierarchical_identifier_exprt : public binary_exprt
     return item();
   }
 
+  const irep_idt &identifier() const
+  {
+    return get(ID_identifier);
+  }
+
+  void identifier(irep_idt _identifier)
+  {
+    set(ID_identifier, _identifier);
+  }
+
 protected:
   using binary_exprt::op0;
   using binary_exprt::op1;

diff --git a/src/verilog/verilog_synthesis.cpp b/src/verilog/verilog_synthesis.cpp
@@ -256,12 +256,12 @@ void verilog_synthesist::expand_hierarchical_identifier(
   const irep_idt &rhs_identifier = expr.rhs().get_identifier();
 
   // just patch together
-
-  std::string full_identifier=
-    id2string(lhs_identifier)+"."+id2string(rhs_identifier);
 
-  // Note: may not yet be in symbol table, as the inst module
-  // item may be later.
+  irep_idt full_identifier =
+    id2string(lhs_identifier) + '.' + id2string(rhs_identifier);
+
+  // Note: the instance copy may not yet be in symbol table,
+  // as the inst module item may be later.
   // The type checker already checked that it's fine.
 
   symbol_exprt new_symbol{full_identifier, expr.type()};

diff --git a/src/verilog/verilog_typecheck_expr.cpp b/src/verilog/verilog_typecheck_expr.cpp
@@ -791,22 +791,27 @@ void verilog_typecheck_exprt::convert_hierarchical_identifier(
 {
   convert_expr(expr.lhs());
 
-  if(expr.lhs().id() != ID_symbol)
-  {
-    throw errort().with_location(expr.source_location())
-      << "expected symbol on lhs of `.'";
-  }
+  const irep_idt &lhs_identifier = [](const exprt &lhs) {
+    if(lhs.id() == ID_symbol)
+      return to_symbol_expr(lhs).get_identifier();
+    else if(lhs.id() == ID_hierarchical_identifier)
+      return to_hierarchical_identifier_expr(lhs).identifier();
+    else
+    {
+      throw errort().with_location(lhs.source_location())
+        << "expected symbol or hierarchical identifier on lhs of `.'";
+    }
+  }(expr.lhs());
 
   DATA_INVARIANT(expr.rhs().id() == ID_symbol, "expected symbol on rhs of `.'");
 
-  const irep_idt &lhs_identifier = to_symbol_expr(expr.lhs()).get_identifier();
   const irep_idt &rhs_identifier = expr.rhs().get_identifier();
 
   irep_idt full_identifier;
 
   if(expr.lhs().type().id() == ID_module_instance)
   {
-    // figure out which module this is
+    // figure out which module the lhs is
     const symbolt *module_instance_symbol;
     if(ns.lookup(lhs_identifier, module_instance_symbol))
     {
@@ -817,6 +822,7 @@ void verilog_typecheck_exprt::convert_hierarchical_identifier(
 
     const irep_idt &module=module_instance_symbol->value.get(ID_module);
 
+    // the identifier in the module
     full_identifier=
       id2string(module)+"."+id2string(rhs_identifier);
 
@@ -839,6 +845,9 @@ void verilog_typecheck_exprt::convert_hierarchical_identifier(
         << "identifier `" << rhs_identifier << "' not found in module `"
         << module_instance_symbol->pretty_name << "'";
     }
+
+    // We remember the identifier of the symbol.
+    expr.identifier(full_identifier);
   }
   else if(expr.lhs().type().id() == ID_named_block)
   {