Skip to content

Added support for >>> operator and simple for loops #416

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

Merged
merged 3 commits into from
Dec 8, 2018

Conversation

Scott-Young-6746
Copy link
Contributor

Description

Added support for the {VAR, NUM} >>> NUM operator.
Added support for for loops of the following form:

for(VAR = NUM; VAR {<, <=, >=, >, ==, != } NUM; VAR = VAR {+, -, *, /} NUM;) STATEMENT

Where STATEMENT does not contain any lines of the form VAR = EXPRESSION
NUM is a constant number after the folding of constant expressions
VAR is a symbol.

Related Issue

Fixes issues #415 and #87 .

Motivation and Context

Adds language coverage for a new operator and fixes a long standing bug.

How Has This Been Tested?

Tested with two new regression tests (one for each change), the verify_odin.sh script and a clean compile of the code submitted with issue #87 (which did not contain expected output for given input and was therefore tested only to the point were it did not reproduce the problem documented in the issue.)

Types of changes

  • [ x ] Bug fix (change which fixes an issue)
  • [ x ] New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [ x ] My change requires a change to the documentation
  • I have updated the documentation accordingly
  • [ x ] I have added tests to cover my changes
  • [ x ] All new and existing tests passed

@probot-autolabeler probot-autolabeler bot added lang-cpp C/C++ code lang-hdl Hardware Description Language (Verilog/VHDL) Odin Odin II Logic Synthesis Tool: Unsorted item tests labels Dec 4, 2018
.gitignore Outdated
#
# Personal circuit scratch directory
#
circuits/
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like an individual developer configuration, it probably shouldn't end up in the master branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed a change to remove this from the pull request

@kmurray
Copy link
Contributor

kmurray commented Dec 5, 2018

Thanks for the pull request! I'll defer to @jeanlego for the ODIN related changes.

Copy link
Contributor

@jeanlego jeanlego left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! are the test robust enough to be included as part of the daily regression suite? and are any of them failing? if so move the failing one into a separate directory (there is a _bug test directory for the test that needs fixing).

also, there are a lot of whitespace changes if you could clean those (or omit altogether) as they break the alignments in a lot of cases.

also, I think more test cases for 'for loop' would help keep this feature functional in the long term.

@Scott-Young-6746
Copy link
Contributor Author

@jeanlego I had been using tabs->spaces in my editor and noticed that the where I made changes to code it had replaced the tabs that were there with spaces. The white space changes you see in fixed indentation were replacing the 4 spaces for indentation I had used with tabs.

Assuming this is not what you are referring to in your comment, can you highlight places I missed so I can change them? I would like to ensure all my changes use the same indentation to avoid inconsistent indentation across editors.

I'll add some failing tests to the _bug folder and push that as soon as possible. I'll also add a test to see if creating modules in a loop works correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code lang-hdl Hardware Description Language (Verilog/VHDL) Odin Odin II Logic Synthesis Tool: Unsorted item tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants