Skip to content

Warn or Error on errors in constants? #49791

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.

Sign up for GitHub

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

Jump to bottom
Closed
oli-obk opened this issue Apr 8, 2018 · 5 comments · Fixed by #50110
Closed

Warn or Error on errors in constants? #49791

oli-obk opened this issue Apr 8, 2018 · 5 comments · Fixed by #50110
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Milestone
1.26

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Apr 8, 2018 •
edited
Loading

Before the miri merger, you were allowed to write pub const FOO: usize = 0 - 1; (producing just a warning), but using it either in this crate or another produced an error.

Since the miri merger, these kinds of constants have become hard errors (just like within statics).

This is of course a breaking change. So we need to decide whether to move back to the warning, or break this now. The warnings have existed for a very long time, but were never moved to deny-by-default

cc @rust-lang/lang

related: #47054
@oli-obk oli-obk added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-lang Relevant to the language team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Apr 8, 2018
@nikomatsakis
Copy link
Contributor

@oli-obk do we have some idea of the fallout here?

@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 12, 2018

Crater didn't find anything on the miri PR. But an old version of winapi is affected. Old versions of other things might be affected, too. Or more windows things. Both are untested on crater

@oli-obk oli-obk mentioned this issue Apr 12, 2018
Closed
@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 20, 2018

This has been "fixed" on beta and nightly (there's no more error), but now there's also no more warning. I'm readding the warning by reintroducing the lint pass that visits all constants and type aliases

pub type Foo = [u8; 0 - 1]; // no warning on beta/nihgtly

@oli-obk oli-obk mentioned this issue Apr 20, 2018
Merged
bors added a commit that referenced this issue Apr 25, 2018
@bors
Auto merge of #50110 - oli-obk:warn_all_the_constants, r=estebank
5ec6637 
Warn on all erroneous constants

fixes #49791
fixes #47054

@Zoxc this PR triggers the nondeterministic errors of #49950 (comment) really often (at least on stage1).
@pietroalbini pietroalbini removed the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Apr 25, 2018
@pietroalbini pietroalbini added this to the 1.26 milestone Apr 25, 2018
@pietroalbini
Copy link
Member

This still needs a backport to beta.

@pietroalbini pietroalbini reopened this Apr 25, 2018
@pietroalbini pietroalbini mentioned this issue Apr 25, 2018
Merged
@pietroalbini
Copy link
Member

Backported to beta.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.26
Development

Successfully merging a pull request may close this issue.

Warn on all erroneous constants oli-obk/rust
3 participants
@nikomatsakis @oli-obk @pietroalbini