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

Improve diagnostic messages for AssignmentStatement, Operation_Binary, ListExpression, and StructExpression #3232

Conversation

kfcripps
Copy link
Contributor

No description provided.

Copy link
Contributor

@mihaibudiu mihaibudiu left a comment

Choose a reason for hiding this comment

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

This doesn't work so well when the printed expression is not one that appeared in the source program. This is not a problem related to this PR, but this PR makes the problem a bit worse.

@@ -1,6 +1,6 @@
parser_pragma2.p4(18): [--Wwarn=parser-transition] warning: SelectCase: unreachable
0xAB00 : Cowles;
^^^^^^
parser_pragma2.p4(16): [--Wwarn=parser-transition] warning: 'ListExpression': transition does not depend on select argument
parser_pragma2.p4(16): [--Wwarn=parser-transition] warning: '{ tmp }': transition does not depend on select argument
Copy link
Contributor

Choose a reason for hiding this comment

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

This is one problem with this improvement: this error message no longer makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that too and I am not sure of a good way to fix this. Is this problem unique to ListExpression components, or is it a problem when printing compiler-generated Expressions in other places, too?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it happens in other places as well.

@@ -1,3 +1,3 @@
issue2317.p4(16): [--Werror=type-error] error: ==: cannot be applied to action results
issue2317.p4(16): [--Werror=type-error] error: f == f: cannot be applied to action results
Copy link
Contributor

Choose a reason for hiding this comment

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

this one is also confusing

Copy link
Contributor Author

@kfcripps kfcripps Apr 20, 2022

Choose a reason for hiding this comment

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

This one can be fixed by enhancing MethodCallExpression's toString() method. This change would affect ~110 tests so I opted to leave that for a separate PR, but I can make that change here if you prefer.
This is not unique to printing MethodCallExpressions used in binary operations, though. The MethodCallExpression will be printed as f in other places too.

@kfcripps
Copy link
Contributor Author

superseded by #4353

@kfcripps kfcripps closed this Jan 23, 2024
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