Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why Should Verilog and SystemVerilog source file be split in two classes? #51

Open
RasmusGOlsen opened this issue Jun 19, 2023 · 5 comments

Comments

@RasmusGOlsen
Copy link
Contributor

SystemVerilog is a Verilog release. It can be distinguished by the Verilog version. I don't see any reason for making two different classes.

@Paebbels
Copy link
Member

SystemVerilog is a Verilog release. It can be distinguished by the Verilog version.

Good point.

I don't see any reason for making two different classes.

Currently the file extension is also (implicitly?) bound to the class.
Maybe we need a concept for an n-to-m mapping.

@RasmusGOlsen
Copy link
Contributor Author

RasmusGOlsen commented Jun 19, 2023

I don't think there is anything wrong with having both a VerilogSoureFile and a SystemVerilogSourceFile class. But to share the similarities the SystemVerilogSourceFile class should basically just look like this since the only difference is the value of the Verilog version.

class SystemVerilogSourceFile(VerilogSourceFile):
    pass

@Paebbels
Copy link
Member

This sounds reasonable.

As a consequence, the fields _verilogVersion and _svVersion, and there properties should be merged.

IEEE Std. 1800 superseeded IEEE Std. 1364, so only SystemVerilog parts should exist. On the other hand, Verilog is the common name part in both standards.

We could model it as follows:

  1. derive SystemVerilogSourceFile from VerilogSourceFile.
  2. same for both header file variants SystemVerilogHeaderFile from VerilogHeaderFile.
  3. have only a single _...Version field.
  4. Offer a VerilogVersion property on Verilog classes.
  5. Offer a SVVersion property on SystemVerilog classes.

@Paebbels
Copy link
Member

Latest changes are on dev branch.

@Paebbels
Copy link
Member

As an update: I'm still struggling with a major change to pyTooling (changing to v6.0), which blocks me on other repositories to upgrade and merge items.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants