Skip to content

Commit

Permalink
Auto merge of rust-lang#12884 - y21:issue12881, r=Alexendoo
Browse files Browse the repository at this point in the history
Only run `suboptimal_flops` on inherent method calls

Fixes rust-lang#12881

`suboptimal_flops` was making the wrong assumption that a `.log()` method call on a float literal must choose the inherent log method that will always have an argument present (in which case `args[0]` indexing would be fine), but that wasn't the case in the linked issue because at the point of the method call, the exact float type hadn't been inferred yet (and method selection can't select inherent methods when the exact float type has not yet been inferred, in which case it falls back to looking for trait impls and chooses the one that didn't have any parameters).

This fixes it by actually making sure it's a call to an inherent method (could also fix the linked ICE by simply using fallibly indexing via `.get()`, but this felt like it'd fix the root cause: even if there were one argument, it would still be wrong to emit a warning there because it's not the `log` method the lint was expecting). I'm not sure if we need that extra function be in `clippy_utils` but it feels like it could be useful.

changelog: Fixes an ICE in [`suboptimal_flops`]
  • Loading branch information
bors committed Jun 4, 2024
2 parents 5d568ad + 708ef79 commit 46e3304
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 2 deletions.
5 changes: 3 additions & 2 deletions clippy_lints/src/floating_point_arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use clippy_utils::consts::Constant::{Int, F32, F64};
use clippy_utils::consts::{constant, constant_simple, Constant};
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::{
eq_expr_value, get_parent_expr, higher, in_constant, is_no_std_crate, numeric_literal, peel_blocks, sugg,
eq_expr_value, get_parent_expr, higher, in_constant, is_inherent_method_call, is_no_std_crate, numeric_literal,
peel_blocks, sugg,
};
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, PathSegment, UnOp};
Expand Down Expand Up @@ -759,7 +760,7 @@ impl<'tcx> LateLintPass<'tcx> for FloatingPointArithmetic {
if let ExprKind::MethodCall(path, receiver, args, _) = &expr.kind {
let recv_ty = cx.typeck_results().expr_ty(receiver);

if recv_ty.is_floating_point() && !is_no_std_crate(cx) {
if recv_ty.is_floating_point() && !is_no_std_crate(cx) && is_inherent_method_call(cx, expr) {
match path.ident.name.as_str() {
"ln" => check_ln1p(cx, expr, receiver),
"log" => check_log_base(cx, expr, receiver, args),
Expand Down
9 changes: 9 additions & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,15 @@ pub fn match_trait_method(cx: &LateContext<'_>, expr: &Expr<'_>, path: &[&str])
.map_or(false, |trt_id| match_def_path(cx, trt_id, path))
}

/// Checks if the given method call expression calls an inherent method.
pub fn is_inherent_method_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
if let Some(method_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) {
cx.tcx.trait_of_item(method_id).is_none()
} else {
false
}
}

/// Checks if a method is defined in an impl of a diagnostic item
pub fn is_diag_item_method(cx: &LateContext<'_>, def_id: DefId, diag_item: Symbol) -> bool {
if let Some(impl_did) = cx.tcx.impl_of_method(def_id) {
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/floating_point_log.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,19 @@ fn check_ln1p() {
let _ = (1.0 + x - 2.0).ln();
}

fn issue12881() {
pub trait MyLog {
fn log(&self) -> Self;
}

impl MyLog for f32 {
fn log(&self) -> Self {
4.
}
}

let x = 2.0;
x.log();
}

fn main() {}
15 changes: 15 additions & 0 deletions tests/ui/floating_point_log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,19 @@ fn check_ln1p() {
let _ = (1.0 + x - 2.0).ln();
}

fn issue12881() {
pub trait MyLog {
fn log(&self) -> Self;
}

impl MyLog for f32 {
fn log(&self) -> Self {
4.
}
}

let x = 2.0;
x.log();
}

fn main() {}

0 comments on commit 46e3304

Please sign in to comment.