Fix overflows in the implementation of `overflowing_literals` lint's help #135249

s-cerevisiae · 2025-01-08T11:16:16Z

This PR fixes two overflow problems that cause the overflowing_literals lint to behave incorrectly in some edge cases.

When an integer literal is between i128::MAX and u128::MAX, an overflowing as cast can cause the suggested type to be overly small. It's fixed by using checked type conversion and returning u128 when it's the only choice. (Fixes Wrong type suggested for overflowing_literals when the number is big #135248)
When an integer literal is i128::MIN but is of a smaller type, an overflowing negation cause the compiler to panic in debug build. Fixed by checking the number size beforehand and wrapping_neg. (Fixes ICE: attempt to negate with overflow #131849)

Edit: extracted the type conversion part into a standalone function to separate the concern of overflowing.

rustbot · 2025-01-08T11:16:25Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @chenyukang (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

Apply eta-reduction on map to simplify code and make the style more consistent

tests/ui/lint/type-overflow.rs

chenyukang · 2025-01-12T09:27:19Z

you may add:

Fixes #135399 
Fixes #131849

in the PR description so that the two issue will be closed after PR merged.

chenyukang · 2025-01-12T09:35:47Z

seems we does not find the proper type for different range of number literals , for example:

let _ = 18446744073709551614;

the value is u64::MAX - 1, the current suggestion is:

  |
3 |     let _ = 18446744073709551614;
  |             ^^^^^^^^^^^^^^^^^^^^
  |
  = note: the literal `18446744073709551614` does not fit into the type `i32` whose range is `-2147483648..=2147483647`
  = help: consider using the type `i128` instead

maybe better choice is suggesting u64.

s-cerevisiae · 2025-01-12T12:02:29Z

@chenyukang I preserved the original behavior which is to suggest types with the same signedness unless the value fits into an unsigned number of the same size. I was focusing on fixing the overflows and outright wrong suggestions in this PR, but if you feel that it's ok to change the behavior here, I'll change it so that it always suggests the smallest fitting type.

Changes the behavior of the `overflowing_literals` suggestion so that it always suggest the smallest type regardless of the original type size.

s-cerevisiae · 2025-01-12T12:26:14Z

Done implementing that too. Hope that there was no special reason for the old, less straightforward behavior.

chenyukang · 2025-01-16T04:06:54Z

@bors r+ rollup

bors · 2025-01-16T04:06:56Z

📌 Commit 74e2e8b has been approved by chenyukang

It is now in the queue for this repository.

…als-help, r=chenyukang Fix overflows in the implementation of `overflowing_literals` lint's help This PR fixes two overflow problems that cause the `overflowing_literals` lint to behave incorrectly in some edge cases. 1. When an integer literal is between `i128::MAX` and `u128::MAX`, an overflowing `as` cast can cause the suggested type to be overly small. It's fixed by using checked type conversion and returning `u128` when it's the only choice. (Fixes rust-lang#135248) 2. When an integer literal is `i128::MIN` but is of a smaller type, an overflowing negation cause the compiler to panic in debug build. Fixed by checking the number size beforehand and `wrapping_neg`. (Fixes rust-lang#131849) Edit: extracted the type conversion part into a standalone function to separate the concern of overflowing.

Rollup of 5 pull requests Successful merges: - rust-lang#134286 (Enable `unreachable_pub` lint in core) - rust-lang#135249 (Fix overflows in the implementation of `overflowing_literals` lint's help) - rust-lang#135534 (use indirect return for `i128` and `f128` on wasm32) - rust-lang#135556 (Clarify note in `std::sync::LazyLock` example) - rust-lang#135560 (Update `compiler-builtins` to 0.1.144) r? `@ghost` `@rustbot` modify labels: rollup

…als-help, r=chenyukang Fix overflows in the implementation of `overflowing_literals` lint's help This PR fixes two overflow problems that cause the `overflowing_literals` lint to behave incorrectly in some edge cases. 1. When an integer literal is between `i128::MAX` and `u128::MAX`, an overflowing `as` cast can cause the suggested type to be overly small. It's fixed by using checked type conversion and returning `u128` when it's the only choice. (Fixes rust-lang#135248) 2. When an integer literal is `i128::MIN` but is of a smaller type, an overflowing negation cause the compiler to panic in debug build. Fixed by checking the number size beforehand and `wrapping_neg`. (Fixes rust-lang#131849) Edit: extracted the type conversion part into a standalone function to separate the concern of overflowing.

…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#135249 (Fix overflows in the implementation of `overflowing_literals` lint's help) - rust-lang#135534 (use indirect return for `i128` and `f128` on wasm32) - rust-lang#135556 (Clarify note in `std::sync::LazyLock` example) - rust-lang#135560 (Update `compiler-builtins` to 0.1.144) r? `@ghost` `@rustbot` modify labels: rollup

…als-help, r=chenyukang Fix overflows in the implementation of `overflowing_literals` lint's help This PR fixes two overflow problems that cause the `overflowing_literals` lint to behave incorrectly in some edge cases. 1. When an integer literal is between `i128::MAX` and `u128::MAX`, an overflowing `as` cast can cause the suggested type to be overly small. It's fixed by using checked type conversion and returning `u128` when it's the only choice. (Fixes rust-lang#135248) 2. When an integer literal is `i128::MIN` but is of a smaller type, an overflowing negation cause the compiler to panic in debug build. Fixed by checking the number size beforehand and `wrapping_neg`. (Fixes rust-lang#131849) Edit: extracted the type conversion part into a standalone function to separate the concern of overflowing.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#133720 ([cfg_match] Adjust syntax) - rust-lang#134496 (Update documentation for Arc::from_raw, Arc::increment_strong_count, and Arc::decrement_strong_count to clarify allocator requirement) - rust-lang#134754 (Implement `use` associated items of traits) - rust-lang#135249 (Fix overflows in the implementation of `overflowing_literals` lint's help) - rust-lang#135251 (Only treat plain literal patterns as short) - rust-lang#135556 (Clarify note in `std::sync::LazyLock` example) - rust-lang#135560 (Update `compiler-builtins` to 0.1.144) r? `@ghost` `@rustbot` modify labels: rollup

…als-help, r=chenyukang Fix overflows in the implementation of `overflowing_literals` lint's help This PR fixes two overflow problems that cause the `overflowing_literals` lint to behave incorrectly in some edge cases. 1. When an integer literal is between `i128::MAX` and `u128::MAX`, an overflowing `as` cast can cause the suggested type to be overly small. It's fixed by using checked type conversion and returning `u128` when it's the only choice. (Fixes rust-lang#135248) 2. When an integer literal is `i128::MIN` but is of a smaller type, an overflowing negation cause the compiler to panic in debug build. Fixed by checking the number size beforehand and `wrapping_neg`. (Fixes rust-lang#131849) Edit: extracted the type conversion part into a standalone function to separate the concern of overflowing.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#133720 ([cfg_match] Adjust syntax) - rust-lang#134496 (Update documentation for Arc::from_raw, Arc::increment_strong_count, and Arc::decrement_strong_count to clarify allocator requirement) - rust-lang#135249 (Fix overflows in the implementation of `overflowing_literals` lint's help) - rust-lang#135251 (Only treat plain literal patterns as short) - rust-lang#135556 (Clarify note in `std::sync::LazyLock` example) - rust-lang#135560 (Update `compiler-builtins` to 0.1.144) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#133720 ([cfg_match] Adjust syntax) - rust-lang#134496 (Update documentation for Arc::from_raw, Arc::increment_strong_count, and Arc::decrement_strong_count to clarify allocator requirement) - rust-lang#135249 (Fix overflows in the implementation of `overflowing_literals` lint's help) - rust-lang#135251 (Only treat plain literal patterns as short) - rust-lang#135556 (Clarify note in `std::sync::LazyLock` example) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#135249 - s-cerevisiae:fix-overflowing-literals-help, r=chenyukang Fix overflows in the implementation of `overflowing_literals` lint's help This PR fixes two overflow problems that cause the `overflowing_literals` lint to behave incorrectly in some edge cases. 1. When an integer literal is between `i128::MAX` and `u128::MAX`, an overflowing `as` cast can cause the suggested type to be overly small. It's fixed by using checked type conversion and returning `u128` when it's the only choice. (Fixes rust-lang#135248) 2. When an integer literal is `i128::MIN` but is of a smaller type, an overflowing negation cause the compiler to panic in debug build. Fixed by checking the number size beforehand and `wrapping_neg`. (Fixes rust-lang#131849) Edit: extracted the type conversion part into a standalone function to separate the concern of overflowing.

s-cerevisiae added 3 commits January 8, 2025 18:58

Detect overflow when the literal is larger than i128::MAX

be23697

Detect overflow when the literal is negative

330be17

Add test cases and test for HELP

1517a41

rustbot assigned chenyukang Jan 8, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 8, 2025

s-cerevisiae added 2 commits January 9, 2025 02:02

Extract integer conversion into a function

c04e65d

Minor simplification

4a85755

Apply eta-reduction on map to simplify code and make the style more consistent

chenyukang reviewed Jan 12, 2025

View reviewed changes

tests/ui/lint/type-overflow.rs Show resolved Hide resolved

Add comment on case to mark the original issue

f52724c

Suggest the smallest fitting type instead

74e2e8b

Changes the behavior of the `overflowing_literals` suggestion so that it always suggest the smallest type regardless of the original type size.

s-cerevisiae mentioned this pull request Jan 12, 2025

Wrong type suggested in overflowing_bin_hex #135404

Open

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 16, 2025

jhpratt mentioned this pull request Jan 16, 2025

Rollup of 5 pull requests #135567

Closed

matthiaskrgr mentioned this pull request Jan 16, 2025

Rollup of 4 pull requests #135569

Closed

matthiaskrgr mentioned this pull request Jan 16, 2025

Rollup of 7 pull requests #135576

Closed

matthiaskrgr mentioned this pull request Jan 16, 2025

Rollup of 6 pull requests #135584

Closed

matthiaskrgr mentioned this pull request Jan 16, 2025

Rollup of 5 pull requests #135587

Merged

bors merged commit 8f9ccc5 into rust-lang:master Jan 16, 2025
6 checks passed

rustbot added this to the 1.86.0 milestone Jan 16, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix overflows in the implementation of `overflowing_literals` lint's help #135249

Fix overflows in the implementation of `overflowing_literals` lint's help #135249

s-cerevisiae commented Jan 8, 2025 •

edited

Loading

rustbot commented Jan 8, 2025

chenyukang commented Jan 12, 2025 •

edited

Loading

chenyukang commented Jan 12, 2025

s-cerevisiae commented Jan 12, 2025 •

edited

Loading

s-cerevisiae commented Jan 12, 2025

chenyukang commented Jan 16, 2025

bors commented Jan 16, 2025

Fix overflows in the implementation of overflowing_literals lint's help #135249

Fix overflows in the implementation of overflowing_literals lint's help #135249

Conversation

s-cerevisiae commented Jan 8, 2025 • edited Loading

rustbot commented Jan 8, 2025

chenyukang commented Jan 12, 2025 • edited Loading

chenyukang commented Jan 12, 2025

s-cerevisiae commented Jan 12, 2025 • edited Loading

s-cerevisiae commented Jan 12, 2025

chenyukang commented Jan 16, 2025

bors commented Jan 16, 2025

Fix overflows in the implementation of `overflowing_literals` lint's help #135249

Fix overflows in the implementation of `overflowing_literals` lint's help #135249

s-cerevisiae commented Jan 8, 2025 •

edited

Loading

chenyukang commented Jan 12, 2025 •

edited

Loading

s-cerevisiae commented Jan 12, 2025 •

edited

Loading