From cb3bf494f28b404c7a0a32deec9ddfaf2ec616ca Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 8 Aug 2026 19:00:17 -0700 Subject: [PATCH] Consolidate variable dimension and hierarchy index grammar Variable declaration dimensions and hierarchical identifier indices both use bracketed components, but are parsed by separate grammar rules. Consolidate them into a shared `index_component` rule and convert the components once the surrounding context is known. This prepares for accepting a `TYPE_IDENTIFIER` token as an ordinary name in a hierarchical path. A path component can have the same spelling as a typedef visible in the current scope. In the path it is an ordinary identifier. The hierarchical reference does not override or replace the typedef. The shared rule parses component forms that are not valid in both contexts. Preserve the previous restrictions by explicitly rejecting indexed part selects in variable dimensions and empty or bounded-queue components in hierarchical identifiers. Track the location of each component so these errors point to the invalid suffix. Signed-off-by: Lars-Peter Clausen --- parse.y | 298 ++++++++++++++++++++++++++++++++------------------ pform_types.h | 19 ++-- 2 files changed, 202 insertions(+), 115 deletions(-) diff --git a/parse.y b/parse.y index 3e09e739f..b4f75eb10 100644 --- a/parse.y +++ b/parse.y @@ -164,6 +164,134 @@ static void delete_type_id_range(T&value) value.ranges = nullptr; } +static index_component_t *make_index_component(const struct vlltype &loc, + index_component_t::ctype_t sel, + PExpr *msb = nullptr, + PExpr *lsb = nullptr) +{ + auto component = new index_component_t; + FILE_NAME(component, loc); + component->sel = sel; + component->msb = msb; + component->lsb = lsb; + return component; +} + +static void index_component_requires_sv(const index_component_t &component, + const char *feature) +{ + if (gn_system_verilog()) + return; + + cerr << component.get_fileline() << ": error: " << feature + << " requires SystemVerilog." << endl; + error_count += 1; +} + +static void index_component_error(const index_component_t &component, + const char *message) +{ + cerr << component.get_fileline() << ": error: " << message << endl; + error_count += 1; +} + +static void validate_hierarchy_index_components( + std::list &components) +{ + for (auto cur = components.begin(); cur != components.end();) { + bool invalid = false; + + if (cur->sel == index_component_t::SEL_NONE) { + index_component_error(*cur, + "Empty index is not allowed in a hierarchy identifier."); + invalid = true; + } else if (cur->sel == index_component_t::SEL_QUEUE_BOUND) { + index_component_error(*cur, + "Queue bounds are not allowed in a hierarchy identifier."); + invalid = true; + } else if (cur->sel == index_component_t::SEL_BIT_LAST) { + index_component_requires_sv(*cur, + "Last element expression ($)"); + } + + if (invalid) { + delete cur->msb; + delete cur->lsb; + cur = components.erase(cur); + } else { + ++cur; + } + } +} + +static void append_hierarchy_identifier_component( + pform_name_t &path, perm_string name, + std::list *components) +{ + path.emplace_back(name); + + std::unique_ptr> component_list(components); + if (!component_list) + return; + + validate_hierarchy_index_components(*component_list); + path.back().index.splice(path.back().index.end(), *component_list); +} + +static std::list * +make_dimensions(std::list *components) +{ + std::unique_ptr> component_list(components); + + if (!component_list) + return nullptr; + + auto dimensions = new std::list; + + for (auto &component : *component_list) { + switch (component.sel) { + case index_component_t::SEL_NONE: + index_component_requires_sv(component, + "Dynamic array declaration"); + dimensions->push_back(pform_range_t(nullptr, nullptr)); + break; + case index_component_t::SEL_BIT: + if (!gn_system_verilog()) { + warn_count += 1; + cerr << component.get_fileline() + << ": warning: Use of SystemVerilog [size] dimension. " + << "Use at least -g2005-sv to remove this warning." + << endl; + } + dimensions->push_back(pform_range_t(component.msb, nullptr)); + break; + case index_component_t::SEL_BIT_LAST: + index_component_requires_sv(component, "Queue declaration"); + dimensions->push_back(pform_range_t(new PEQueueDimension, + nullptr)); + break; + case index_component_t::SEL_PART: + dimensions->push_back(pform_range_t(component.msb, + component.lsb)); + break; + case index_component_t::SEL_QUEUE_BOUND: + index_component_requires_sv(component, "Queue declaration"); + dimensions->push_back(pform_range_t(new PEQueueDimension, + component.lsb)); + break; + case index_component_t::SEL_IDX_UP: + case index_component_t::SEL_IDX_DO: + index_component_error(component, + "An indexed part select is not allowed in a dimension."); + dimensions->push_back(pform_range_t(component.msb, + component.lsb)); + break; + } + } + + return dimensions; +} + /* The rules sometimes push attributes into a global context where sub-rules may grab them. This makes parser rules a little easier to write in some cases. */ @@ -797,6 +925,8 @@ Module::port_t *module_declare_interface_port(const YYLTYPE&loc, char *type, std::list*named_pexprs; struct parmvalue_t*parmvalue; std::list*ranges; + index_component_t *index_component; + std::list *index_components; PExpr*expr; std::list*exprs; @@ -1045,7 +1175,10 @@ Module::port_t *module_declare_interface_port(const YYLTYPE&loc, char *type, %type let_port_list_opt let_port_list %type let_port_item -%type hierarchy_identifier implicit_class_handle class_hierarchy_identifier +%type hierarchy_identifier hierarchy_identifier_component +%type implicit_class_handle class_hierarchy_identifier +%type index_component +%type index_components_opt index_components %type spec_notifier_opt spec_notifier %type spec_reference_event %type setuphold_opt_args recrem_opt_args setuphold_recrem_opt_notifier @@ -1093,7 +1226,6 @@ Module::port_t *module_declare_interface_port(const YYLTYPE&loc, char *type, %type class_item_qualifier_opt property_qualifier_opt %type random_qualifier -%type variable_dimension %type dimensions_opt dimensions %type net_type net_type_opt net_type_or_var net_type_or_var_opt @@ -3168,47 +3300,29 @@ value_range /* IEEE1800-2005: A.8.3 */ { } ; -variable_dimension /* IEEE1800-2005: A.2.5 */ - : '[' expression ':' expression ']' - { std::list *tmp = new std::list; - pform_range_t index ($2,$4); - tmp->push_back(index); - $$ = tmp; - } - | '[' expression ']' - { // SystemVerilog canonical range - if (!gn_system_verilog()) { - warn_count += 1; - cerr << @2 << ": warning: Use of SystemVerilog [size] dimension. " - << "Use at least -g2005-sv to remove this warning." << endl; - } - list *tmp = new std::list; - pform_range_t index ($2,0); - tmp->push_back(index); - $$ = tmp; - } - | '[' ']' - { std::list *tmp = new std::list; - pform_range_t index (0,0); - pform_requires_sv(@$, "Dynamic array declaration"); - tmp->push_back(index); - $$ = tmp; +index_component + : '[' expression ']' + { $$ = make_index_component(@$, index_component_t::SEL_BIT, $2); } | '[' '$' ']' - { // SystemVerilog queue - list *tmp = new std::list; - pform_range_t index (new PEQueueDimension,0); - pform_requires_sv(@$, "Queue declaration"); - tmp->push_back(index); - $$ = tmp; + { $$ = make_index_component(@$, index_component_t::SEL_BIT_LAST); + } + | '[' expression ':' expression ']' + { $$ = make_index_component(@$, index_component_t::SEL_PART, $2, $4); + } + | '[' ']' + { $$ = make_index_component(@$, index_component_t::SEL_NONE); } | '[' '$' ':' expression ']' - { // SystemVerilog queue with a max size - list *tmp = new std::list; - pform_range_t index (new PEQueueDimension,$4); - pform_requires_sv(@$, "Queue declaration"); - tmp->push_back(index); - $$ = tmp; + { $$ = make_index_component(@$, + index_component_t::SEL_QUEUE_BOUND, + nullptr, $4); + } + | '[' expression K_PO_POS expression ']' + { $$ = make_index_component(@$, index_component_t::SEL_IDX_UP, $2, $4); + } + | '[' expression K_PO_NEG expression ']' + { $$ = make_index_component(@$, index_component_t::SEL_IDX_DO, $2, $4); } ; @@ -5026,72 +5140,46 @@ switchtype names. */ hierarchy_identifier - : IDENTIFIER - { $$ = new pform_name_t; - $$->push_back(name_component_t(lex_strings.make($1))); - delete[]$1; - } - | hierarchy_identifier '.' identifier_name - { pform_name_t * tmp = $1; - tmp->push_back(name_component_t(lex_strings.make($3))); + : hierarchy_identifier_component + | hierarchy_identifier '.' identifier_name index_components_opt + { auto tmp = $1; + append_hierarchy_identifier_component(*tmp, lex_strings.make($3), $4); delete[]$3; $$ = tmp; } /* "unique" is a keyword (K_unique) but also a queue/array method name. */ - | hierarchy_identifier '.' K_unique - { pform_name_t * tmp = $1; - tmp->push_back(name_component_t(lex_strings.make("unique"))); + | hierarchy_identifier '.' K_unique index_components_opt + { auto tmp = $1; + append_hierarchy_identifier_component(*tmp, lex_strings.make("unique"), + $4); $$ = tmp; } - | hierarchy_identifier '[' expression ']' - { pform_name_t * tmp = $1; - name_component_t&tail = tmp->back(); - index_component_t itmp; - itmp.sel = index_component_t::SEL_BIT; - itmp.msb = $3; - tail.index.push_back(itmp); - $$ = tmp; + ; + +hierarchy_identifier_component + : IDENTIFIER index_components_opt + { $$ = new pform_name_t; + append_hierarchy_identifier_component(*$$, lex_strings.make($1), $2); + delete[]$1; } - | hierarchy_identifier '[' '$' ']' - { pform_requires_sv(@3, "Last element expression ($)"); - pform_name_t * tmp = $1; - name_component_t&tail = tmp->back(); - index_component_t itmp; - itmp.sel = index_component_t::SEL_BIT_LAST; - itmp.msb = 0; - itmp.lsb = 0; - tail.index.push_back(itmp); - $$ = tmp; + ; + +index_components_opt + : index_components + | + { $$ = nullptr; } + ; + +index_components + : index_components index_component + { $1->push_back(*$2); + delete $2; + $$ = $1; } - | hierarchy_identifier '[' expression ':' expression ']' - { pform_name_t * tmp = $1; - name_component_t&tail = tmp->back(); - index_component_t itmp; - itmp.sel = index_component_t::SEL_PART; - itmp.msb = $3; - itmp.lsb = $5; - tail.index.push_back(itmp); - $$ = tmp; - } - | hierarchy_identifier '[' expression K_PO_POS expression ']' - { pform_name_t * tmp = $1; - name_component_t&tail = tmp->back(); - index_component_t itmp; - itmp.sel = index_component_t::SEL_IDX_UP; - itmp.msb = $3; - itmp.lsb = $5; - tail.index.push_back(itmp); - $$ = tmp; - } - | hierarchy_identifier '[' expression K_PO_NEG expression ']' - { pform_name_t * tmp = $1; - name_component_t&tail = tmp->back(); - index_component_t itmp; - itmp.sel = index_component_t::SEL_IDX_DO; - itmp.msb = $3; - itmp.lsb = $5; - tail.index.push_back(itmp); - $$ = tmp; + | index_component + { $$ = new std::list; + $$->push_back(*$1); + delete $1; } ; @@ -6607,20 +6695,14 @@ port_reference_list /* The range is a list of variable dimensions. */ dimensions_opt - : { $$ = 0; } - | dimensions { $$ = $1; } + : index_components_opt + { $$ = make_dimensions($1); + } ; dimensions - : variable_dimension - { $$ = $1; } - | dimensions variable_dimension - { std::list *tmp = $1; - if ($2) { - tmp->splice(tmp->end(), *$2); - delete $2; - } - $$ = tmp; + : index_components + { $$ = make_dimensions($1); } ; diff --git a/pform_types.h b/pform_types.h index fa54f0795..f18e12608 100644 --- a/pform_types.h +++ b/pform_types.h @@ -133,16 +133,21 @@ struct pform_port_t { * * - The SEL_BIT_LAST index component is an array/queue [$] index, * that is the last item in the variable. + * + * - SEL_NONE represents an empty [] dimension and SEL_QUEUE_BOUND represents + * a bounded queue dimension [$:]. These forms are parsed together with + * the other index components and validated when their context is known. */ -struct index_component_t { - enum ctype_t { SEL_NONE, SEL_BIT, SEL_BIT_LAST, SEL_PART, SEL_IDX_UP, SEL_IDX_DO }; +struct index_component_t : public LineInfo { + enum ctype_t { SEL_NONE, SEL_BIT, SEL_BIT_LAST, SEL_PART, + SEL_QUEUE_BOUND, SEL_IDX_UP, SEL_IDX_DO }; - index_component_t() : sel(SEL_NONE), msb(0), lsb(0) { }; - ~index_component_t() { } + index_component_t() = default; + ~index_component_t() override = default; - ctype_t sel; - class PExpr*msb; - class PExpr*lsb; + ctype_t sel = SEL_NONE; + PExpr *msb = nullptr; + PExpr *lsb = nullptr; }; struct name_component_t {