jeremiah-c-leary / vhdl-style-guide

Style guide enforcement for VHDL
GNU General Public License v3.0
175 stars 38 forks source link

Subtype exceptions #1156

Closed LarsAsplund closed 3 months ago

LarsAsplund commented 3 months ago

Hi,

I'm working in a project which has enabled the prefix rules for (sub)types. All (sub)types must be prefixed with t_. That works well but there is an exception. When specifying a field in a register we use the following construct:

subtype A_FIELD is integer range 7 downto 0;

While A_FIELD is a subtype, it is not used as such and we would like to have an exception. From what I can tell, this the prefix rule for subtypes doesn't support exceptions and even if it did, does exceptions support regexps? Something like `exception = ['*FIELD']?

As a workaround I can:

  1. use vsg_on and vsg_off
  2. prefix the subtype with t_ and then have an alias for the subtype without the prefix.

What I would like is a more direct way to deal with this in the VSG configuration

JHertz5 commented 3 months ago

Hi @LarsAsplund

From your message, it's not clear to me exactly what you're looking for. Are you trying to allow for subtypes with the string FIELD, or with the prefix A_? I have some suggestions that may be helpful though. The rule that checks subtype prefixes is called subtype_004. If you run vsg -rc subtype_004 you can see the default configuration of this rule.

$ vsg -rc subtype_004
{
  "rule": {
    "subtype_004": {
      "indentStyle": "spaces",
      "indentSize": 2,
      "phase": 7,
      "disable": true,
      "fixable": false,
      "severity": "Error",
      "prefixes": [
        "st_"
      ],
      "exceptions": []
    }
  }
}

Option 1: Exceptions

As you can see, there is an exceptions field. You can read about how to use this field here, but as you guessed, I don't think that it supports regexps. If you only need to exclude A_FIELD, you could use the following config

rule:
  subtype_004:
    disable: false
    exceptions: ['A_FIELD']
  subtype_501:
    case: 'upper'

When used on the following file

architecture rtl of test is

  subtype T_ABC is integer range 7 downto 0;
  subtype A_ABC is integer range 7 downto 0;

  subtype A_FIELD is integer range 7 downto 0;

begin

end architecture rtl;

This gives

$ vsg -f test.vhd -c test.yml
================================================================================
File:  test.vhd
================================================================================
Phase 7 of 7... Reporting
Total Rules Checked: 736
Total Violations:    2
  Error   :     2
  Warning :     0
----------------------------+------------+------------+--------------------------------------
  Rule                      |  severity  |  line(s)   | Solution
----------------------------+------------+------------+--------------------------------------
  subtype_004               | Error      |          3 | Prefix T_ABC with one of the following: st_
  subtype_004               | Error      |          4 | Prefix A_ABC with one of the following: st_
----------------------------+------------+------------+--------------------------------------
NOTE: Refer to online documentation at https://vhdl-style-guide.readthedocs.io/en/latest/index.html for more information.

Note that no warning is raised over A_FIELD.


Option 2: Additional valid prefixes

If you're trying to accept both a_ and t_ as valid prefixes, this is something that VSG can already handle. using the config

rule:
  subtype_004:
    disable: false
    prefixes: ['t_', 'a_']

with the file

architecture rtl of test is

  subtype t_abc is integer range 7 downto 0;
  subtype a_abc is integer range 7 downto 0;

  subtype A_FIELD is integer range 7 downto 0;

begin

end architecture rtl;

gives

$ vsg -f test.vhd -c test.yml -ap
================================================================================
File:  test.vhd
================================================================================
Phase 7 of 7... Reporting
Total Rules Checked: 736
Total Violations:    1
  Error   :     1
  Warning :     0
----------------------------+------------+------------+--------------------------------------
  Rule                      |  severity  |  line(s)   | Solution
----------------------------+------------+------------+--------------------------------------
  subtype_501               | Error      |          6 | Change "A_FIELD" to "a_field"
----------------------------+------------+------------+--------------------------------------
NOTE: Refer to online documentation at https://vhdl-style-guide.readthedocs.io/en/latest/index.html for more information.

The error is because this config does not allow for uppercase subtypes.

Do either of these options solve your problem?

LarsAsplund commented 3 months ago

@JHertz5 A_FIELD is just an example identifier for a field of a register. For example, if I have a version register with a major and minor version number, there would be a MAJOR_FIELD and a MINOR_FIELD.

I did try to use the exception as you suggested but for some reason that failed which made me conclude that exceptions aren't supported for subtypes. I have to revisit that to see what went wrong.

What I would like is to exclude all subtype identifiers ending with _FIELD. Is that supported by exceptions or does it only support exact matches?

If supported, I will have a similar style for accessing both fields and individual bits of a register. Register bits are simply defined as constants and I would like the fields to have the same naming convention used for constants in that project.

version_reg(MAJOR_FIELD) interrupt_reg(PACKET_RECEIVED_BIT)

JHertz5 commented 3 months ago

Hi @LarsAsplund

I see, thanks for the clarification. From what I understand, the exceptions field of the subtype prefix rule only supports exact matches and does not support regular expressions or patterns. This does sound like a useful feature to be added.

jeremiah-c-leary commented 3 months ago

Morning @LarsAsplund and @JHertz5 ,

I pushed an update to the issue-1156 branch. When you get a chance could you check it out on your end and let me know what you think?

What I would like is to exclude all subtype identifiers ending with _FIELD. Is that supported by exceptions or does it only support exact matches?

The original implementation only supported exact case insensitive matches. I updated the exceptions to allow for case insensitive regular expressions. This applies to every prefix and suffix rule.

@JHertz5 , thanks for helping to clarify the request.

Regards,

--Jeremy

LarsAsplund commented 3 months ago

Thanks @jeremiah-c-leary, I will give this a try after my Easter break.

/Lars

jeremiah-c-leary commented 3 months ago

Morning @LarsAsplund ,

That is awesome, I will get this merged into master.

--Jeremy