Skip to content

[#5331] Do not emit mismatch or overflow warnings for explicit casts #5334

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

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

Conversation

kfcripps
Copy link
Contributor

Addresses the issue of explicit casts being warned about, as discussed in #5331.

@kfcripps kfcripps requested review from asl and ChrisDodd June 30, 2025 15:14
@kfcripps kfcripps added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Jun 30, 2025
@kfcripps kfcripps force-pushed the 5331-cast-warning branch from 00259cc to 9e9ca07 Compare June 30, 2025 15:25
@kfcripps
Copy link
Contributor Author

The last test case that I added shows an example where this PR does not warn about an implicit cast when it should be (before this PR, it was warned about). I am not sure if it would be better to add this warning to TypeInference (where the constant 72 is transformed into (bit<4>)72), or to propagate information to ConstantFolding about whether an IR::Cast was implicit or explicit so that it can decide to warn about implicit casts and not explicit casts.

@kfcripps kfcripps marked this pull request as draft June 30, 2025 16:14
@kfcripps kfcripps force-pushed the 5331-cast-warning branch from afab515 to 842ff9c Compare June 30, 2025 16:21
@kfcripps
Copy link
Contributor Author

or to propagate information to ConstantFolding about whether an IR::Cast was implicit or explicit so that it can decide to warn about implicit casts and not explicit casts.

I implemented this option for now.

@kfcripps kfcripps marked this pull request as ready for review June 30, 2025 18:18
@kfcripps kfcripps force-pushed the 5331-cast-warning branch from d6ea299 to fd1d673 Compare June 30, 2025 22:29
@kfcripps kfcripps requested review from oleg-ran-amd, asl and ChrisDodd and removed request for asl and ChrisDodd July 1, 2025 22:09
@@ -516,6 +516,11 @@ class This : Expression {

class Cast : Operation_Unary {
Type destType = type;
bool implicit = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

If you make this optional bool implicit = false;, the two ctors will be autogenerated for you.

Also, the following comment is about destType, so this should be after that, not between the declaration of destType and the comment. Though given it is a /// comment it should be before the declaration, not after it (or have a <), so that Doxygen associates it with the member properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok the placement of that comment did seem strange to me.. I will move it above the destType declaration.

@kfcripps kfcripps force-pushed the 5331-cast-warning branch from fd1d673 to 0010585 Compare July 4, 2025 13:46
@kfcripps kfcripps requested a review from ChrisDodd July 4, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Topics concerning the core segments of the compiler (frontend, midend, parser)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants