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

New deny(const_err) in unreachable code #52966

Closed
cuviper opened this issue Aug 1, 2018 · 6 comments
Closed

New deny(const_err) in unreachable code #52966

cuviper opened this issue Aug 1, 2018 · 6 comments
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. relnotes Marks issues that should be documented in the release notes of the next release. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cuviper
Copy link
Member

cuviper commented Aug 1, 2018

use std::{u64, usize};

fn main() {
    if (usize::MAX as u64) < u64::MAX {
        println!("{}", 1 + usize::MAX as u64);
    } else {
        println!("disqualified!");
    }
}

(playground)

This compiles and runs fine on a 32-bit target, but fails on 64-bit 1.29.0-beta.1:

error: attempt to add with overflow
 --> src/main.rs:5:24
  |
5 |         println!("{}", 1 + usize::MAX as u64);
  |                        ^^^^^^^^^^^^^^^^^^^^^
  |
  = note: #[deny(const_err)] on by default

The same code is fine without even a warning on 64-bit stable 1.27.2. It's also fine on the pre-release 1.28.0 where const-err became deny-by-default (#50653), because the lint isn't triggering at all.

I'm guessing that const-eval is just evaluating more now, thus triggering the lint more often. If this is decided not to be called a regression, it at least warrants a release note for 1.29.

@cuviper cuviper added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. C-bug Category: This is a bug. A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) labels Aug 1, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Aug 2, 2018

I'm fairly certain this used to also lint last year. This lint occurring again is due to a regression fix for #49760

@pnkfelix pnkfelix added the P-high High priority label Aug 2, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Aug 2, 2018

Assigning high priority, not because this is necessarily a bug that needs to be fixed, but rather so that we remember that we need to decide 1. whether it is a bug, and 2. if its not a bug, whether it warrants release notes.

@oli-obk oli-obk added the relnotes Marks issues that should be documented in the release notes of the next release. label Aug 2, 2018
@cuviper
Copy link
Member Author

cuviper commented Aug 2, 2018

I'm fairly certain this used to also lint last year.

Hmm, you're right -- rustc 1.10 through 1.25 do lint this as a warning. 1.26 through 1.28 do not, and now 1.29 lints it again, denied by default.

@nikomatsakis
Copy link
Contributor

We discussed this in the compiler meeting.

Since this won't affect dependencies, it's not a "true regression", and we're inclined to leave it as is -- @cuviper, thoughts?

(You could also downgrade the lint to allow if desired, of course.)

@cuviper
Copy link
Member Author

cuviper commented Aug 9, 2018

I'm ok with that.

@nikomatsakis
Copy link
Contributor

OK, closing. Thanks @cuviper!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. relnotes Marks issues that should be documented in the release notes of the next release. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants