lint: Use rustc_apfloat for overflowing_literals, add f16 and f128
#151529
Conversation
rustbot
added
S-waiting-on-review
|
r? @nnethercote rustbot has assigned @nnethercote. Use |
| $(impl GeneralFormat for $t { | ||
| fn already_rounded_value_should_use_exponential(&self) -> bool { | ||
| #[allow(overflowing_literals)] // f16 is always within this value | ||
| let max_abs = 1e+16; |
There was a problem hiding this comment.
I found this comment hard to understand. I think you're saying that 1e+16 is too big too represent as an f16, which is why we get the error, so we need to suppress that with the allow. Is the abs >= max_abs comparison still valid? Does 1e+16 get rounded up to positive infinity and therefore the comparison always fails?
Some more detail is needed to make this clear.
There was a problem hiding this comment.
That's exactly it; abs >= 1e+16 is always true since f16 has a max of 65504.0. In context here, it just means that there isn't a f16 with a positive exponent that will get formatted in scientific notation by default.
I'll clarify this comment soon.
|
Nice change. r=me with the nit fixed. |
Switch to parsing float literals for overflow checks using `rustc_apfloat` rather than host floats. This avoids small variations in platform support and makes it possible to start checking `f16` and `f128` as well. Using APFloat matches what we try to do elsewhere to avoid platform inconsistencies.
tgross35
force-pushed
the
lint-apfloat
branch
from
1c65c55 to
9b15010
Compare
|
Thanks for reviewing, updated the comment to be more clear. @bors r=nnethercote rollup |
rust-bors
bot
added
S-waiting-on-bors
Rollup of 12 pull requests Successful merges: - #147996 (Stabilize ppc inline assembly) - #148718 (Do not mention `-Zmacro-backtrace` for std macros that are a wrapper around a compiler intrinsic) - #151137 (checksum-freshness: Fix invalid checksum calculation for binary files) - #151680 (Update backtrace and windows-bindgen) - #150863 (Adds two new Tier 3 targets - `aarch64v8r-unknown-none{,-softfloat}`) - #151040 (Don't expose redundant information in `rustc_public`'s `LayoutShape`) - #151383 (remove `#[deprecated]` from unstable & internal `SipHasher13` and `24` types) - #151529 (lint: Use rustc_apfloat for `overflowing_literals`, add f16 and f128) - #151669 (rename uN::{gather,scatter}_bits to uN::{extract,deposit}_bits) - #151689 (Fix broken Xtensa installation link) - #151699 (Update books) - #151700 (os allow missing_docs)
| // `max_abs` rounds to infinity for `f16`. This is fine to save us from a more | ||
| // complex macro, it just means a positive-exponent `f16` will never print as | ||
| // scientific notation by default (reasonably, the max is 65504.0). | ||
| #[allow(overflowing_literals)] |
Rollup merge of #151529 - tgross35:lint-apfloat, r=nnethercote lint: Use rustc_apfloat for `overflowing_literals`, add f16 and f128 Switch to parsing float literals for overflow checks using `rustc_apfloat` rather than host floats. This avoids small variations in platform support and makes it possible to start checking `f16` and `f128` as well. Using APFloat matches what we try to do elsewhere to avoid platform inconsistencies.
|
Perf results show regressions in |
|
As long as this only shows up in stress tests, and provides more consistent behavior overall, I think it is fine. |
5 participants
Switch to parsing float literals for overflow checks using
rustc_apfloatrather than host floats. This avoids small variations in platform support and makes it possible to start checkingf16andf128as well.Using APFloat matches what we try to do elsewhere to avoid platform inconsistencies.