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

File-extension language option not consistently applied #1384

Closed
veripoolbot opened this issue Jan 3, 2019 · 5 comments
Closed

File-extension language option not consistently applied #1384

veripoolbot opened this issue Jan 3, 2019 · 5 comments

Comments

@veripoolbot
Copy link
Contributor

@veripoolbot veripoolbot commented Jan 3, 2019


Author Name: Al Grant
Original Redmine Issue: 1384 from https://www.veripool.org

Original Assignee: Wilson Snyder (@wsnyder)


Suppose you have a Verilog (not SystemVerilog) file testdo1.v:

module testdo1(input do);
endmodule

This uses 'do' as a signal name, so you use +1364-1995ext+v to force .v files to be compiled a Verilog. This generates a SYMRSVDWORD warning but that's ok.

Now suppose you have testdo2.v and compile with +1364-1995ext+v:

module testdo2(input do);
  testdo3 m(do);
endmodule

which automatically pulls in testdo3.v:

module testdo3(input do);
endmodule

This is now faulted with

%Error: testdo3.v:1: Unexpected "do": "do" is a SystemVerilog keyword misused as an identifier.

It looks like the language override is only being applied to explicitly named source files and not to other sources. This isn't specific to the name "do", it affects other words like "byte".

This isn't a mixed-language design, it's pure Verilog, but it seems there is no way to get Verilator to compile as pure Verilog.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Jan 3, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-01-03T23:24:34Z


I don't see anything immediately obviously wrong, but certainly wouldn't surprise me if there are issues.

Could you provide a self-test in verilator test_regress format showing the problem?

Also perhaps attempt a patch? V3Options::fileLanguage would be a good place to start looking.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Jan 21, 2019


Original Redmine Comment
Author Name: Al Grant
Original Date: 2019-01-21T09:40:55Z


It looks like the extension-to-keywords mapping is being applied too early.

When Verilator needs to get a definition for another module, it calls V3ParseImp::parseFile() with 'modfilename' set to the bare module name, and that then calls V3PreShell::preprocOpen(), which calls V3Options::filePath() to turn the module name into a file name. But by that time the `begin_keywords line has already been stacked up on the input and the actual file extension isn't checked.

So in the above example it calls parseFile("testdo3"), generates `begin_keywords "1800-2017", and then later on testdo3.v is found and parsed as SystemVerilog.

I guess this could be fixed simply by moving the file extension check to preprocOpen(), but maybe the language determination belongs in V3Options.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Jan 22, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-01-22T23:48:51Z


Good debugging, that sort of fix makes sense; I'd probably pass the default file type into preprocOpen though am open to alternatives, would you like to send a fix and test?

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Mar 10, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-03-10T18:09:53Z


Fixed in git towards 4.012.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Mar 24, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-03-24T01:15:53Z


In 4.012.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.