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

constant evaluation should not mask overflow for cases that are otherwise checked #22531

Closed
pnkfelix opened this issue Feb 19, 2015 · 4 comments · Fixed by #23863
Closed

constant evaluation should not mask overflow for cases that are otherwise checked #22531

pnkfelix opened this issue Feb 19, 2015 · 4 comments · Fixed by #23863
Labels
A-type-system Area: Type system
Milestone

Comments

@pnkfelix
Copy link
Member

While working on rebasing #20795 I discovered that the new run-fail tests it adds were failing to panic, because the arithmetic in question was being handled by the constant evaluation, and the overflow was thus happening at compile time rather than run time.

An example of such an expression:

fn main() {
    let x = 200u8 + 200u8 + 200u8;
}

(Thank goodness it didn't cause my rustc to panic! That would have been a bit frustrating.)

Anyway, we should fix that, as part of #22020.

That is, the program above should fail to compile, and it should not cause an ICE; it should signal a proper error message saying that the arithmetic expression overflowed (because 200+200+200 == 600 > 255 == u8::MAX).

@oli-obk
Copy link
Contributor

oli-obk commented Feb 19, 2015

cc #13768

@pnkfelix
Copy link
Member Author

nominating, P-backcompat-lang. Suggest 1.0 polish.

Though arguably that combination is oxymoronic, and we should instead classify as 1.0 beta.

@pnkfelix
Copy link
Member Author

I'm going to actually add the tag and milestone myself, in anticipation of @nrc's triage service...

@pnkfelix
Copy link
Member Author

cc #23897

alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 1, 2015
const_eval : add overflow-checking for {`+`, `-`, `*`, `/`, `<<`, `>>`}.

One tricky detail here: There is some duplication of labor between `rustc::middle::const_eval` and `rustc_trans::trans::consts`. It might be good to explore ways to try to factor out the common structure to the two passes (by abstracting over the particular value-representation used in the compile-time interpreter).

----

Update: Rebased atop rust-lang#23841

Fix rust-lang#22531

Fix rust-lang#23030

Fix rust-lang#23221

Fix rust-lang#23235
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants