Closed RalfJung closed 1 year ago
The problem is that a false-positive error is triggered with the MSRV 1.38.0, see CI log from draft PR #69.
cargo-bisect-rustc tells me that this was fixed in nightly-2020-02-21 (regression = success). So I can remove allow(const_err)
but then I need to bump the MSRV to 1.43.0.
********************************************************************************
Regression in nightly-2020-02-21
********************************************************************************
fetching https://static.rust-lang.org/dist/2020-02-20/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2020-02-20: 40 B / 40 B [===================================] 100.00 % 275.82 KB/s converted 2020-02-20 to 7760cd0fbbbf2c59a625e075a5bdfa88b8e30f8a
fetching https://static.rust-lang.org/dist/2020-02-21/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2020-02-21: 40 B / 40 B [===================================] 100.00 % 257.34 KB/s converted 2020-02-21 to 2c462a2f776b899d46743b1b44eda976e846e61d
looking for regression commit between 2020-02-20 and 2020-02-21
opening existing repository at "/Users/hans/dev/rust"
Found origin remote under name `origin-https`
refreshing repository at "/Users/hans/dev/rust"
fetching (via local git) commits from 7760cd0fbbbf2c59a625e075a5bdfa88b8e30f8a to 2c462a2f776b899d46743b1b44eda976e846e61d
opening existing repository at "/Users/hans/dev/rust"
Found origin remote under name `origin-https`
refreshing repository at "/Users/hans/dev/rust"
looking up first commit
looking up second commit
checking that commits are by bors and thus have ci artifacts...
finding bors merge commits
found 8 bors merge commits in the specified range
commit[0] 2020-02-19UTC: Auto merge of #69293 - Dylan-DPC:rollup-imcbvgo, r=Dylan-DPC
commit[1] 2020-02-19UTC: Auto merge of #68988 - Zoxc:query-caches, r=eddyb
commit[2] 2020-02-20UTC: Auto merge of #69256 - nnethercote:misc-inlining, r=Centril
commit[3] 2020-02-20UTC: Auto merge of #67925 - petertodd:2020-fromstr-infallible, r=LukasKalbertodt
commit[4] 2020-02-20UTC: Auto merge of #68847 - ecstatic-morse:const-impl, r=oli-obk
commit[5] 2020-02-20UTC: Auto merge of #69309 - Dylan-DPC:rollup-gjdqx7l, r=Dylan-DPC
commit[6] 2020-02-20UTC: Auto merge of #69145 - matthewjasper:mir-typeck-static-ty, r=nikomatsakis
commit[7] 2020-02-20UTC: Auto merge of #69325 - Centril:rollup-vce2ko2, r=Centril
ERROR: no CI builds available between 7760cd0fbbbf2c59a625e075a5bdfa88b8e30f8a and 2c462a2f776b899d46743b1b44eda976e846e61d within last 167 days
Oh interesting, I didn't know/remember we had such false positives in the past. I hope they are fixed now (but I assume we would have heard about them otherwise^^).
You could also remove the deny(warnings)
and instead set RUSTFLAGS="-D warnings -A const_err"
on CI only for the MSRV build (and -D warnings
for everything else), then the
'const_err lint has been removed' warning won't cause a build failure.
(Anyway your users are fine since all lints are capped to warnings for dependencies. Still AFAIK the usual advice is not to put deny(warnings)
into the code.)
That nightly was the first to ship with https://github.com/rust-lang/rust/pull/69185, which split some things out of const_err
into new lints unconditional_panic
and arithmetic_overflow
. So it probably didn't remove the false positive, just shifted it to a different lint.
Looking at the code, this is indeed an unconditional panic, but inside dead code. This is another instance of the same issue. It is currently by-design and hard to fix but indeed somewhat unfortunate (and might be an argument for making the lint warn-by-default).
Anyway I just wanted to make sure there is not some subtle bug in our diagnostics here that I did not know about. :)
This crate carries a
allow(const_err)
. That lint is going away since it becomes a hard error, which causes a warning due to the removed lint being used, which then triggersdeny(warnings)
.The crate does not actually seem to trigger const_err (according to crater), so the hard error itself should not be a problem. The
allow(const_err)
can likely just be removed.