Deny the overflowing_literals lint for all editions
#55632
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
ollie27
force-pushed
the
deny_overflowing_literals
branch
from
ca22dab to
b049141
Compare
nikomatsakis
added
I-nominated
S-waiting-on-team
|
Nominating for @rust-lang/lang team discussion -- do we want to do this, and -- if so -- what procedure is required? My guess is "yes", we do want to, but we need at least an FCP, and maybe a crater run to assess overall impact? |
|
Or perhaps that was done already, I forget? |
|
@nikomatsakis I don't recall any crater run being done; I did some looking around and couldn't find any such run so I think it wasn't done. To be on the safe side I think we need at least such a crater run to be done. I think if crater shows up 0 or some very very low number then it could be feasible. |
|
I still want this, and we can do it regardless of crater because it's just a lint level. Obviously running a crater run for it is nice -- and is relatively cheap, since it's just a check run with |
|
We discussed this in the lang team meeting today. Generally everyone is in favor, but we would like to see a crater run just to assess whether it'd be helpful to adjust some key crates before this gets fully rolled out. |
|
@bors try |
|
⌛ Trying commit b049141773332c40635f9da831beb9e81d6387db with merge dc13be39fae8d4c607889b27de374b52586485a3... |
|
☀️ Test successful - status-travis |
|
@craterbot run start=master#653da4fd006c97625247acd7e076d0782cdc149b end=try#dc13be39fae8d4c607889b27de374b52586485a3 mode=check-only I think this should be the right config but not entirely certain (lints are odd, sometimes). Crater queue is empty anyway. |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
|
craterbot
added
S-waiting-on-review
|
@ollie27 could you reach out to affected regressed crates (with PRs perhaps) so that we can move ahead? |
bors
added
S-waiting-on-author
|
That should be fixed now. |
|
@bors r+ |
|
📌 Commit e7296fd has been approved by |
bors
added
S-waiting-on-bors
…r=Centril Deny the `overflowing_literals` lint for all editions The `overflowing_literals` was made deny by default for the 2018 edition by rust-lang#54507, however I'm not aware of any reason it can't be made deny by default for the 2015 edition as well.
…r=Centril Deny the `overflowing_literals` lint for all editions The `overflowing_literals` was made deny by default for the 2018 edition by rust-lang#54507, however I'm not aware of any reason it can't be made deny by default for the 2015 edition as well.
…r=Centril Deny the `overflowing_literals` lint for all editions The `overflowing_literals` was made deny by default for the 2018 edition by rust-lang#54507, however I'm not aware of any reason it can't be made deny by default for the 2015 edition as well.
Rollup of 10 pull requests Successful merges: - #55632 (Deny the `overflowing_literals` lint for all editions) - #58687 (Reduce Miri Code Repetition like `(n << amt) >> amt`) - #58690 (Reduce a Code Repetition like `(n << amt) >> amt`) - #58718 (Apply docs convention: Replace # Unsafety with # Safety in docs) - #58719 (librustc_codegen_llvm: #![deny(elided_lifetimes_in_paths)]) - #58720 (librustc_codegen_ssa: #![deny(elided_lifetimes_in_paths)]) - #58722 (librustc_typeck: deny(elided_lifetimes_in_paths)) - #58723 (librustc: deny(elided_lifetimes_in_paths)) - #58725 (Test that binop subtyping in rustc_typeck fixes #27949) - #58727 (bootstrap: deny(rust_2018_idioms)) Failed merges: r? @ghost
|
FWIW, this broke byteorder's test suite. It looks like many of its doc tests were (incorrectly) using overflowing literals. I don't set Obviously, I will just fix the doc tests here, but I'm curious why this change was made in Rust 2015. The original RFC seems to target Rust 2018, probably because this is a breaking change, no? Have we converted lints to More pragmatically, there may be other breakage in the ecosystem that is being missed because it seems like the initial crater run didn't catch this. |
|
FWIW we can probably do another crater run in rustdoc mode (I think that'll catch the lint errors, but not sure). How did you notice that it was broken? |
|
@Dylan-DPC did a crater run: BurntSushi/byteorder#144 I'm not too familiar with crater and what it can and can't catch, so I'm not sure why it wasn't caught before. But if there's a rustdoc mode, then that might explain things. I also usually have daily cron jobs setup on CI, but it wasn't enabled for byteorder. I added that back in, so that would have caught it too once the new nightly was released. |
|
I think #41574 would have helped catch the bug on my end as soon as the overflowing literal made it into the doc test. |
|
Yeah, that would've been the beta crater run -- interestingly, not the rustdoc mode. (https://crater-reports.s3.amazonaws.com/beta-1.34-1/beta-2019-02-27/reg/byteorder-1.2.7/log.txt is the relevant log). I think crater presumably invokes rustdoc with that flag? Not sure why it wasn't caught in crater on this PR though. |
In a future version of Rust the `overflowing_literals` lint will become deny by default. See rust-lang/rust#55632. When checking for breakage in crates uploaded to crates.io it was discovered that this crate will no longer compile thanks to this lint. The error produced is [here](https://crater-reports.s3.amazonaws.com/pr-55632/try%23dc13be39fae8d4c607889b27de374b52586485a3/gh/BProg.des_chipher_rust/log.txt). This PR should fix the false positive.
The
overflowing_literalswas made deny by default for the 2018 edition by #54507, however I'm not aware of any reason it can't be made deny by default for the 2015 edition as well.