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

Ban mistyped wrapping arithmetic? #8382

Closed
thomastanck opened this issue Mar 28, 2021 · 3 comments
Closed

Ban mistyped wrapping arithmetic? #8382

thomastanck opened this issue Mar 28, 2021 · 3 comments
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@thomastanck
Copy link

I think %- (mod with negated RHS) with no space in between should be a parse error, since the user most likely intended -% and there is a chance (albeit small) that it can parse and run.

This came from a question posted on #zig-help on the Discord community:

    error: negation of type 'u8'
const new = old %- amount;

The poster intended -% but the transposed %- parses as well, and instead the poster got an error about illegally negating amount.

I also managed to come up with a test that causes the unintuitive behaviour to parse and run:

pub fn comptimeWrappingSub(comptime a: comptime_int, comptime b: comptime_int) comptime_int {
    return a %- b;
}

pub fn main() u8 {
    return comptimeWrappingSub(1, -2);
}
// returns 1 instead of 3?

It's surprisingly hard to hit this case, because if a and b were signed, you get an error about needing to use @rem or @mod instead. But here it passes through fine since both a and b are comptime_int.

@jedisct1
Copy link
Contributor

Isn't zig fmt immediately going to turn %- into % -?

With proper highlighting, both symbols are even of different color.

@ifreund
Copy link
Member

ifreund commented Mar 28, 2021

There's a precedent for disallowing confusing (lack of) whitespace like this: #6823.

@ifreund ifreund added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Mar 28, 2021
@ifreund ifreund added this to the 0.8.0 milestone Mar 28, 2021
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@andrewrk andrewrk added the accepted This proposal is planned. label Nov 23, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 24, 2021
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
@Vexu
Copy link
Member

Vexu commented Jan 2, 2023

This now results in:

error: binary operator `%` has whitespace on one side, but not the other.
    return a %- b;
             ^

@Vexu Vexu closed this as completed Jan 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

5 participants