Skip to content

Fix accidentally not emitting overflowing literals lints anymore in patterns #136393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,9 @@ pub trait Visitor<'v>: Sized {
fn visit_pat_expr(&mut self, expr: &'v PatExpr<'v>) -> Self::Result {
walk_pat_expr(self, expr)
}
fn visit_lit(&mut self, _hir_id: HirId, _lit: &'v Lit, _negated: bool) -> Self::Result {
Self::Result::output()
}
fn visit_anon_const(&mut self, c: &'v AnonConst) -> Self::Result {
walk_anon_const(self, c)
}
Expand Down Expand Up @@ -764,7 +767,7 @@ pub fn walk_pat_field<'v, V: Visitor<'v>>(visitor: &mut V, field: &'v PatField<'
pub fn walk_pat_expr<'v, V: Visitor<'v>>(visitor: &mut V, expr: &'v PatExpr<'v>) -> V::Result {
try_visit!(visitor.visit_id(expr.hir_id));
match &expr.kind {
PatExprKind::Lit { .. } => V::Result::output(),
PatExprKind::Lit { lit, negated } => visitor.visit_lit(expr.hir_id, lit, *negated),
PatExprKind::ConstBlock(c) => visitor.visit_inline_const(c),
PatExprKind::Path(qpath) => visitor.visit_qpath(qpath, expr.hir_id, expr.span),
}
Expand Down Expand Up @@ -912,7 +915,8 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)
try_visit!(visitor.visit_expr(expr));
visit_opt!(visitor, visit_ty_unambig, ty);
}
ExprKind::Lit(_) | ExprKind::Err(_) => {}
ExprKind::Lit(lit) => try_visit!(visitor.visit_lit(expression.hir_id, lit, false)),
ExprKind::Err(_) => {}
}
V::Result::output()
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_lint/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas
hir_visit::walk_pat(self, p);
}

fn visit_lit(&mut self, hir_id: HirId, lit: &'tcx hir::Lit, negated: bool) {
lint_callback!(self, check_lit, hir_id, lit, negated);
}

fn visit_expr_field(&mut self, field: &'tcx hir::ExprField<'tcx>) {
self.with_lint_attrs(field.hir_id, |cx| hir_visit::walk_expr_field(cx, field))
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ macro_rules! late_lint_methods {
fn check_stmt(a: &'tcx rustc_hir::Stmt<'tcx>);
fn check_arm(a: &'tcx rustc_hir::Arm<'tcx>);
fn check_pat(a: &'tcx rustc_hir::Pat<'tcx>);
fn check_lit(hir_id: rustc_hir::HirId, a: &'tcx rustc_hir::Lit, negated: bool);
fn check_expr(a: &'tcx rustc_hir::Expr<'tcx>);
fn check_expr_post(a: &'tcx rustc_hir::Expr<'tcx>);
fn check_ty(a: &'tcx rustc_hir::Ty<'tcx, rustc_hir::AmbigArg>);
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_abi::{BackendRepr, ExternAbi, TagEncoding, VariantIdx, Variants, Wrapp
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::DiagMessage;
use rustc_hir::intravisit::VisitorExt;
use rustc_hir::{AmbigArg, Expr, ExprKind, LangItem};
use rustc_hir::{AmbigArg, Expr, ExprKind, HirId, LangItem};
use rustc_middle::bug;
use rustc_middle::ty::layout::{LayoutOf, SizeSkeleton};
use rustc_middle::ty::{
Expand Down Expand Up @@ -536,6 +536,16 @@ fn lint_fn_pointer<'tcx>(
}

impl<'tcx> LateLintPass<'tcx> for TypeLimits {
fn check_lit(
&mut self,
cx: &LateContext<'tcx>,
hir_id: HirId,
lit: &'tcx hir::Lit,
negated: bool,
) {
lint_literal(cx, self, hir_id, lit.span, lit, negated)
}

fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx hir::Expr<'tcx>) {
match e.kind {
hir::ExprKind::Unary(hir::UnOp::Neg, expr) => {
Expand All @@ -557,7 +567,6 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
}
}
}
hir::ExprKind::Lit(lit) => lint_literal(cx, self, e, lit),
hir::ExprKind::Call(path, [l, r])
if let ExprKind::Path(ref qpath) = path.kind
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
Expand Down
74 changes: 44 additions & 30 deletions compiler/rustc_lint/src/types/literal.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use hir::{ExprKind, Node, is_range_literal};
use rustc_abi::{Integer, Size};
use rustc_hir::HirId;
use rustc_middle::ty::Ty;
use rustc_middle::ty::layout::IntegerExt;
use rustc_middle::{bug, ty};
use rustc_span::Span;
use {rustc_ast as ast, rustc_attr_parsing as attr, rustc_hir as hir};

use crate::LateContext;
Expand All @@ -21,21 +23,22 @@ fn lint_overflowing_range_endpoint<'tcx>(
lit: &hir::Lit,
lit_val: u128,
max: u128,
expr: &'tcx hir::Expr<'tcx>,
hir_id: HirId,
lit_span: Span,
ty: &str,
) -> bool {
// Look past casts to support cases like `0..256 as u8`
let (expr, lit_span) = if let Node::Expr(par_expr) = cx.tcx.parent_hir_node(expr.hir_id)
let (hir_id, span) = if let Node::Expr(par_expr) = cx.tcx.parent_hir_node(hir_id)
&& let ExprKind::Cast(_, _) = par_expr.kind
{
(par_expr, expr.span)
(par_expr.hir_id, par_expr.span)
} else {
(expr, expr.span)
(hir_id, lit_span)
};

// We only want to handle exclusive (`..`) ranges,
// which are represented as `ExprKind::Struct`.
let Node::ExprField(field) = cx.tcx.parent_hir_node(expr.hir_id) else { return false };
let Node::ExprField(field) = cx.tcx.parent_hir_node(hir_id) else { return false };
let Node::Expr(struct_expr) = cx.tcx.parent_hir_node(field.hir_id) else { return false };
if !is_range_literal(struct_expr) {
return false;
Expand All @@ -45,7 +48,7 @@ fn lint_overflowing_range_endpoint<'tcx>(
// We can suggest using an inclusive range
// (`..=`) instead only if it is the `end` that is
// overflowing and only by 1.
if !(end.expr.hir_id == expr.hir_id && lit_val - 1 == max) {
if !(end.expr.hir_id == hir_id && lit_val - 1 == max) {
return false;
};

Expand All @@ -57,7 +60,7 @@ fn lint_overflowing_range_endpoint<'tcx>(
_ => bug!(),
};

let sub_sugg = if expr.span.lo() == lit_span.lo() {
let sub_sugg = if span.lo() == lit_span.lo() {
let Ok(start) = cx.sess().source_map().span_to_snippet(start.span) else { return false };
UseInclusiveRange::WithoutParen {
sugg: struct_expr.span.shrink_to_lo().to(lit_span.shrink_to_hi()),
Expand All @@ -67,7 +70,7 @@ fn lint_overflowing_range_endpoint<'tcx>(
}
} else {
UseInclusiveRange::WithParen {
eq_sugg: expr.span.shrink_to_lo(),
eq_sugg: span.shrink_to_lo(),
lit_sugg: lit_span,
literal: lit_val - 1,
suffix,
Expand Down Expand Up @@ -125,7 +128,8 @@ fn get_bin_hex_repr(cx: &LateContext<'_>, lit: &hir::Lit) -> Option<String> {

fn report_bin_hex_error(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
hir_id: HirId,
span: Span,
ty: attr::IntType,
size: Size,
repr_str: String,
Expand All @@ -144,19 +148,19 @@ fn report_bin_hex_error(
};
let sign =
if negative { OverflowingBinHexSign::Negative } else { OverflowingBinHexSign::Positive };
let sub = get_type_suggestion(cx.typeck_results().node_type(expr.hir_id), val, negative).map(
let sub = get_type_suggestion(cx.typeck_results().node_type(hir_id), val, negative).map(
|suggestion_ty| {
if let Some(pos) = repr_str.chars().position(|c| c == 'i' || c == 'u') {
let (sans_suffix, _) = repr_str.split_at(pos);
OverflowingBinHexSub::Suggestion { span: expr.span, suggestion_ty, sans_suffix }
OverflowingBinHexSub::Suggestion { span, suggestion_ty, sans_suffix }
} else {
OverflowingBinHexSub::Help { suggestion_ty }
}
},
);
let sign_bit_sub = (!negative)
.then(|| {
let ty::Int(int_ty) = cx.typeck_results().node_type(expr.hir_id).kind() else {
let ty::Int(int_ty) = cx.typeck_results().node_type(hir_id).kind() else {
return None;
};

Expand All @@ -177,7 +181,7 @@ fn report_bin_hex_error(
};

Some(OverflowingBinHexSignBitSub {
span: expr.span,
span,
lit_no_suffix,
negative_val: actually.clone(),
int_ty: int_ty.name_str(),
Expand All @@ -186,7 +190,7 @@ fn report_bin_hex_error(
})
.flatten();

cx.emit_span_lint(OVERFLOWING_LITERALS, expr.span, OverflowingBinHex {
cx.emit_span_lint(OVERFLOWING_LITERALS, span, OverflowingBinHex {
ty: t,
lit: repr_str.clone(),
dec: val,
Expand Down Expand Up @@ -236,23 +240,26 @@ fn literal_to_i128(val: u128, negative: bool) -> Option<i128> {
fn lint_int_literal<'tcx>(
cx: &LateContext<'tcx>,
type_limits: &TypeLimits,
e: &'tcx hir::Expr<'tcx>,
hir_id: HirId,
span: Span,
lit: &hir::Lit,
t: ty::IntTy,
v: u128,
negated: bool,
) {
let int_type = t.normalize(cx.sess().target.pointer_width);
let (min, max) = int_ty_range(int_type);
let max = max as u128;
let negative = type_limits.negated_expr_id == Some(e.hir_id);
let negative = negated ^ (type_limits.negated_expr_id == Some(hir_id));

// Detect literal value out of range [min, max] inclusive
// avoiding use of -min to prevent overflow/panic
if (negative && v > max + 1) || (!negative && v > max) {
if let Some(repr_str) = get_bin_hex_repr(cx, lit) {
report_bin_hex_error(
cx,
e,
hir_id,
span,
attr::IntType::SignedInt(ty::ast_int_ty(t)),
Integer::from_int_ty(cx, t).size(),
repr_str,
Expand All @@ -262,18 +269,18 @@ fn lint_int_literal<'tcx>(
return;
}

if lint_overflowing_range_endpoint(cx, lit, v, max, e, t.name_str()) {
if lint_overflowing_range_endpoint(cx, lit, v, max, hir_id, span, t.name_str()) {
// The overflowing literal lint was emitted by `lint_overflowing_range_endpoint`.
return;
}

let span = if negative { type_limits.negated_expr_span.unwrap() } else { e.span };
let span = if negative { type_limits.negated_expr_span.unwrap() } else { span };
let lit = cx
.sess()
.source_map()
.span_to_snippet(span)
.unwrap_or_else(|_| if negative { format!("-{v}") } else { v.to_string() });
let help = get_type_suggestion(cx.typeck_results().node_type(e.hir_id), v, negative)
let help = get_type_suggestion(cx.typeck_results().node_type(hir_id), v, negative)
.map(|suggestion_ty| OverflowingIntHelp { suggestion_ty });

cx.emit_span_lint(OVERFLOWING_LITERALS, span, OverflowingInt {
Expand All @@ -288,7 +295,8 @@ fn lint_int_literal<'tcx>(

fn lint_uint_literal<'tcx>(
cx: &LateContext<'tcx>,
e: &'tcx hir::Expr<'tcx>,
hir_id: HirId,
span: Span,
lit: &hir::Lit,
t: ty::UintTy,
) {
Expand All @@ -302,7 +310,7 @@ fn lint_uint_literal<'tcx>(
};

if lit_val < min || lit_val > max {
if let Node::Expr(par_e) = cx.tcx.parent_hir_node(e.hir_id) {
if let Node::Expr(par_e) = cx.tcx.parent_hir_node(hir_id) {
match par_e.kind {
hir::ExprKind::Cast(..) => {
if let ty::Char = cx.typeck_results().expr_ty(par_e).kind() {
Expand All @@ -316,14 +324,15 @@ fn lint_uint_literal<'tcx>(
_ => {}
}
}
if lint_overflowing_range_endpoint(cx, lit, lit_val, max, e, t.name_str()) {
if lint_overflowing_range_endpoint(cx, lit, lit_val, max, hir_id, span, t.name_str()) {
// The overflowing literal lint was emitted by `lint_overflowing_range_endpoint`.
return;
}
if let Some(repr_str) = get_bin_hex_repr(cx, lit) {
report_bin_hex_error(
cx,
e,
hir_id,
span,
attr::IntType::UnsignedInt(ty::ast_uint_ty(t)),
Integer::from_uint_ty(cx, t).size(),
repr_str,
Expand All @@ -332,7 +341,7 @@ fn lint_uint_literal<'tcx>(
);
return;
}
cx.emit_span_lint(OVERFLOWING_LITERALS, e.span, OverflowingUInt {
cx.emit_span_lint(OVERFLOWING_LITERALS, span, OverflowingUInt {
ty: t.name_str(),
lit: cx
.sess()
Expand All @@ -348,19 +357,24 @@ fn lint_uint_literal<'tcx>(
pub(crate) fn lint_literal<'tcx>(
cx: &LateContext<'tcx>,
type_limits: &TypeLimits,
e: &'tcx hir::Expr<'tcx>,
hir_id: HirId,
span: Span,
lit: &hir::Lit,
negated: bool,
) {
match *cx.typeck_results().node_type(e.hir_id).kind() {
match *cx.typeck_results().node_type(hir_id).kind() {
ty::Int(t) => {
match lit.node {
ast::LitKind::Int(v, ast::LitIntType::Signed(_) | ast::LitIntType::Unsuffixed) => {
lint_int_literal(cx, type_limits, e, lit, t, v.get())
lint_int_literal(cx, type_limits, hir_id, span, lit, t, v.get(), negated)
}
_ => bug!(),
};
}
ty::Uint(t) => lint_uint_literal(cx, e, lit, t),
ty::Uint(t) => {
assert!(!negated);
lint_uint_literal(cx, hir_id, span, lit, t)
}
ty::Float(t) => {
let (is_infinite, sym) = match lit.node {
ast::LitKind::Float(v, _) => match t {
Expand All @@ -374,7 +388,7 @@ pub(crate) fn lint_literal<'tcx>(
_ => bug!(),
};
if is_infinite == Ok(true) {
cx.emit_span_lint(OVERFLOWING_LITERALS, e.span, OverflowingLiteral {
cx.emit_span_lint(OVERFLOWING_LITERALS, span, OverflowingLiteral {
ty: t.name_str(),
lit: cx
.sess()
Expand Down
Loading
Loading