VHDL-LS / rust_hdl

Other
336 stars 65 forks source link

[BUG] Errors reported when using attribute (subtype) on indexed array #254

Open nselvara opened 8 months ago

nselvara commented 8 months ago

Bug description: Hi guys, the following errors are reported for the snippet below: Expected '{identifier}' and Expected 'library', 'use', 'context', 'entity', 'architecture', 'configuration' or 'package' It seems to be that VHDL LS doesn't allow arrays be indexed when they're used as a type identifier. With the error itself I can live with but it also breaks the entire hovering over labels and stops to produce warning other stuffs. The code works with Questa Sim 2022.4.

Code to reproduce the error:

type test_arr_t is array (natural range <>) of std_logic_vector(3 downto 0);
signal test_arr_sig: test_arr_t(3 downto 0);
signal test_sig: test_arr_sig(0)'subtype := (others => '0');

Screenshots image image

lgu-appear commented 6 months ago

Isn't this fixed @Schottkyc137? On my side, it works today

Schottkyc137 commented 5 months ago

I still see the error on my side.

Maybe a bit of background on why this is not an easy fix: Names are hard to parse in VHDL. Especially names where constraints can follow. For example, it's not inherently (i.e., just from the syntax) clear, what the syntax

foo(bar)

should compile to. It could be 1) A function named foo, called with a parameter named bar 2) A signal / variable named foo, indexed with bar 3) A type named foo with a constraint named bar. The VHDL standard 'solves' this ambiguity by resolving the kind of foo very early. So, as far as the standard is concerned, it is already clear at the time of parsing that foo is a function / signal / type. Therefore, there is no ambiguity concerning the syntax from above. This is ok if you want to build a compiler that just transforms the text to some intermediate representation, but it makes stuff harder when you want to develop a language server where constructs should also parse ok that are not correct from an analysis point of view. vhdl_ls solves this issue by assuming that a type name can only be an indexed name (attributes are also fine). This works in 99% of cases, but does not catch the edge cases from the issue.

However, for a quick fix; the example could also be written as

type test_arr_t is array (natural range <>) of std_logic_vector(3 downto 0);
signal test_arr_sig: test_arr_t(3 downto 0);
signal test_sig: test_arr_sig'element := (others => '0');

for VHDL > 2008. The 'element attribute just returns the subtype of the array elements.

nselvara commented 5 months ago

I also see the error and I'm using the latest version of VHDL-LS. The workaround that you've provided is cool though, didn't know that. I don't know if that's feasible: Is it possible to parse the tokens foo and bar and get returns that resolve to symbolic names which then can be counterchecked.

Schottkyc137 commented 5 months ago

I also see the error and I'm using the latest version of VHDL-LS. The workaround that you've provided is cool though, didn't know that. I don't know if that's feasible: Is it possible to parse the tokens foo and bar and get returns that resolve to symbolic names which then can be counterchecked.

Yea, something like that is definitely possible and will be implemented at some point. Note, however, that the example I gave is very simple. In a more general case, foo and bar will be more complex expression, potentially containing names themselves together with a conglomerate of expressions, constraints and indexing terms. So it's not as simple as just looking at the tokens unfortunately.

nselvara commented 5 months ago

Yea, something like that is definitely possible and will be implemented at some point. Note, however, that the example I gave is very simple. In a more general case, foo and bar will be more complex expression, potentially containing names themselves together with a conglomerate of expressions, constraints and indexing terms. So it's not as simple as just looking at the tokens unfortunately.

Yeah, I think any other resolving strategy will lead to something similar like a(n) interpreter/compiler which only adds more complexity and becomes just another GHDL/nvc. I guess for corner cases there could be an exception handling that somewhat does further analysis like seeing if the content within parentheses is valid though. But I guess you guys have more experience resolving this kind of strategy.