nickg / nvc

VHDL compiler and simulator
https://www.nickg.me.uk/nvc/
GNU General Public License v3.0
631 stars 77 forks source link

Fix basic_identifier #896

Closed typingArtist closed 4 months ago

typingArtist commented 4 months ago

In the LRM (VHDL-2008 at least) basic identifiers are defined as:

 basic_identifier ::= letter { [ underline ] letter_or_digit }

The LRM defines brackets [] as optional elements (regex question mark ?) and braces {} as optional repeated (zero or more, regex asterisk *).

For underlines, this means that an identifier must never start with it, end with it, and there must never be more than one underscore in sequence. Before this PR, nvc doesn’t parse basic identifiers according to that definition but permits underscores in any place of an identifier, except for the first position. This patch checks for the proper behavior and also fixes the issue in the parser.

With just 47d0963 applied, make check fails. 62c2c0a fixes the issue. Neither verilog nor tcl were enabled, so a couple of unrelated tests have not been run.

While regenerating test/dist.mk, a lot of missing entries show up, but for atomicity of the patch, only the new dependency was added.

nickg commented 4 months ago

While regenerating test/dist.mk, a lot of missing entries show up, but for atomicity of the patch, only the new dependency was added.

I normally only regenerate that when I'm about to make a release. It's not necessary to keep it always in sync.

nickg commented 4 months ago

I extended this a bit in 8fd2b74 so it now prints a more helpful error message:

** Error: a VHDL basic identifier must not start or end with an underscore, or contain 
          multiple consecutive underscores
   > ../test/parse/basic_identifier.vhd:1
   |
 1 | entity ee_ is end entity; -- ERROR expecting is (after ee)
   |        ^^^
   |
   = Help: IEEE Std 1076-2008 section 15.4.2 "Basic identifiers"