From 74b433c083bb59d3ea3bdbb54ff1274647e55c2e Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 15 Jan 2022 12:54:15 +0100 Subject: [PATCH 1/3] parse.y: Refector enum rule Refactor the enum rule by adding a enum_base_type rule which handles the type specific initialization. This allows to keep the non-type specific parts in a common rule, which makes it easier to modify in future changes. Signed-off-by: Lars-Peter Clausen --- parse.y | 61 ++++++++++++++++++++++----------------------------------- 1 file changed, 23 insertions(+), 38 deletions(-) diff --git a/parse.y b/parse.y index b00658723..243f6703b 100644 --- a/parse.y +++ b/parse.y @@ -600,7 +600,7 @@ static void current_function_set_statement(const YYLTYPE&loc, std::vector tf_port_item_expr_opt value_range_expression %type enum_name_list enum_name -%type enum_data_type +%type enum_data_type enum_base_type %type function_item function_item_list function_item_list_opt %type task_item task_item_list task_item_list_opt @@ -2863,65 +2863,50 @@ type_declaration for an optional base type. The default base type is "int", but it can be any of the integral or vector types. */ -enum_data_type - : K_enum '{' enum_name_list '}' +enum_base_type /* IEEE 1800-2012 A.2.2.1 */ + : { enum_type_t*enum_type = new enum_type_t; - FILE_NAME(enum_type, @1); - enum_type->names .reset($3); enum_type->base_type = IVL_VT_BOOL; enum_type->signed_flag = true; enum_type->integer_flag = false; enum_type->range.reset(make_range_from_width(32)); $$ = enum_type; } - | K_enum atom2_type signed_unsigned_opt '{' enum_name_list '}' + | atom2_type signed_unsigned_opt { enum_type_t*enum_type = new enum_type_t; - FILE_NAME(enum_type, @1); - enum_type->names .reset($5); enum_type->base_type = IVL_VT_BOOL; - enum_type->signed_flag = $3; + enum_type->signed_flag = $2; enum_type->integer_flag = false; - enum_type->range.reset(make_range_from_width($2)); + enum_type->range.reset(make_range_from_width($1)); $$ = enum_type; } - | K_enum K_integer signed_unsigned_opt '{' enum_name_list '}' + | K_integer signed_unsigned_opt { enum_type_t*enum_type = new enum_type_t; - FILE_NAME(enum_type, @1); - enum_type->names .reset($5); enum_type->base_type = IVL_VT_LOGIC; - enum_type->signed_flag = $3; + enum_type->signed_flag = $2; enum_type->integer_flag = true; enum_type->range.reset(make_range_from_width(integer_width)); $$ = enum_type; } - | K_enum K_logic unsigned_signed_opt dimensions_opt '{' enum_name_list '}' - { enum_type_t*enum_type = new enum_type_t; - FILE_NAME(enum_type, @1); - enum_type->names .reset($6); - enum_type->base_type = IVL_VT_LOGIC; - enum_type->signed_flag = $3; + | integer_vector_type unsigned_signed_opt dimensions_opt + { ivl_variable_type_t use_vtype = $1; + if (use_vtype == IVL_VT_NO_TYPE) + use_vtype = IVL_VT_LOGIC; + + enum_type_t*enum_type = new enum_type_t; + enum_type->base_type = use_vtype; + enum_type->signed_flag = $2; enum_type->integer_flag = false; - enum_type->range.reset($4 ? $4 : make_range_from_width(1)); + enum_type->range.reset($3 ? $3 : make_range_from_width(1)); $$ = enum_type; } - | K_enum K_reg unsigned_signed_opt dimensions_opt '{' enum_name_list '}' - { enum_type_t*enum_type = new enum_type_t; + ; + +enum_data_type /* IEEE 1800-2012 A.2.2.1 */ + : K_enum enum_base_type '{' enum_name_list '}' + { enum_type_t*enum_type = $2; FILE_NAME(enum_type, @1); - enum_type->names .reset($6); - enum_type->base_type = IVL_VT_LOGIC; - enum_type->signed_flag = $3; - enum_type->integer_flag = false; - enum_type->range.reset($4 ? $4 : make_range_from_width(1)); - $$ = enum_type; - } - | K_enum K_bit unsigned_signed_opt dimensions_opt '{' enum_name_list '}' - { enum_type_t*enum_type = new enum_type_t; - FILE_NAME(enum_type, @1); - enum_type->names .reset($6); - enum_type->base_type = IVL_VT_BOOL; - enum_type->signed_flag = $3; - enum_type->integer_flag = false; - enum_type->range.reset($4 ? $4 : make_range_from_width(1)); + enum_type->names.reset($4); $$ = enum_type; } ; From 9e6f651e0963cc179be7f84313196b1039e4dfc1 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 15 Jan 2022 13:08:32 +0100 Subject: [PATCH 2/3] Put enum type into scope when declaring it When creating an enum type it must be added to the scope where it is declared so it can later be elaborated and the enum and its names can be referenced in expressions. In addition the names of the enum must be added to the lexor scope so that name collisions are detected and can be reported as errors. This is done with pform_put_enum_type_in_scope() function. At the moment the function is called from two different places * When adding a typedef of a enum type * When creating a signal of a enum type In addition the enum_type_t is added to a class scope `enum_sets` when declaring a enum property in a class. But this only makes sure that the enum gets elaborated, its names are not added to the lexor scope. This works fine for the most part, but breaks for a few corner cases. E.g. it is possible to declare a enum type as part of the subtype of another packed type such as structs or packed arrays. E.g. ``` struct packed { enum { A } e; } s; ``` This is not covered by either of the cases above and neither do the names of the enum get added to the lexor scope, nor is the enum type elaborated. Another corner case that is currently not working is declaring a class property where the type is a typedef of a enum that is declared outside of the class. In this case the enum is elaborated again inside the class scope. E.g. the below is supposed to work, but fails with an already declared symbol error. ``` typedef enum { A } e_t; class C; typedef enum { A } e1; e_t e2; endclass ``` In addition since for enums declared in classes they are only added to `enum_sets`, but names are not added to the lexor scope, it is possible to declare a different symbol in the class scope with the same name. E.g. the following elaborates fine ``` class C; enum { A } e; typedef int A; endclass ``` To fix this call pform_put_enum_type_in_scope() when the enum_type_t is created in the parser. This makes sure that it is handled the same regardless where the type is declared or used. Signed-off-by: Lars-Peter Clausen --- parse.y | 1 + pform.cc | 9 +-------- pform.h | 2 ++ pform_pclass.cc | 4 ---- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/parse.y b/parse.y index 243f6703b..6c285e71f 100644 --- a/parse.y +++ b/parse.y @@ -2907,6 +2907,7 @@ enum_data_type /* IEEE 1800-2012 A.2.2.1 */ { enum_type_t*enum_type = $2; FILE_NAME(enum_type, @1); enum_type->names.reset($4); + pform_put_enum_type_in_scope(enum_type); $$ = enum_type; } ; diff --git a/pform.cc b/pform.cc index 98c49fb5e..1ee8de7d9 100644 --- a/pform.cc +++ b/pform.cc @@ -827,7 +827,7 @@ static void pform_put_wire_in_scope(perm_string name, PWire*net) lexical_scope->wires[name] = net; } -static void pform_put_enum_type_in_scope(enum_type_t*enum_set) +void pform_put_enum_type_in_scope(enum_type_t*enum_set) { if (lexical_scope->enum_sets.count(enum_set)) return; @@ -888,9 +888,6 @@ void pform_set_typedef(perm_string name, data_type_t*data_type, std::listname = name; - - if (enum_type_t*enum_type = dynamic_cast(data_type)) - pform_put_enum_type_in_scope(enum_type); } void pform_set_type_referenced(const struct vlltype&loc, const char*name) @@ -3592,10 +3589,6 @@ static void pform_set_enum(const struct vlltype&li, enum_type_t*enum_type, // Add the file and line information to the enumeration type. FILE_NAME(&(enum_type->li), li); - // If this is an anonymous enumeration, attach it to the current scope. - if (enum_type->name.nil()) - pform_put_enum_type_in_scope(enum_type); - // Now apply the checked enumeration type to the variables // that are being declared with this type. for (list::iterator cur = names->begin() diff --git a/pform.h b/pform.h index a2d2d3f8d..e95e164ec 100644 --- a/pform.h +++ b/pform.h @@ -613,4 +613,6 @@ extern void pform_set_timeprec(const char*txt, bool initial_decl); extern bool allow_timeunit_decl; extern bool allow_timeprec_decl; +void pform_put_enum_type_in_scope(enum_type_t*enum_set); + #endif /* IVL_pform_H */ diff --git a/pform_pclass.cc b/pform_pclass.cc index 15c3d0e2a..e9facc013 100644 --- a/pform_pclass.cc +++ b/pform_pclass.cc @@ -69,10 +69,6 @@ void pform_class_property(const struct vlltype&loc, { assert(pform_cur_class); - if (enum_type_t*enum_set = dynamic_cast(data_type)) { - pform_cur_class->enum_sets .insert(enum_set); - } - // Add the non-static properties to the class type // object. Unwind the list of names to make a map of name to // type. From d856a35e4d146e258697918a5274387b4cbbeba4 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 15 Jan 2022 20:27:02 +0100 Subject: [PATCH 3/3] Add regression test for enums declared in structs and classes These tests check that a enum that is declared in a struct or class are correctly elaborated and also name collisions with the enum names are detected. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/enum_in_class.v | 52 +++++++++++++++++++++++ ivtest/ivltests/enum_in_class_name_coll.v | 13 ++++++ ivtest/ivltests/enum_in_struct.v | 21 +++++++++ ivtest/regress-sv.list | 3 ++ ivtest/regress-vlog95.list | 2 + ivtest/regression_report-devel.txt | 5 ++- ivtest/regression_report-fsv.txt | 5 ++- ivtest/regression_report-strict.txt | 5 ++- ivtest/regression_report-vlog95.txt | 5 ++- 9 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 ivtest/ivltests/enum_in_class.v create mode 100644 ivtest/ivltests/enum_in_class_name_coll.v create mode 100644 ivtest/ivltests/enum_in_struct.v diff --git a/ivtest/ivltests/enum_in_class.v b/ivtest/ivltests/enum_in_class.v new file mode 100644 index 000000000..df5749f57 --- /dev/null +++ b/ivtest/ivltests/enum_in_class.v @@ -0,0 +1,52 @@ +// Check that when a enum type is declared inside a class that the enum is +// properly installed in the scope and the enum items are available. +// +// Also check that when using a typedef of a enum inside a class that the enum +// is not elaborated inside the class and it is possible to have a enum with the +// same names inside the class scope. + +module test; + +typedef enum integer { + A = 1 +} e1_t; + +class C; + typedef enum integer { + A = 10 + } e2_t; + e1_t e1; + e2_t e2; + + function new(); + e1 = test.A; + e2 = A; + endfunction + + function void set(e2_t new_e2); + e2 = new_e2; + endfunction + +endclass + +C c; + +initial begin + c = new; + c.e1 = A; + c.set(c.e2); + + // Not yet supported + // c.e2 = C::A; + // c.e2 = c.A; + + // Check that they have the numerical value from the right scope + if (c.e1 == 1 && c.e2 == 10) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end +end + + +endmodule diff --git a/ivtest/ivltests/enum_in_class_name_coll.v b/ivtest/ivltests/enum_in_class_name_coll.v new file mode 100644 index 000000000..af161f0f5 --- /dev/null +++ b/ivtest/ivltests/enum_in_class_name_coll.v @@ -0,0 +1,13 @@ +// Check that the enum names are added to the lexor scope of the class and +// name collisions with other symbols are reported as errors. + +module test; + +class C; + enum { + A = 10 + } e; + typedef int A; +endclass + +endmodule diff --git a/ivtest/ivltests/enum_in_struct.v b/ivtest/ivltests/enum_in_struct.v new file mode 100644 index 000000000..2d81ee6dc --- /dev/null +++ b/ivtest/ivltests/enum_in_struct.v @@ -0,0 +1,21 @@ +// Check that when a enum type is declared inside a struct that the enum is +// properly installed in the scope and the enum items are available + +module test; + +struct packed { + enum integer { + A + } e; +} s; + +initial begin + s.e = A; + if (s.e == A) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end +end + +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index 763de3c0e..530bfd72c 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -231,6 +231,9 @@ edge normal,-g2009 ivltests enum_base_range normal,-g2005-sv ivltests enum_elem_ranges normal,-g2005-sv ivltests enum_dims_invalid CE,-g2005-sv ivltests +enum_in_struct normal,-g2005-sv ivltests +enum_in_class normal,-g2005-sv ivltests +enum_in_class_name_coll CE,-g2005-sv ivltests enum_next normal,-g2005-sv ivltests enum_ports normal,-g2005-sv ivltests enum_test1 normal,-g2005-sv ivltests diff --git a/ivtest/regress-vlog95.list b/ivtest/regress-vlog95.list index a6e23db82..fdb1934a6 100644 --- a/ivtest/regress-vlog95.list +++ b/ivtest/regress-vlog95.list @@ -336,6 +336,8 @@ br_gh391 CE,-g2009 ivltests br_gh437 CE,-g2009 ivltests br_gh445 CE,-g2009 ivltests br_gh461 CE,-g2009 ivltests +enum_in_class CE,-g2005-sv ivltests +enum_in_class_name_coll CE,-g2005-sv ivltests sv_class1 CE,-g2009 ivltests sv_class2 CE,-g2009 ivltests sv_class3 CE,-g2009 ivltests diff --git a/ivtest/regression_report-devel.txt b/ivtest/regression_report-devel.txt index f4cd7746b..795b643ce 100644 --- a/ivtest/regression_report-devel.txt +++ b/ivtest/regression_report-devel.txt @@ -2006,6 +2006,9 @@ test_mos_strength_reduction: Passed. enum_base_range: Passed. enum_elem_ranges: Passed. enum_dims_invalid: Passed - CE. + enum_in_struct: Passed. + enum_in_class: Passed. + enum_in_class_name_coll: Passed - CE. enum_next: Passed. enum_ports: Passed. enum_test1: Passed. @@ -2562,4 +2565,4 @@ test_mos_strength_reduction: Passed. ufuncsynth1: Passed. ============================================================================ Test results: - Total=2560, Passed=2554, Failed=3, Not Implemented=0, Expected Fail=3 + Total=2563, Passed=2557, Failed=3, Not Implemented=0, Expected Fail=3 diff --git a/ivtest/regression_report-fsv.txt b/ivtest/regression_report-fsv.txt index c91018241..d9a06e151 100644 --- a/ivtest/regression_report-fsv.txt +++ b/ivtest/regression_report-fsv.txt @@ -2013,6 +2013,9 @@ test_mos_strength_reduction: Passed. enum_base_range: Passed. enum_elem_ranges: Passed. enum_dims_invalid: Passed - CE. + enum_in_struct: Passed. + enum_in_class: Passed. + enum_in_class_name_coll: Passed - CE. enum_next: Passed. enum_ports: Passed. enum_test1: Passed. @@ -2562,4 +2565,4 @@ test_mos_strength_reduction: Passed. ufuncsynth1: Passed. ============================================================================ Test results: - Total=2560, Passed=2540, Failed=17, Not Implemented=3, Expected Fail=0 + Total=2563, Passed=2543, Failed=17, Not Implemented=3, Expected Fail=0 diff --git a/ivtest/regression_report-strict.txt b/ivtest/regression_report-strict.txt index 44bfa4d87..684e37358 100644 --- a/ivtest/regression_report-strict.txt +++ b/ivtest/regression_report-strict.txt @@ -2002,6 +2002,9 @@ test_mos_strength_reduction: Passed. enum_base_range: Passed. enum_elem_ranges: Passed. enum_dims_invalid: Passed - CE. + enum_in_struct: Passed. + enum_in_class: Passed. + enum_in_class_name_coll: Passed - CE. enum_next: Passed. enum_ports: Passed. enum_test1: Passed. @@ -2559,4 +2562,4 @@ test_mos_strength_reduction: Passed. ufuncsynth1: Passed. ============================================================================ Test results: - Total=2557, Passed=2551, Failed=3, Not Implemented=0, Expected Fail=3 + Total=2560, Passed=2554, Failed=3, Not Implemented=0, Expected Fail=3 diff --git a/ivtest/regression_report-vlog95.txt b/ivtest/regression_report-vlog95.txt index 783ba0429..f59f4c4de 100644 --- a/ivtest/regression_report-vlog95.txt +++ b/ivtest/regression_report-vlog95.txt @@ -248,6 +248,8 @@ Running vlog95 compiler/VVP tests for Icarus Verilog version: 12. br_gh437: Passed - CE. br_gh445: Passed - CE. br_gh461: Passed - CE. + enum_in_class: Passed - CE. + enum_in_class_name_coll: Passed - CE. sv_class1: Passed - CE. sv_class2: Passed - CE. sv_class3: Passed - CE. @@ -2309,6 +2311,7 @@ test_mos_strength_reduction: Passed. edge: Passed. enum_base_range: Passed. enum_dims_invalid: Passed - CE. + enum_in_struct: Passed. enum_test2: Passed. enum_test3: Passed - CE. enum_test4: Passed. @@ -2562,4 +2565,4 @@ test_mos_strength_reduction: Passed. synth_if_no_else: Passed. ============================================================================ Test results: - Total=2560, Passed=2521, Failed=2, Not Implemented=3, Expected Fail=34 + Total=2563, Passed=2524, Failed=2, Not Implemented=3, Expected Fail=34