Patch 1

[norbertbonnici]

Added missing library and constant declarations

[eine]

@norbertbonnici, thanks for opening this PR! Overall, it looks good, but I think that additional tests should be added, instead of putting them all together. This is because simulators/tools that do not support a single feature might crash during analysis, making it impossible to test other features.

Can you please create several sources, similar to tb_matching_operator_*gt*.vhd? So tb_generics_in_packages_integer.vhd, tb_generics_in_packages_std_logic.vhd, tb_generics_in_packages_std_logic_vector.vhd, etc.

Also, 98ec125 seems like a fix for some simulator having partial support (it feels like ghdl/ghdl#1063 (comment)). I think that the test should not include it. Optionally, we can have a test with it and another one without it.

By the same token, 4c6369c should be two separate tests.

Overall, note that the purpose of this repository is to test the compliance of certain tools with the standard. The fact that we are using GHDL (or any other simulator) in the CI job should not affect the tests. That's why 'fixes' should be separated. Some other tool might work without the fix but not with it.

[norbertbonnici]

@norbertbonnici, thanks for opening this PR! Overall, it looks good, but I think that additional tests should be added, instead of putting them all together. This is because simulators/tools that do not support a single feature might crash during analysis, making it impossible to test other features.

Can you please create several sources, similar to tb_matching_operator_*gt*.vhd? So tb_generics_in_packages_integer.vhd, tb_generics_in_packages_std_logic.vhd, tb_generics_in_packages_std_logic_vector.vhd, etc.

Yes of course! I will have a look at the LRM and see what is required.

Also, 98ec125 seems like a fix for some simulator having partial support (it feels like ghdl/ghdl#1063 (comment)). I think that the test should not include it. Optionally, we can have a test with it and another one without it.

Is this a GHDL or a vunit issue? I think I had similar issues with vunit and modelsim yesterday.

By the same token, 4c6369c should be two separate tests.

Overall, note that the purpose of this repository is to test the compliance of certain tools with the standard. The fact that we are using GHDL (or any other simulator) in the CI job should not affect the tests. That's why 'fixes' should be separated. Some other tool might work without the fix but not with it.

Gotcha!

[eine]

Yes of course! I will have a look at the LRM and see what is required.

Note that, unlike VHDL 2019 (which contains ~60 modifications/features), VHDL 2008 was a major rewrite of the standard. This is to say that you could go mad if you tried to write tests for all the possible cases! Hence, the approach we suggest is for users to take the testbenches they already have and extract MWEs for them, rather than writing "complete" artificial tests from scratch. This is an interesting approach because it lets us know which features do we prioritise as a community.

Also, 98ec125 seems like a fix for some simulator having partial support (it feels like ghdl/ghdl#1063 (comment)). I think that the test should not include it. Optionally, we can have a test with it and another one without it.

Is this a GHDL or a vunit issue? I think I had similar issues with vunit and modelsim yesterday.

I am pretty sure this is NOT an issue with VUnit, because VUnit's check functions/procedures are essentially wrappers around VHDL assertions. You can see in ghdl/ghdl#1063 that the underlying issue was GHDL not guessing the type of the literal. However, when Tristan fixed it, both syntaxes work. Hence, in this case, I believe that both simulators (GHDL or ModelSim/QuestaSim) should accept both syntaxes. That's why we should have two tests. Anyway, I'm pinging @Paebbels and @LarsAsplund here for confirmation.

[bpadalino]

This is a really old PR - does it still apply?

[umarcor]

@bpadalino I think it does. It needs refactoring, as discussed above, but the additional tests introduced in this PR are not covered in the master|main branch yet.

[bpadalino]

@norbertbonnici Are you willing to resurrect this PR, or do you want someone else to adapt it for you?