From 8e56c2c5f1e8417ae7dcaf1fad1d72dd1ca964f5 Mon Sep 17 00:00:00 2001 From: Mu42 Date: Fri, 24 Mar 2023 14:24:25 +0800 Subject: [PATCH 1/4] Suggest ..= when someone tries to create an overflowing range --- compiler/rustc_lint/src/types.rs | 27 ++++++++++++++++++++------- tests/ui/lint/issue-109529.rs | 5 +++++ tests/ui/lint/issue-109529.stderr | 10 ++++++++++ 3 files changed, 35 insertions(+), 7 deletions(-) create mode 100644 tests/ui/lint/issue-109529.rs create mode 100644 tests/ui/lint/issue-109529.stderr diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 7ca50f5a2db70..db93f6d340200 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -136,6 +136,13 @@ fn lint_overflowing_range_endpoint<'tcx>( expr: &'tcx hir::Expr<'tcx>, ty: &str, ) -> bool { + let (expr, cast_ty) = if let Node::Expr(par_expr) = cx.tcx.hir().get(cx.tcx.hir().parent_id(expr.hir_id)) + && let ExprKind::Cast(_, ty) = par_expr.kind { + (par_expr, Some(ty)) + } else { + (expr, None) + }; + // We only want to handle exclusive (`..`) ranges, // which are represented as `ExprKind::Struct`. let par_id = cx.tcx.hir().parent_id(expr.hir_id); @@ -157,13 +164,19 @@ fn lint_overflowing_range_endpoint<'tcx>( }; let Ok(start) = cx.sess().source_map().span_to_snippet(eps[0].span) else { return false }; - use rustc_ast::{LitIntType, LitKind}; - let suffix = match lit.node { - LitKind::Int(_, LitIntType::Signed(s)) => s.name_str(), - LitKind::Int(_, LitIntType::Unsigned(s)) => s.name_str(), - LitKind::Int(_, LitIntType::Unsuffixed) => "", - _ => bug!(), + let suffix = if let Some(cast_ty) = cast_ty { + let Ok(ty) = cx.sess().source_map().span_to_snippet(cast_ty.span) else { return false }; + format!(" as {}", ty) + } else { + use rustc_ast::{LitIntType, LitKind}; + match lit.node { + LitKind::Int(_, LitIntType::Signed(s)) => s.name_str().to_owned(), + LitKind::Int(_, LitIntType::Unsigned(s)) => s.name_str().to_owned(), + LitKind::Int(_, LitIntType::Unsuffixed) => "".to_owned(), + _ => bug!(), + } }; + cx.emit_spanned_lint( OVERFLOWING_LITERALS, struct_expr.span, @@ -172,7 +185,7 @@ fn lint_overflowing_range_endpoint<'tcx>( suggestion: struct_expr.span, start, literal: lit_val - 1, - suffix, + suffix: &suffix, }, ); diff --git a/tests/ui/lint/issue-109529.rs b/tests/ui/lint/issue-109529.rs new file mode 100644 index 0000000000000..f6829d0c76a55 --- /dev/null +++ b/tests/ui/lint/issue-109529.rs @@ -0,0 +1,5 @@ +fn main() { + for i in 0..256 as u8 { //~ ERROR range endpoint is out of range + println!("{}", i); + } +} diff --git a/tests/ui/lint/issue-109529.stderr b/tests/ui/lint/issue-109529.stderr new file mode 100644 index 0000000000000..87c9119ee6f1c --- /dev/null +++ b/tests/ui/lint/issue-109529.stderr @@ -0,0 +1,10 @@ +error: range endpoint is out of range for `u8` + --> $DIR/issue-109529.rs:2:14 + | +LL | for i in 0..256 as u8 { + | ^^^^^^^^^^^^ help: use an inclusive range instead: `0..=255 as u8` + | + = note: `#[deny(overflowing_literals)]` on by default + +error: aborting due to previous error + From 6034b2fcb80915d35a871d438663758b4241a0fa Mon Sep 17 00:00:00 2001 From: Mu42 Date: Fri, 24 Mar 2023 20:09:02 +0800 Subject: [PATCH 2/4] Use independent suggestions --- compiler/rustc_lint/src/lints.rs | 7 ++++--- compiler/rustc_lint/src/types.rs | 33 +++++++++++++------------------ tests/ui/lint/issue-109529.rs | 2 +- tests/ui/lint/issue-109529.stderr | 12 +++++++++-- 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 308c02929ca42..925e9654f931d 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1210,9 +1210,10 @@ impl<'a> DecorateLint<'a, ()> for DropGlue<'_> { #[diag(lint_range_endpoint_out_of_range)] pub struct RangeEndpointOutOfRange<'a> { pub ty: &'a str, - #[suggestion(code = "{start}..={literal}{suffix}", applicability = "machine-applicable")] - pub suggestion: Span, - pub start: String, + #[suggestion(code = "=", applicability = "machine-applicable")] + pub eq_suggestion: Span, + #[suggestion(code = "{literal}{suffix}", applicability = "machine-applicable")] + pub lit_suggestion: Span, pub literal: u128, pub suffix: &'a str, } diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index db93f6d340200..1cc873c6de9c4 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -136,11 +136,12 @@ fn lint_overflowing_range_endpoint<'tcx>( expr: &'tcx hir::Expr<'tcx>, ty: &str, ) -> bool { - let (expr, cast_ty) = if let Node::Expr(par_expr) = cx.tcx.hir().get(cx.tcx.hir().parent_id(expr.hir_id)) - && let ExprKind::Cast(_, ty) = par_expr.kind { - (par_expr, Some(ty)) + // Look past casts to support cases like `0..256 as u8` + let (expr, lit_span) = if let Node::Expr(par_expr) = cx.tcx.hir().get(cx.tcx.hir().parent_id(expr.hir_id)) + && let ExprKind::Cast(_, _) = par_expr.kind { + (par_expr, expr.span) } else { - (expr, None) + (expr, expr.span) }; // We only want to handle exclusive (`..`) ranges, @@ -162,19 +163,13 @@ fn lint_overflowing_range_endpoint<'tcx>( if !(eps[1].expr.hir_id == expr.hir_id && lit_val - 1 == max) { return false; }; - let Ok(start) = cx.sess().source_map().span_to_snippet(eps[0].span) else { return false }; - let suffix = if let Some(cast_ty) = cast_ty { - let Ok(ty) = cx.sess().source_map().span_to_snippet(cast_ty.span) else { return false }; - format!(" as {}", ty) - } else { - use rustc_ast::{LitIntType, LitKind}; - match lit.node { - LitKind::Int(_, LitIntType::Signed(s)) => s.name_str().to_owned(), - LitKind::Int(_, LitIntType::Unsigned(s)) => s.name_str().to_owned(), - LitKind::Int(_, LitIntType::Unsuffixed) => "".to_owned(), - _ => bug!(), - } + use rustc_ast::{LitIntType, LitKind}; + let suffix = match lit.node { + LitKind::Int(_, LitIntType::Signed(s)) => s.name_str(), + LitKind::Int(_, LitIntType::Unsigned(s)) => s.name_str(), + LitKind::Int(_, LitIntType::Unsuffixed) => "", + _ => bug!(), }; cx.emit_spanned_lint( @@ -182,10 +177,10 @@ fn lint_overflowing_range_endpoint<'tcx>( struct_expr.span, RangeEndpointOutOfRange { ty, - suggestion: struct_expr.span, - start, + eq_suggestion: expr.span.shrink_to_lo(), + lit_suggestion: lit_span, literal: lit_val - 1, - suffix: &suffix, + suffix, }, ); diff --git a/tests/ui/lint/issue-109529.rs b/tests/ui/lint/issue-109529.rs index f6829d0c76a55..36c6d4fdfe3b4 100644 --- a/tests/ui/lint/issue-109529.rs +++ b/tests/ui/lint/issue-109529.rs @@ -1,5 +1,5 @@ fn main() { - for i in 0..256 as u8 { //~ ERROR range endpoint is out of range + for i in 0..(256 as u8) { //~ ERROR range endpoint is out of range println!("{}", i); } } diff --git a/tests/ui/lint/issue-109529.stderr b/tests/ui/lint/issue-109529.stderr index 87c9119ee6f1c..e9f6d546ac20b 100644 --- a/tests/ui/lint/issue-109529.stderr +++ b/tests/ui/lint/issue-109529.stderr @@ -1,10 +1,18 @@ error: range endpoint is out of range for `u8` --> $DIR/issue-109529.rs:2:14 | -LL | for i in 0..256 as u8 { - | ^^^^^^^^^^^^ help: use an inclusive range instead: `0..=255 as u8` +LL | for i in 0..(256 as u8) { + | ^^^^^^^^^^^^^^ | = note: `#[deny(overflowing_literals)]` on by default +help: use an inclusive range instead + | +LL | for i in 0..=(256 as u8) { + | + +help: use an inclusive range instead + | +LL | for i in 0..(255 as u8) { + | ~~~ error: aborting due to previous error From 910a5ad2dfbff6c29092b73e696349eb45eb1aa5 Mon Sep 17 00:00:00 2001 From: Mu001999 Date: Sat, 25 Mar 2023 01:00:49 +0800 Subject: [PATCH 3/4] Emits suggestions for expressions with parentheses or not separately --- compiler/rustc_lint/messages.ftl | 4 +++- compiler/rustc_lint/src/lints.rs | 33 +++++++++++++++++++++++++------ compiler/rustc_lint/src/types.rs | 28 ++++++++++++++++++-------- tests/ui/lint/issue-109529.rs | 5 ++--- tests/ui/lint/issue-109529.stderr | 21 ++++++++++++-------- 5 files changed, 65 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 68e62c9789aed..5b7e994e035f4 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -197,7 +197,9 @@ lint_drop_glue = types that do not implement `Drop` can still have drop glue, consider instead using `{$needs_drop}` to detect whether a type is trivially dropped lint_range_endpoint_out_of_range = range endpoint is out of range for `{$ty}` - .suggestion = use an inclusive range instead + +lint_range_use_inclusive_range = use an inclusive range instead + lint_overflowing_bin_hex = literal out of range for `{$ty}` .negative_note = the literal `{$lit}` (decimal `{$dec}`) does not fit into the type `{$ty}` diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 925e9654f931d..8ec4c2b3d463a 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1210,12 +1210,33 @@ impl<'a> DecorateLint<'a, ()> for DropGlue<'_> { #[diag(lint_range_endpoint_out_of_range)] pub struct RangeEndpointOutOfRange<'a> { pub ty: &'a str, - #[suggestion(code = "=", applicability = "machine-applicable")] - pub eq_suggestion: Span, - #[suggestion(code = "{literal}{suffix}", applicability = "machine-applicable")] - pub lit_suggestion: Span, - pub literal: u128, - pub suffix: &'a str, + #[subdiagnostic] + pub sub: UseInclusiveRange<'a>, +} + +#[derive(Subdiagnostic)] +pub enum UseInclusiveRange<'a> { + #[suggestion( + lint_range_use_inclusive_range, + code = "{start}..={literal}{suffix}", + applicability = "machine-applicable" + )] + WithoutParen { + #[primary_span] + sugg: Span, + start: String, + literal: u128, + suffix: &'a str, + }, + #[multipart_suggestion(lint_range_use_inclusive_range, applicability = "machine-applicable")] + WithParen { + #[suggestion_part(code = "=")] + eq_sugg: Span, + #[suggestion_part(code = "{literal}{suffix}")] + lit_sugg: Span, + literal: u128, + suffix: &'a str, + }, } #[derive(LintDiagnostic)] diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 1cc873c6de9c4..f6bca7045c846 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -4,7 +4,8 @@ use crate::{ AtomicOrderingFence, AtomicOrderingLoad, AtomicOrderingStore, ImproperCTypes, InvalidAtomicOrderingDiag, OnlyCastu8ToChar, OverflowingBinHex, OverflowingBinHexSign, OverflowingBinHexSub, OverflowingInt, OverflowingIntHelp, OverflowingLiteral, - OverflowingUInt, RangeEndpointOutOfRange, UnusedComparisons, VariantSizeDifferencesDiag, + OverflowingUInt, RangeEndpointOutOfRange, UnusedComparisons, UseInclusiveRange, + VariantSizeDifferencesDiag, }, }; use crate::{LateContext, LateLintPass, LintContext}; @@ -172,16 +173,27 @@ fn lint_overflowing_range_endpoint<'tcx>( _ => bug!(), }; + let sub_sugg = if expr.span.lo() == lit_span.lo() { + let Ok(start) = cx.sess().source_map().span_to_snippet(eps[0].span) else { return false }; + UseInclusiveRange::WithoutParen { + sugg: struct_expr.span.shrink_to_lo().to(lit_span.shrink_to_hi()), + start, + literal: lit_val - 1, + suffix, + } + } else { + UseInclusiveRange::WithParen { + eq_sugg: expr.span.shrink_to_lo(), + lit_sugg: lit_span, + literal: lit_val - 1, + suffix, + } + }; + cx.emit_spanned_lint( OVERFLOWING_LITERALS, struct_expr.span, - RangeEndpointOutOfRange { - ty, - eq_suggestion: expr.span.shrink_to_lo(), - lit_suggestion: lit_span, - literal: lit_val - 1, - suffix, - }, + RangeEndpointOutOfRange { ty, sub: sub_sugg }, ); // We've just emitted a lint, special cased for `(...)..MAX+1` ranges, diff --git a/tests/ui/lint/issue-109529.rs b/tests/ui/lint/issue-109529.rs index 36c6d4fdfe3b4..c9b4da4b26e75 100644 --- a/tests/ui/lint/issue-109529.rs +++ b/tests/ui/lint/issue-109529.rs @@ -1,5 +1,4 @@ fn main() { - for i in 0..(256 as u8) { //~ ERROR range endpoint is out of range - println!("{}", i); - } + for _ in 0..256 as u8 {} //~ ERROR range endpoint is out of range + for _ in 0..(256 as u8) {} //~ ERROR range endpoint is out of range } diff --git a/tests/ui/lint/issue-109529.stderr b/tests/ui/lint/issue-109529.stderr index e9f6d546ac20b..15b259ad55ee4 100644 --- a/tests/ui/lint/issue-109529.stderr +++ b/tests/ui/lint/issue-109529.stderr @@ -1,18 +1,23 @@ error: range endpoint is out of range for `u8` --> $DIR/issue-109529.rs:2:14 | -LL | for i in 0..(256 as u8) { - | ^^^^^^^^^^^^^^ +LL | for _ in 0..256 as u8 {} + | ------^^^^^^ + | | + | help: use an inclusive range instead: `0..=255` | = note: `#[deny(overflowing_literals)]` on by default -help: use an inclusive range instead + +error: range endpoint is out of range for `u8` + --> $DIR/issue-109529.rs:3:14 + | +LL | for _ in 0..(256 as u8) {} + | ^^^^^^^^^^^^^^ | -LL | for i in 0..=(256 as u8) { - | + help: use an inclusive range instead | -LL | for i in 0..(255 as u8) { - | ~~~ +LL | for _ in 0..=(255 as u8) {} + | + ~~~ -error: aborting due to previous error +error: aborting due to 2 previous errors From dde26b31b6b625496e8dc0933e231c27f35b8933 Mon Sep 17 00:00:00 2001 From: Mu42 Date: Wed, 29 Mar 2023 09:56:28 +0800 Subject: [PATCH 4/4] add run-rustfix --- tests/ui/lint/issue-109529.fixed | 6 ++++++ tests/ui/lint/issue-109529.rs | 2 ++ tests/ui/lint/issue-109529.stderr | 4 ++-- 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 tests/ui/lint/issue-109529.fixed diff --git a/tests/ui/lint/issue-109529.fixed b/tests/ui/lint/issue-109529.fixed new file mode 100644 index 0000000000000..5ad489073eead --- /dev/null +++ b/tests/ui/lint/issue-109529.fixed @@ -0,0 +1,6 @@ +// run-rustfix + +fn main() { + for _ in 0..=255 as u8 {} //~ ERROR range endpoint is out of range + for _ in 0..=(255 as u8) {} //~ ERROR range endpoint is out of range +} diff --git a/tests/ui/lint/issue-109529.rs b/tests/ui/lint/issue-109529.rs index c9b4da4b26e75..383d7bc4cf31f 100644 --- a/tests/ui/lint/issue-109529.rs +++ b/tests/ui/lint/issue-109529.rs @@ -1,3 +1,5 @@ +// run-rustfix + fn main() { for _ in 0..256 as u8 {} //~ ERROR range endpoint is out of range for _ in 0..(256 as u8) {} //~ ERROR range endpoint is out of range diff --git a/tests/ui/lint/issue-109529.stderr b/tests/ui/lint/issue-109529.stderr index 15b259ad55ee4..9e857d1b0ab5d 100644 --- a/tests/ui/lint/issue-109529.stderr +++ b/tests/ui/lint/issue-109529.stderr @@ -1,5 +1,5 @@ error: range endpoint is out of range for `u8` - --> $DIR/issue-109529.rs:2:14 + --> $DIR/issue-109529.rs:4:14 | LL | for _ in 0..256 as u8 {} | ------^^^^^^ @@ -9,7 +9,7 @@ LL | for _ in 0..256 as u8 {} = note: `#[deny(overflowing_literals)]` on by default error: range endpoint is out of range for `u8` - --> $DIR/issue-109529.rs:3:14 + --> $DIR/issue-109529.rs:5:14 | LL | for _ in 0..(256 as u8) {} | ^^^^^^^^^^^^^^