Generic case is not fixed everywhere

[Emilv2]

Environment

VHDL Style Guide (VSG) version 2.0.0

Describe the bug

To Reproduce

vsg -f test.vhd --fix

-- test.vhd 
entity test is
  generic (
    my_generic : std_logic
  );
  port (
    output : out   std_logic
  );
end entity test;

architecture rtl of test is

begin

  output <= My_gEneriC;

end architecture rtl;

Expected behavior I would expect the case of My_gEneriC being fixed according to the settings of rule generic_007

Additional context Thank you for this progam, it looks really good :)

[jeremiah-c-leary]

Hey @Emilv2 , This is similar to rule signal_004. I created another rule signal_024 that checked for the consistent case of signals.

Generics are an interesting case. If the entity is not declared in the same file, then there will be nothing to check against. Another corner case is if there are multiple architectures defined in a file.

If you don't mind, could we change this to an enhancement and I can work on a consistent case rule for generics.

I could probably do the same for ports.

hmm....I wonder if the entity and architecture are in a separate file, but you analyze them together could the checks be done. I would have to keep a history of generics and ports, but it could be done.

Thank you for this progam, it looks really good :)

Thanks for the kind words. I am happy you are finding value in it.

[Emilv2]

I didn't think of those corner cases, do with this what you think is best and realisable.

I haven't taken a look at your code, but I dont' think you're really parsing vhld since vsg also works on invalid vhdl. In an ideal case I'd say you would have to write a complete vhld parser to solve issues like this. Or use one, it should be possible to use GHDL (https://github.com/ghdl/ghdl/issues/1156) and/or maybe https://github.com/Paebbels/pyVHDLParser since that one preserves comments, whitespace and case. Totally understand it if this would be out of scope for this project for you.

[jeremiah-c-leary]

Funny you mention that, there was an issue that talked about it #312 . I am currently in the process of switching to a better parser, or one could argue a real parser. I was originally using regular expressions. While it worked for generally well formatted code, it could not handle anything too extraordinary.

I think I have decided that I need to abstract away the parser so I can focus on the rules. Unfortunately, getting to that point will be painful.

[umarcor]

@jeremiah-c-leary you might find ghdl/ghdl#1449 interesting. It's an exercise I did for using GHDL's Python interface to libghdl as a parser. So, given a source file, it extracts the list of ports of an entity, the type and direction. It also shows architecture names and the entity they belong to. The Python code of that exercise is the following: https://github.com/ghdl/ghdl/blob/master/testsuite/python/units01/show_ports.py

There is also the language server: https://github.com/ghdl/ghdl/blob/master/python/vhdl_langserver/document.py#L204.

As discussed in that issue and in ghdl/ghdl/issues/1437, that's a very low-level binding to Ada sources. It would be useful to write a more pythonic interface on top of that. However, the underlaying codebase is GHDL, so you get all it's language support already.

[jeremiah-c-leary]

Hey @umarcor, Thanks for the link. I decided to implement my own parser and I am currently in the process of refactoring all my rules to use the output of the new parser.

One question, why were you trying to pull out port names etc...?

[umarcor]

@jeremiah-c-leary thanks for asking. It's a proof of concept for umarcor/hwstudio. That's an structural diagram editor for modern (V)HDL designs, written in Godot (an open source 2D/3D game engine). Tarballs are available for Linux, Windows or Mac (see nightly). There is also an HTML5 version (see umarcor.github.io/hwstudio).

As explained in section (V)HDL centric design of the doc, the main design principle is for users to build schematic connections based on existing VHDL 2008 codebases. That's opposite to most existing "HDL diagram" tools, where entries are graphical only, and then HDL code is generated or some other internal language is used for simulation.

An important design goal is the Integration with other tools. That is, users are expected to use VSCode/vim/Emacs for editing their sources, as they are used to. Then, hwstudio would be a complement/helper for understanding and designing the integration of multiple modules. You will find references to TerosHDL (and vhdl-style-guide, symbolator, wavedrom, GHDL's language server, etc.) all along the doc. Thet's because I don't want to reinvent a single feature.

Have a look at the "blocks" in the demo. Each of them has an instance name, entity name, architecture name, generics and ports. Hence, my experiment with GHDL's Python interface was for extracting those. The idea is as follows:

• Users provide the path or URL to some repository(ies) containing VHDL sources.
• All the sources in the repo(s) are parsed with GHDL.
  • Lists of entities and architectures are extracted.
  • For entities, generics and ports are extracted too.
• The "Repositories" window of hwstudio is filled with the entities, organized in repos/libs.
• Users can "Add" blocks to the diagram. When doing so, they select the instance name, the entity and the architecture. Then, they "drag" the ports for (re)doing connections.

Godot has a built-in "graphical programming language" named VisualScript (see e.g. v=NpE1ig6NdcA). From a technical point of view, it's built on top of classes GraphEdit and GraphNode. The interesting point is that "port types" and "port direction" are built-in features in those classes (see set_slot). For instance, you will see in umarcor.github.io/hwstudio that ports of type out can only be connected to ports of type in. All of them are of the same type yet, but I hope you get the idea. Moreover, that's why I experimented with using GHDL for extracting the name, direction and type of the ports.

Moreover, that experiment alone allows updating the parser of Symbolator. In the end, the blocks in hwstudio are equivalent to the symbolator symbols for documentation. I commented this with TerosHDL developers (@smgl9, @qarlosalberto), but not with @kevinpt yet.

The next step would be to integrate the hwstudio frontend with GHDL's Python interface. However, there are two main challenges to address:

• There is no standard distribution format for (V)HDL "repos". VUnit, PoC, edalize/fusesoc, tsfpga, IP-XACT... I refuse to support all of them, even to implement a single one. I am a heavy user of GHDL and VUnit, hence, I could reuse run.py or hdl-prj.json files. Still, in the context of hwstudio, implementations should be robust, not fast. There are already lots of partial solutions which prioritized time over quality. Hence, this is delayed until eine/vhdl-cfg or some similar solution is usable.
  • There is a discussion in https://umarcor.github.io/hwstudio/doc/#_structure about format *.hwf. However, that is for saving the diagrams to disk, i.e., only the content which is not already defined in (V)HDL. Existing (or generated) (V)HDL sources should use a format/structure standard in the community.
• Constraining port types to std_logic_vector or bit_vector would significantly simplify the management of multiple types. In the end, the type would be reduced to the length of the port (in number of bits). Hence, that might be an initial approach. It would also simplify the integration of VHDL and (System) Verilog modules. However, in the mid-long term, "complex" features from VHDL 2008 and VHDL 2019 should be supported. That is, custom record types, multidimensional arrays, or interfaces should be supported port types. This is related to the analysis in Reading waveforms from HDL simulators with PulseView and sigrok's/pulseview's protocol decoders (written in Python). The complexity relies on replicating in hwstudio the structure of defined types in (V)HDL sources, including all the scope/visbility constraints. It is relatively straightforward to represent the codebase, but allowing graphical tweaks/modifications to the design is tricky.

[jeremiah-c-leary]

@Emilv2 , I am finally ready to start fixing issues again after my parser update. I think it would be feasible to do a consistent case for generics. If the entity and architecture are in the same file, then it would be checked. If not, then it will not. This rule would also check generics within the entity also.

I will make this part of the 3.0.0 release.

[jeremiah-c-leary]

Hey @Emilv2 ,

I just pushed an update to master for this issue. There are two new rules: entity_600 and architecture_600. The first check for consistent generic usage within an entity declaration. The second checks for consistent generic usage within an architecture body.

When you get a chance, could you check it out and let me know if it works for you.

Otherwise, I will close this issue when I release 3.0.0.

Thanks,

--Jeremy

[Emilv2]

I tried adding those to my configuration file but I get the error ERROR: Rule architecture_600 referenced in configuration could not be found and ERROR: Rule entity_600 referenced in configuration could not be found

rule:
  architecture_600:
     case: 'upper'
  entity_600:
     case: 'upper'

Am I doing something wrong?

(running on latest master vsg --version:

VHDL Style Guide (VSG) version 2.2.0
Git commit SHA: 8d8169db

)

[jeremiah-c-leary]

Hey @Emilv2 ,

You will have to pull down master using the Git Hub instructions in installation documentation:

When you issue a version command the version should come back as master.

--Jeremy

[Emilv2]

Ok, seems I looked over that. Version showing the last git commit made it a bit confusing.

The docs don't actually seem to specify how to configure this (and they seem to allow invalid configuration), I can't figure out what entity_600 and architecture_600 do. Only generic_007 seems to fix the case for me. (Which is good enough for me, but I figure you added those options for a reason?)

Also, the violation and fix in the architecture_600 docs are identical.

Thanks for all your work, looks like it's getting along pretty good!

[jeremiah-c-leary]

Version showing the last git commit made it a bit confusing.

Agree. I have two tickets trying to address the version reporting.

The docs don't actually seem to specify how to configure this...

You can see all the configuration options of a rule by using the -rc option at the command line:

$ vsg -rc entity_600
{
  "rule": {
    "entity_600": {
      "indentSize": 2,
      "phase": 6,
      "disable": false,
      "fixable": true,
      "severity": "Error"
    }
  }
}

The configurable items for entity_600 only include the base rule configurable items. All rules have these items. If you check generic_007 you will get:

$ vsg -rc generic_007
{
  "rule": {
    "generic_007": {
      "indentSize": 2,
      "phase": 6,
      "disable": false,
      "fixable": true,
      "severity": "Error",
      "case": "lower"
    }
  }
}

You will see the addition of case to the configurable items.

I can't figure out what entity_600 and architecture_600 do.

So generic_007 sets the case for generic identifiers. It's scope is limited to interface_object_declaration identifiers. The rules entity_600 and architecture_600 then use the results from generic_007 and search the rest of the code for occurrences of the generic identifier and ensure it's case matches.

Looking specifically at entity_600:

In the violation code, the generic G_WIDTH is upper case but it's usage in the two port declarations is lower case. The solution is to change g_width to G_WIDTH in the port declarations, as shown in the fix code.

Also, the violation and fix in the architecture_600 docs are identical.

Good catch. I will push an update.

Thanks for all your work, looks like it's getting along pretty good!

Thanks for the kind words. Having people like you use it and feeding back issues is really helpful. VSG has come a long way from when I first published about two years ago, and it is all due to the user community.

So thank you for using it and helping to make it better.

Regards,

--Jeremy

[Emilv2]

I see now. My requirements are pretty simple so I can't really comment on whether entity_600 and architecture_600 are useful.

For me it works like this so I'll close the issue