jeremiah-c-leary / vhdl-style-guide

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

comment_010 doesn't pickup alignment properly in function returning record types #961

Closed abyszuk closed 1 year ago

abyszuk commented 1 year ago

Environment OS: Ubuntu 22.04 Python: 3.10 VSG: 3.15.0 installed via pip

Describe the bug Given the below MWE I get the following errors:

================================================================================
File:  mwe.vhd
================================================================================
Phase 4 of 7... Reporting
Total Rules Checked: 431
Total Violations:    2
  Error   :     2
  Warning :     0
----------------------------+------------+------------+--------------------------------------
  Rule                      |  severity  |  line(s)   | Solution
----------------------------+------------+------------+--------------------------------------
  comment_010               | Error      |         44 | Use 4 spaces for indent
  comment_010               | Error      |         49 | Use 4 spaces for indent
----------------------------+------------+------------+--------------------------------------
library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

package axi4_interface_pkg is

  --! AXI4 Address Write channel
  type axi4_aw_t is record
    -- AXI4 Lite
    addr  : std_logic_vector;
    prot  : std_logic_vector(2 downto 0);
    valid : std_logic;
    ready : std_logic;
    -- AXI4 Full
    -- AXI recommended 3:0 for master, 7:0 at slave
    id : std_logic_vector;
    -- BurstLength = AxLen+1.  AXI4: 7:0,  AXI3: 3:0
    len : std_logic_vector(7 downto 0);
    -- #Bytes in transfer = 2**AxSize
    size : std_logic_vector(2 downto 0);
    -- AxBurst Binary Encoded (Fixed, Incr, Wrap, NotDefined)
    burst : std_logic_vector(1 downto 0);
    lock  : std_logic;
    -- AxCache One-hot (Write-Allocate, Read-Allocate, Modifiable, Bufferable)
    cache  : std_logic_vector(3 downto 0);
    qos    : std_logic_vector(3 downto 0);
    region : std_logic_vector(3 downto 0);
    user   : std_logic_vector; -- user config
  end record axi4_aw_t;

  --! Channel initialisation (use 'Z' to disable drivers by default)
  function init_axi4_aw (aw : in axi4_aw_t; ival : std_logic := 'Z') return axi4_aw_t;

end package axi4_interface_pkg;

package body axi4_interface_pkg is

  function init_axi4_aw (
    aw   : in axi4_aw_t;
    ival : std_logic := 'Z'
  ) return axi4_aw_t is
  begin
    return (
      -- AXI4 Lite  <<< HERE
      addr   => (aw.addr'range => ival),
      prot   => (aw.prot'range => ival),
      valid  => ival,
      ready  => ival,
      -- AXI 4 Full <<< AND HERE
      id     => (aw.id'range => ival),
      len    => (aw.len'range => ival),
      size   => (aw.size'range => ival),
      burst  => (aw.burst'range => ival),
      lock   => ival,
      cache  => (aw.cache'range => ival),
      qos    => (aw.qos'range => ival),
      region => (aw.region'range => ival),
      user   => (aw.user'range => ival)
    );
  end function init_axi4_aw;

end package body axi4_interface_pkg;

Expected behavior I think that in this case comment should be aligned with the record fields, not with the return statement as that looks a bit awkward:

  function init_axi4_aw (
    aw   : in axi4_aw_t;
    ival : std_logic := 'Z'
  ) return axi4_aw_t is
  begin
    return (
    -- AXI4 Lite  <<< HERE
      addr   => (aw.addr'range => ival),
    -- AXI 4 Full <<< AND HERE
      id     => (aw.id'range => ival),
      len    => (aw.len'range => ival)
    );
  end function init_axi4_aw;
jeremiah-c-leary commented 1 year ago

Morning @abyszuk ,

I pushed an update for this to the issue-961 branch. When you get a chance could you check it out on your end and let me know if it is fixed.

Regards,

--Jeremy

abyszuk commented 1 year ago

Unfortunately that didn't help and seems to brake things even more as now I get tons of errors from many other procedure/function rules

jeremiah-c-leary commented 1 year ago

Morning @abyszuk ,

Could you give me another code example that is failing after my update?

Thanks,

--Jeremy

jeremiah-c-leary commented 1 year ago

Afternoon @abyszuk,

Just wanted to ping you on this issue to see if you provide another example.

Thanks,

--Jeremy

abyszuk commented 1 year ago

Hello @jeremiah-c-leary

Apologies for this really late reply. I've attached an archive with a full VHDL file and a full ruleset file that I use. After applying your patch VSG generates lots of errors - I've tested this by manually changing the VSG installation in my $HOME/.local/lib/python/site-packages directory and re-running VSG. axi4_interface_pkg.zip

jeremiah-c-leary commented 1 year ago

Good evening @abyszuk,

I just ran some experiments on the axi4_interface_pkg.vhd file using the following command: vsg -c vsg_rules.yml -f axi4_interface_pkg.vhd --fix. I was wondering if you are referring to the indenting being off starting at line 306.

Interestingly, when I use the command vsg -f axi4_interface_pkg.vhd --fix, the indenting looks correct. I have not isolated the issue in the vsg_rules.yml yet, I will continue to investigate.

Regards,

--Jeremy

jeremiah-c-leary commented 1 year ago

Hey @abyszuk ,

I think I found the issue. Could you update your vsg_rules.yml file from:

 427     return_statement:
 428       return_keyword:
 429         after: current
 430         token: current

to

 427     return_statement:
 428       return_keyword:
 429         after: '+1'
 430         token: current
 431       semicolon:
 432         token: current
 433         after: '-1'

and let me know if it fixes the problem?

Thanks,

--Jeremy

abyszuk commented 1 year ago

Unfortunately that doesn't help and it breaks VSG:

adrian@pcte247806:~/praca/cce/fgc4$ vsg -c vsg_rules.yml -f common/vhd/axi4_interface_pkg.vhd 
Traceback (most recent call last):
  File "/home/adrian/.local/bin/vsg", line 8, in <module>
    sys.exit(main())
  File "/home/adrian/.local/lib/python3.10/site-packages/vsg/__main__.py", line 108, in main
    oConfig = config.New(commandLineArguments)
  File "/home/adrian/.local/lib/python3.10/site-packages/vsg/config.py", line 211, in New
    dIndent = read_indent_configuration(dConfig)
  File "/home/adrian/.local/lib/python3.10/site-packages/vsg/config.py", line 164, in read_indent_configuration
    dReturn['indent']['tokens'][sGroup][sToken][sParameter] = dGroups[sGroup][sToken][sParameter]
KeyError: 'semicolon'

I've updated to the newest version 3.17.0

jeremiah-c-leary commented 1 year ago

Morning @abyszuk ,

I checked and version 3.17.0 does not have the change required. Could you try with the branch again, or I can show you how to modify the predefined indent configuration to test out on version 3.17.0.

Thanks,

--Jeremy

abyszuk commented 1 year ago

Hello @jeremiah-c-leary ,

Thank you, now I understand it - the default config needs to have all the entries that the custom ruleset has. I've modified the VSG code and I confirm that things seem to work fine now. I think you can merge that branch.

Thank you very much for your work!

jeremiah-c-leary commented 1 year ago

Afternoon @abyszuk ,

Awesome, I will merge this to master.

--Jeremy