Uniformly handle HIR literals in visitors and lints

oli-obk · oli-obk · commit 663191d683f4 · 2025-02-01T22:16:42.000Z
diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs
@@ -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)
     }
@@ -754,7 +757,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),
     }
@@ -902,7 +905,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()
 }
diff --git a/compiler/rustc_lint/src/late.rs b/compiler/rustc_lint/src/late.rs
@@ -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))
     }
diff --git a/compiler/rustc_lint/src/passes.rs b/compiler/rustc_lint/src/passes.rs
@@ -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>);
diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs
@@ -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::{
@@ -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) => {
@@ -557,7 +567,6 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
                     }
                 }
             }
-            hir::ExprKind::Lit(lit) => lint_literal(cx, self, e.hir_id, e.span, 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()
diff --git a/compiler/rustc_lint/src/types/literal.rs b/compiler/rustc_lint/src/types/literal.rs
@@ -245,11 +245,12 @@ fn lint_int_literal<'tcx>(
     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(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
@@ -359,17 +360,21 @@ pub(crate) fn lint_literal<'tcx>(
     hir_id: HirId,
     span: Span,
     lit: &hir::Lit,
+    negated: bool,
 ) {
     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, hir_id, span, 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, hir_id, span, 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 {
diff --git a/src/tools/clippy/clippy_lints/src/approx_const.rs b/src/tools/clippy/clippy_lints/src/approx_const.rs
@@ -3,10 +3,10 @@ use clippy_utils::diagnostics::span_lint_and_help;
 use clippy_utils::msrvs::{self, Msrv};
 use rustc_ast::ast::{FloatTy, LitFloatType, LitKind};
 use rustc_attr_parsing::RustcVersion;
-use rustc_hir::{Expr, ExprKind};
+use rustc_hir::{HirId, Lit};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
-use rustc_span::symbol;
+use rustc_span::{Span, symbol};
 use std::f64::consts as f64;
 
 declare_clippy_lint! {
@@ -74,29 +74,15 @@ impl ApproxConstant {
         }
     }
 
-    fn check_lit(&self, cx: &LateContext<'_>, lit: &LitKind, e: &Expr<'_>) {
-        match *lit {
-            LitKind::Float(s, LitFloatType::Suffixed(fty)) => match fty {
-                FloatTy::F16 => self.check_known_consts(cx, e, s, "f16"),
-                FloatTy::F32 => self.check_known_consts(cx, e, s, "f32"),
-                FloatTy::F64 => self.check_known_consts(cx, e, s, "f64"),
-                FloatTy::F128 => self.check_known_consts(cx, e, s, "f128"),
-            },
-            // FIXME(f16_f128): add `f16` and `f128` when these types become stable.
-            LitKind::Float(s, LitFloatType::Unsuffixed) => self.check_known_consts(cx, e, s, "f{32, 64}"),
-            _ => (),
-        }
-    }
-
-    fn check_known_consts(&self, cx: &LateContext<'_>, e: &Expr<'_>, s: symbol::Symbol, module: &str) {
+    fn check_known_consts(&self, cx: &LateContext<'_>, span: Span, s: symbol::Symbol, module: &str) {
         let s = s.as_str();
         if s.parse::<f64>().is_ok() {
             for &(constant, name, min_digits, msrv) in &KNOWN_CONSTS {
                 if is_approx_const(constant, s, min_digits) && msrv.is_none_or(|msrv| self.msrv.meets(msrv)) {
                     span_lint_and_help(
                         cx,
                         APPROX_CONSTANT,
-                        e.span,
+                        span,
                         format!("approximate value of `{module}::consts::{name}` found"),
                         None,
                         "consider using the constant directly",
@@ -111,9 +97,17 @@ impl ApproxConstant {
 impl_lint_pass!(ApproxConstant => [APPROX_CONSTANT]);
 
 impl<'tcx> LateLintPass<'tcx> for ApproxConstant {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
-        if let ExprKind::Lit(lit) = &e.kind {
-            self.check_lit(cx, &lit.node, e);
+    fn check_lit(&mut self, cx: &LateContext<'_>, _hir_id: HirId, lit: &Lit, _negated: bool) {
+        match lit.node {
+            LitKind::Float(s, LitFloatType::Suffixed(fty)) => match fty {
+                FloatTy::F16 => self.check_known_consts(cx, lit.span, s, "f16"),
+                FloatTy::F32 => self.check_known_consts(cx, lit.span, s, "f32"),
+                FloatTy::F64 => self.check_known_consts(cx, lit.span, s, "f64"),
+                FloatTy::F128 => self.check_known_consts(cx, lit.span, s, "f128"),
+            },
+            // FIXME(f16_f128): add `f16` and `f128` when these types become stable.
+            LitKind::Float(s, LitFloatType::Unsuffixed) => self.check_known_consts(cx, lit.span, s, "f{32, 64}"),
+            _ => (),
         }
     }
 
diff --git a/tests/ui/pattern/overflowing-literals.rs b/tests/ui/pattern/overflowing-literals.rs
@@ -3,8 +3,6 @@
 #![feature(pattern_types)]
 #![feature(pattern_type_macro)]
 
-//@ check-pass
-
 use std::pat::pattern_type;
 
 // FIXME(pattern_types): also reject overflowing literals
@@ -14,6 +12,7 @@ type TooSmall = pattern_type!(i8 is -500..);
 fn main() {
     match 5_u8 {
         500 => {}
+        //~^ ERROR literal out of range for `u8`
         _ => {}
     }
 }
diff --git a/tests/ui/pattern/overflowing-literals.stderr b/tests/ui/pattern/overflowing-literals.stderr
@@ -0,0 +1,11 @@
+error: literal out of range for `u8`
+  --> $DIR/overflowing-literals.rs:14:9
+   |
+LL |         500 => {}
+   |         ^^^
+   |
+   = note: the literal `500` does not fit into the type `u8` whose range is `0..=255`
+   = note: `#[deny(overflowing_literals)]` on by default
+
+error: aborting due to 1 previous error
+

Original file line number	Diff line number	Diff line change
`@@ -345,6 +345,9 @@ pub trait Visitor<'v>: Sized {`
`345`	`345`	`fn visit_pat_expr(&mut self, expr: &'v PatExpr<'v>) -> Self::Result {`
`346`	`346`	`walk_pat_expr(self, expr)`
`347`	`347`	`}`
	`348`	`+ fn visit_lit(&mut self, _hir_id: HirId, _lit: &'v Lit, _negated: bool) -> Self::Result {`
	`349`	`+ Self::Result::output()`
	`350`	`+ }`
`348`	`351`	`fn visit_anon_const(&mut self, c: &'v AnonConst) -> Self::Result {`
`349`	`352`	`walk_anon_const(self, c)`
`350`	`353`	`}`
`@@ -754,7 +757,7 @@ pub fn walk_pat_field<'v, V: Visitor<'v>>(visitor: &mut V, field: &'v PatField<'`
`754`	`757`	`pub fn walk_pat_expr<'v, V: Visitor<'v>>(visitor: &mut V, expr: &'v PatExpr<'v>) -> V::Result {`
`755`	`758`	`try_visit!(visitor.visit_id(expr.hir_id));`
`756`	`759`	`match &expr.kind {`
`757`		`- PatExprKind::Lit { .. } => V::Result::output(),`
	`760`	`+ PatExprKind::Lit { lit, negated } => visitor.visit_lit(expr.hir_id, lit, *negated),`
`758`	`761`	`PatExprKind::ConstBlock(c) => visitor.visit_inline_const(c),`
`759`	`762`	`PatExprKind::Path(qpath) => visitor.visit_qpath(qpath, expr.hir_id, expr.span),`
`760`	`763`	`}`
`@@ -902,7 +905,8 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)`
`902`	`905`	`try_visit!(visitor.visit_expr(expr));`
`903`	`906`	`visit_opt!(visitor, visit_ty_unambig, ty);`
`904`	`907`	`}`
`905`		`- ExprKind::Lit(_) \| ExprKind::Err(_) => {}`
	`908`	`+ ExprKind::Lit(lit) => try_visit!(visitor.visit_lit(expression.hir_id, lit, false)),`
	`909`	`+ ExprKind::Err(_) => {}`
`906`	`910`	`}`
`907`	`911`	`V::Result::output()`
`908`	`912`	`}`
Original file line number	Diff line number	Diff line change
`@@ -152,6 +152,10 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas`
`152`	`152`	`hir_visit::walk_pat(self, p);`
`153`	`153`	`}`
`154`	`154`
	`155`	`+ fn visit_lit(&mut self, hir_id: HirId, lit: &'tcx hir::Lit, negated: bool) {`
	`156`	`+ lint_callback!(self, check_lit, hir_id, lit, negated);`
	`157`	`+ }`
	`158`	`+`
`155`	`159`	`fn visit_expr_field(&mut self, field: &'tcx hir::ExprField<'tcx>) {`
`156`	`160`	`self.with_lint_attrs(field.hir_id, \|cx\| hir_visit::walk_expr_field(cx, field))`
`157`	`161`	`}`
Original file line number	Diff line number	Diff line change
`@@ -3,8 +3,6 @@`
`3`	`3`	`#![feature(pattern_types)]`
`4`	`4`	`#![feature(pattern_type_macro)]`
`5`	`5`
`6`		`-//@ check-pass`
`7`		`-`
`8`	`6`	`use std::pat::pattern_type;`
`9`	`7`
`10`	`8`	`// FIXME(pattern_types): also reject overflowing literals`
`@@ -14,6 +12,7 @@ type TooSmall = pattern_type!(i8 is -500..);`
`14`	`12`	`fn main() {`
`15`	`13`	`match 5_u8 {`
`16`	`14`	`500 => {}`
	`15`	+ //~^ ERROR literal out of range for `u8`
`17`	`16`	`_ => {}`
`18`	`17`	`}`
`19`	`18`	`}`