Suggest to change numeric literal instead of casting #54273
Conversation
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
src/librustc_typeck/check/demand.rs
Outdated
| { | ||
| // 42u8 | ||
| // ^^ | ||
| let lit_offset = src.len() - checked_ty.to_string().len(); |
There was a problem hiding this comment.
oops, brute sub should be optimized.
src/librustc_typeck/check/demand.rs
Outdated
| expected_ty, | ||
| ), | ||
| suffix_suggestion, | ||
| Applicability::MaybeIncorrect, |
There was a problem hiding this comment.
I feel these might count as MachineApplicable...
src/librustc_typeck/check/demand.rs
Outdated
| ), | ||
| suffix_suggestion, | ||
| Applicability::MaybeIncorrect, | ||
| ); |
There was a problem hiding this comment.
I feel there's a bit of duplication on these, would it make sense to have is_suffixed (with a more accurate name) take mut err, checked_ty and expected_ty, and if it would return true now it would also apply the suggestion, and here you just call it as if !is_sufficed(expr) { with the following block (as each one of these has slightly different wording).
| fn foo(_: u16) {} | ||
| fn main() { | ||
| foo(8u8); | ||
| } |
There was a problem hiding this comment.
Can you add a test for floats too?
estebank
added
S-waiting-on-author
src/librustc_typeck/check/demand.rs
Outdated
| expr: &hir::Expr, | ||
| expected_ty: Ty<'tcx>, | ||
| checked_ty: Ty<'tcx>, | ||
| note: Option<&str>| { |
There was a problem hiding this comment.
It's the first time I've seen a closure that needed multiple arguments and I'm surprised rustfmt suggests exactly this :)
There was a problem hiding this comment.
oh, expr and *_ty aren't needed actually, I'll remove them.
|
@bors r+ rollup |
|
📌 Commit c3b6e3f2ac2c8ef64168db4336645399380c5fdd has been approved by |
bors
added
S-waiting-on-bors
kennytm
added
S-waiting-on-review
|
@bors r+ |
|
📌 Commit 2fb6585 has been approved by |
bors
added
S-waiting-on-bors
Suggest to change numeric literal instead of casting Closes rust-lang#54160 r? @estebank
Rollup of 9 pull requests Successful merges: - #53522 (Add doc for impl From for Addr) - #54097 (rustdoc: Remove namespace for keywords) - #54205 (Add treat-err-as-bug flag in rustdoc) - #54225 (Regression test for #53675.) - #54232 (add `-Z dont-buffer-diagnostics`) - #54273 (Suggest to change numeric literal instead of casting) - #54299 (Issue 54246) - #54311 (Remove README with now-out-of-date docs about docs.) - #54313 (OsStr: Document that it's not NUL terminated) Failed merges: r? @ghost
Closes #54160
r? @estebank