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

Tracking Issue : Default behaviour on integer arithmetic adding overflow checks #555

Closed
kevaundray opened this issue Dec 3, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@kevaundray
Copy link
Contributor

Problem

When a user does a+b where a and b are u32s. If a+b exceeds the value of a 32 bit integer, the default behaviour is to wrap around and not fail.

There are usecases for not checking integer overflow, and there are usecases for checking integer overflow. This issue is concerned with what the deault behaviour should be.

Solution

Enable overflow checks by default, and if users want to add with overflow, they call a method such as overflow_add

(Describe your suggestion.)

Alternatives considered

Not enable overflow checks and if users want to check for overflow, they call a method named checked_add

Additional context

(If applicable.)

@kevaundray kevaundray added the enhancement New feature or request label Dec 3, 2022
@kevaundray
Copy link
Contributor Author

The main argument against this addition is concerns for performance. Although one should note that the compiler is smart enough to evade some overflow checks due to field sizes.

For example:

fn add3(a : u32, b : u32, c : u32) -> u32 {
  a + b + c
}

In the above code, if overflow checks are enabled then the compiler could add a with b with c as a Field, then check if that result is larger than a u32. Instead of checking if a+b = d overflows then checking if d+c

@guipublic
Copy link
Contributor

The problem is that you need to do the check before and after each subtraction, else you would not detect this case (using u8, simpler to write): 255+5-10.
And then this would not work at all with signed integers (i.e we need to check for every operation)

@guipublic
Copy link
Contributor

Related to PR #2713

@Savio-Sou
Copy link
Collaborator

Closed with #2713.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants