From ac815a61189333094fc96f982fb0e7e1a510ff4e Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sun, 9 Jan 2022 19:37:46 +0100 Subject: [PATCH 1/3] netstruct_t: Set line info netstruct_t inherits from LineInfo. But the file and line information is never set leading to messages like :0: error: Member r of packed struct/union must be packed. When elaborating a netstruct_t set the line info from the struct_type_t it is elaborated from. This makes sure that error messages for the struct type have the proper file and line information when printed. Signed-off-by: Lars-Peter Clausen --- elab_type.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/elab_type.cc b/elab_type.cc index 62f28bbd7..e01b520ba 100644 --- a/elab_type.cc +++ b/elab_type.cc @@ -179,6 +179,8 @@ netstruct_t* struct_type_t::elaborate_type_raw(Design*des, NetScope*scope) const { netstruct_t*res = new netstruct_t; + res->set_line(*this); + res->packed(packed_flag); res->set_signed(signed_flag); From 057cd700fe54f867eac6c40dbf620e6e13e3519b Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sun, 9 Jan 2022 21:21:55 +0100 Subject: [PATCH 2/3] netenum_t: Fix line info enum_type_t inherits from LineInfo, but also has a LineInfo field called `li`. When a enum_type_t is created the LineInfo of the object itself is set to the location where the type is declared. The `li` field gets set when a signal of the enum_type_t is created to the location where the signal is created. The `li` field is then used when elaborating a netenum_t to set the line information on the netenum_t. This works fine when the enum is directly used to declare a signal, since the location of the type and signal declaration are the same and there is only one signal of that type. But when using a typedef and declaring multiple signals with the same type the `li` field will be repeatedly set and eventually point to the last signal declaration of that type. On the other hand when using or declaring an enum as part of an aggregate type such as an array, struct or class the line info will never be set. This can cause misleading error messages. E.g. ``` typedef enum { A, B = A } e_t; struct packed { e_t e; } s; ``` will generate ``` :0: error: Enumeration name B and A have the same value: 32'sd0 ``` To fix this use the LineInfo that was assigned to the enum_type_t itself when it was declared and remove the `li` field. Signed-off-by: Lars-Peter Clausen --- elab_scope.cc | 2 +- pform.cc | 3 --- pform_types.h | 1 - 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/elab_scope.cc b/elab_scope.cc index 244e1bed1..c7d205d1f 100644 --- a/elab_scope.cc +++ b/elab_scope.cc @@ -203,7 +203,7 @@ static void elaborate_scope_enumeration(Design*des, NetScope*scope, enum_type->names->size(), enum_type); - use_enum->set_line(enum_type->li); + use_enum->set_line(*enum_type); scope->add_enumeration_set(enum_type, use_enum); size_t name_idx = 0; diff --git a/pform.cc b/pform.cc index 3abce4071..b45d172ab 100644 --- a/pform.cc +++ b/pform.cc @@ -3588,9 +3588,6 @@ static void pform_set_enum(const struct vlltype&li, enum_type_t*enum_type, // IVL_VT_BOOL. assert(enum_type->base_type==IVL_VT_LOGIC || enum_type->base_type==IVL_VT_BOOL); - // Add the file and line information to the enumeration type. - FILE_NAME(&(enum_type->li), li); - // 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_types.h b/pform_types.h index 2b15810fb..a7f58152c 100644 --- a/pform_types.h +++ b/pform_types.h @@ -191,7 +191,6 @@ struct enum_type_t : public data_type_t { bool integer_flag; // True if "integer" was used std::unique_ptr< std::list > range; std::unique_ptr< std::list > names; - LineInfo li; }; struct struct_member_t : public LineInfo { From fa643cbfe11f0ab31076d869be5451c756b9f424 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 22 Jan 2022 11:52:10 +0100 Subject: [PATCH 3/3] Add regression tests for enum and struct line info Check that when an error message for a enum or struct data type is generated it points to the location of the declaration of the type. Signed-off-by: Lars-Peter Clausen --- ivtest/gold/enum_line_info.gold | 5 +++++ ivtest/gold/struct_line_info.gold | 4 ++++ ivtest/ivltests/enum_line_info.v | 32 ++++++++++++++++++++++++++++++ ivtest/ivltests/struct_line_info.v | 24 ++++++++++++++++++++++ ivtest/regress-sv.list | 2 ++ 5 files changed, 67 insertions(+) create mode 100644 ivtest/gold/enum_line_info.gold create mode 100644 ivtest/gold/struct_line_info.gold create mode 100644 ivtest/ivltests/enum_line_info.v create mode 100644 ivtest/ivltests/struct_line_info.v diff --git a/ivtest/gold/enum_line_info.gold b/ivtest/gold/enum_line_info.gold new file mode 100644 index 000000000..55c71d642 --- /dev/null +++ b/ivtest/gold/enum_line_info.gold @@ -0,0 +1,5 @@ +./ivltests/enum_line_info.v:7: error: Enumeration name B and A have the same value: 1'd0 +./ivltests/enum_line_info.v:12: error: Enumeration name D and C have the same value: 1'd0 +./ivltests/enum_line_info.v:17: error: Enumeration name F and E have the same value: 1'd0 +./ivltests/enum_line_info.v:22: error: Enumeration name H and G have the same value: 1'd0 +5 error(s) during elaboration. diff --git a/ivtest/gold/struct_line_info.gold b/ivtest/gold/struct_line_info.gold new file mode 100644 index 000000000..81ea6baff --- /dev/null +++ b/ivtest/gold/struct_line_info.gold @@ -0,0 +1,4 @@ +./ivltests/struct_line_info.v:7: error: Member r of packed struct/union must be packed. +./ivltests/struct_line_info.v:12: error: Member r of packed struct/union must be packed. +./ivltests/struct_line_info.v:17: error: Member r of packed struct/union must be packed. +3 error(s) during elaboration. diff --git a/ivtest/ivltests/enum_line_info.v b/ivtest/ivltests/enum_line_info.v new file mode 100644 index 000000000..db19ae741 --- /dev/null +++ b/ivtest/ivltests/enum_line_info.v @@ -0,0 +1,32 @@ +// Checks that the line and file information is correctly attached to a enum +// data type and will be used when printing an error message about the enum. + +module test; + +// Direct +enum logic { + A, B = A +} e1; + +// Used in a struct +typedef enum logic { + C, D = C +} enum1_type; + +// Used as a signal type +typedef enum logic { + E, F = E +} enum2_type; + +// Unreferenced +typedef enum logic { + G, H = G +} enum3_type; + +struct packed { + enum1_type e; +} s; + +enum2_type e2; + +endmodule diff --git a/ivtest/ivltests/struct_line_info.v b/ivtest/ivltests/struct_line_info.v new file mode 100644 index 000000000..b3ce55a0e --- /dev/null +++ b/ivtest/ivltests/struct_line_info.v @@ -0,0 +1,24 @@ +// Checks that the line and file information is correctly attached to a struct +// data type and will be used when printing an error message about the struct. + +module test; + +// Direct +struct packed { + real r; +} s1; + +// Used in a struct +typedef struct packed { + real r; +} struct1_type; + +// Used as a signal type +typedef struct packed { + struct1_type s; + real r; +} struct2_type; + +struct2_type s2; + +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index b5d223bca..b65e12961 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -236,6 +236,7 @@ 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_line_info CE,-g2005-sv ivltests gold=enum_line_info.gold enum_next normal,-g2005-sv ivltests enum_order normal,-g2005-sv ivltests enum_ports normal,-g2005-sv ivltests @@ -381,6 +382,7 @@ struct6 normal,-g2009 ivltests struct7 normal,-g2009 ivltests struct8 normal,-g2009 ivltests struct9 normal,-g2009 ivltests +struct_line_info CE,-g2009 ivltests gold=struct_line_info.gold struct_packed_array normal,-g2009 ivltests struct_packed_array2 normal,-g2009 ivltests struct_packed_sysfunct normal,-g2009 ivltests