Parser: Report a Warning if implicit integer conversion truncates value or sign conversion. #717
Conversation
src/aro/Parser.zig
Outdated
| try res.val.intCast(int_ty, p.comp); | ||
| const old_val = res.val; | ||
| const value_change_kind = try res.val.intCast(int_ty, p.comp); | ||
| if (value_change_kind == .value_changed and !(res.ty.makeIntegerUnsigned().specifier == int_ty.makeIntegerUnsigned().specifier)) |
There was a problem hiding this comment.
Is there a specific scenario the !(res.ty.makeIntegerUnsigned().specifier == int_ty.makeIntegerUnsigned().specifier) part guards against (probably better expressed using != instead)?
|
The windows |
|
For the 2 new warnings in |
|
BTW thank you for the kind words and contribution! |
Organ1sm
force-pushed
the
implicit-integer-conversion
branch
from
b375e1a to
2e55dfd
Compare
|
According to your advice, I revised the code and pushed it. Thank you for your code review. :) |
|
Hello, it seems this PR hasn't shown any signs of being merged. I'd like to know if there are any parts of the code that aren't well written so I can continue to improve them. |
|
I had a question about the |
|
For example: The left-hand side and right-hand side of this statement only differ in terms of being unsigned and signed. At the semantic level, there is an implicit conversion. So initially, my approach to solving this was not to add this extra condition, and I also wanted to add a warning for this implicit conversion. However, I later found that both Clang and GCC do not issue warnings for this statement by default. I think we should add a new diagnostic option called |
|
I agree that adding a |
Make `intCast` function return value more specific.
Organ1sm
force-pushed
the
implicit-integer-conversion
branch
from
504e752 to
a3560e3
Compare
Organ1sm
changed the title
src/aro/Value.zig
Outdated
| const value_bits = big.bitCountTwosComp(); | ||
|
|
||
| // if big is negative or zero, then is signed. | ||
| const src_signed = (!big.positive or big.limbs[0] == 0); |
There was a problem hiding this comment.
| const src_signed = (!big.positive or big.limbs[0] == 0); | |
| const src_signed = (!big.positive or big.eqlZero()); |
|
Looks good to me! |
Fix typo Co-authored-by: Evan Haas <[email protected]>
Organ1sm
force-pushed
the
implicit-integer-conversion
branch
from
795bae1 to
26d8718
Compare
src/aro/Value.zig
Outdated
| // if big is negative or zero, then is signed. | ||
| const src_signed = (!big.positive or big.eqlZero()); |
There was a problem hiding this comment.
Why is zero considered signed? Clang and gcc don't do so.
There was a problem hiding this comment.
yes，I tested the behavior of clang and gcc. It was my oversight, so I wrote a new commit to correct this.
…when execute intcast. This change matches the behavior of gcc/clang
When I wanted to learn how to write a C23 C language compiler, this project was extremely helpful to me, no less than having a rich big meal right after finishing a workout. I've been going through this project almost commit by commit, although I haven't finished looking through it all yet. This project not only taught me how to write a C compiler for the C23 standard, but also allowed me to learn a new language, Ziglang, which made me feel very happy. So I want to see if I can do something for this project. I looked at the issue section, and because I don't have experience contributing to open-source projects, I can only start with issues labeled as good first issue.
so I attempted to solve this issue. #701
Value.intCast` currently return a value that indicates whether truncation happens and then issue a diagnostic.