Skip to content

Commit

Permalink
Uniformly handle HIR literals in visitors and lints
Browse files Browse the repository at this point in the history
  • Loading branch information
oli-obk committed Feb 5, 2025
1 parent 9f5473f commit 9a2073d
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 29 deletions.
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.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()
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_lint/src/types/literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
38 changes: 17 additions & 21 deletions src/tools/clippy/clippy_lints/src/approx_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down Expand Up @@ -73,30 +73,36 @@ impl ApproxConstant {
msrv: conf.msrv.clone(),
}
}
}

fn check_lit(&self, cx: &LateContext<'_>, lit: &LitKind, e: &Expr<'_>) {
match *lit {
impl<'tcx> LateLintPass<'tcx> for ApproxConstant {
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, 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"),
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, e, s, "f{32, 64}"),
LitKind::Float(s, LitFloatType::Unsuffixed) => self.check_known_consts(cx, lit.span, s, "f{32, 64}"),
_ => (),
}
}

fn check_known_consts(&self, cx: &LateContext<'_>, e: &Expr<'_>, s: symbol::Symbol, module: &str) {
extract_msrv_attr!(LateContext);
}

impl ApproxConstant {
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",
Expand All @@ -110,16 +116,6 @@ 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);
}
}

extract_msrv_attr!(LateContext);
}

/// Returns `false` if the number of significant figures in `value` are
/// less than `min_digits`; otherwise, returns true if `value` is equal
/// to `constant`, rounded to the number of digits present in `value`.
Expand Down
1 change: 1 addition & 0 deletions tests/ui/pattern/overflowing-literals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type TooBigSigned = pattern_type!(i8 is 200..);
fn main() {
match 5_u8 {
500 => {}
//~^ ERROR literal out of range for `u8`
_ => {}
}
}
10 changes: 9 additions & 1 deletion tests/ui/pattern/overflowing-literals.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,13 @@ LL | type TooBigSigned = pattern_type!(i8 is 200..);
= note: the literal `200` does not fit into the type `i8` whose range is `-128..=127`
= help: consider using the type `u8` instead

error: aborting due to 3 previous errors
error: literal out of range for `u8`
--> $DIR/overflowing-literals.rs:17:9
|
LL | 500 => {}
| ^^^
|
= note: the literal `500` does not fit into the type `u8` whose range is `0..=255`

error: aborting due to 4 previous errors

0 comments on commit 9a2073d

Please sign in to comment.