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

Print single instead of double equals sign #20

Merged
merged 2 commits into from
Mar 13, 2021

Conversation

LincB
Copy link
Contributor

@LincB LincB commented Mar 13, 2021

ACT currently prints the E_EQ expression with a double equals sign, but the parser expects a single equals sign, so some expressions printed using the ACT tools aren't parsable. This commit makes the expression print function use a single equals sign.

Since the expression printing code is widely used, there may be potential consequences to this change that I don't anticipate, so please let me know if this change isn't possible for some reason.

@rmanohar
Copy link
Member

This looks good, but it breaks one of the test cases. The test case "golden output" needs to be updated (it prints out an error message with ==, which now changes to =). Could you submit an updated pull request with this change?

Run "make runtest" in the act/ directory to see the failing case. The failing case is misc/8.act. To update the correct test case output, run ../validate_subdir.sh 8.act in the misc/ directory. That creates the "golden output" for the test case based on the act-test binary. Now make runtest should pass. git status will show you what was modified.

The new pull request should have your expr2.cc change, plus the new misc/runs/8.act.stderr

@LincB
Copy link
Contributor Author

LincB commented Mar 13, 2021

I added a commit to this pull request with the updated golden output. Please let me know if there are any other issues!

@rmanohar rmanohar merged commit 75a22c9 into asyncvlsi:master Mar 13, 2021
@LincB LincB deleted the single-equals branch March 13, 2021 19:49
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.

2 participants