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

Add support for VHDL time units #1937

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sparr
Copy link

@sparr sparr commented Jan 3, 2024

https://redirect.cs.umbc.edu/portal/help/VHDL/types.html
https://www.aldec.com/en/support/resources/documentation/articles/1165

This is only a partial fix. A more correct solution would be to create a new non-generic type for time which keeps track of units separately from the value. However, this was the best I could do quickly, and it does succeed at loading the vhdl files I was working with at the time which all use relatively small units.

@@ -287,7 +288,7 @@ private boolean parsePorts(Scanner input) throws IllegalVhdlContentException {
if (!input.next(OPENLIST)) throw new IllegalVhdlContentException(S.get("portDeclarationException"));
parsePort(input);
while (input.next(SEMICOLON)) parsePort(input);
if (!input.next(DONELIST)) throw new IllegalVhdlContentException(S.get("portDeclarationException"));
if (!input.next(DONELIST)) throw new IllegalVhdlContentException(S.get("portDeclarationException") + " before " + input.remaining());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This additional error output was very helpful in identifying the problem.

@BFH-ktt1
Copy link
Collaborator

@sparr : What I am missing, or maybe not understanding correctly, is following: I do not find a possibility to enable or disable the time-units. Hence, how I understand your PR, in VHDL the time-units will always be inserted. If this is the case, it will break a lot in logisim, as synthesis tools will "choke" on the time-units. Please add an option to enable or disable the time-units, with the default to disabled.

@sparr
Copy link
Author

sparr commented Jan 18, 2024

This PR affects loading/parsing of files, not saving/writing.

@BFH-ktt1
Copy link
Collaborator

@sparr : Could you please fix the linter errors, such that I can merge?

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

Successfully merging this pull request may close these issues.

None yet

2 participants