Skip to content

[WIP] Trigger absurd_extreme_comparisons if Duration is less than zero #14817

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alex-semenyuk
Copy link
Member

@alex-semenyuk alex-semenyuk commented May 15, 2025

Close #14524

changelog: [absurd_extreme_comparisons]: triggers if Duration is less than 0

@rustbot
Copy link
Collaborator

rustbot commented May 15, 2025

r? @dswij

rustbot has assigned @dswij.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 15, 2025
@alex-semenyuk alex-semenyuk force-pushed the duration_absurd_extreme_comparisons branch from 118446e to e47e2fc Compare May 15, 2025 19:09
Copy link
Contributor

@samueltardieu samueltardieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In its current state, the lint would:

  • fail with a false positive in some situations where a non-zero Duration is returned by a function (see detailed comment)
  • fail with a false negative with other const initializers, such as Duration::new(0, 0), Duration::from_secs_f32(0), and Duration::from_secs_f64(0).

This can be fixed in several ways:

  • A list of methods to match against can be used, with a special case for Duration::new(0, 0) (easier)
  • Or the const evaluator could be enriched to include a Duration variant (more complex, probably not worth it)

Comment on lines 124 to 125
if let ExprKind::Call(_, args) = &expr.kind
&& args.len() == 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use something like:

Suggested change
if let ExprKind::Call(_, args) = &expr.kind
&& args.len() == 1
if let ExprKind::Call(_, [arg]) = &expr.kind

Comment on lines 126 to 127
&& is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Duration)
&& let Some(Constant::Int(0)) = ConstEvalCtxt::new(cx).eval(&args[0])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not robust enough, as this will match any function whose argument is 0 and returns a Duration, including a function such as:

    fn one_second_plus(n: u64) -> Duration {
        Duration::from_secs(n+1)
    }

which does not return a zero duration.

@alex-semenyuk alex-semenyuk force-pushed the duration_absurd_extreme_comparisons branch from e47e2fc to 22ce6a7 Compare May 15, 2025 21:26
@alex-semenyuk alex-semenyuk force-pushed the duration_absurd_extreme_comparisons branch from 22ce6a7 to d6feb1f Compare May 15, 2025 21:27
@alex-semenyuk alex-semenyuk marked this pull request as draft May 15, 2025 21:32
@alex-semenyuk alex-semenyuk changed the title Trigger absurd_extreme_comparisons if Duration is less than zero [WIP] Trigger absurd_extreme_comparisons if Duration is less than zero May 15, 2025
@samueltardieu
Copy link
Contributor

I know the PR is still in draft mode, but I'll comment right now since function name matching has changed recently: we don't use &str anymore to compare a function name against an expected value when we can use interned symbols from clippy_utils::sym, as comparisons are much faster. If sym::from_secs (for example) is not available already, you can add it to clippy_utils::sym and it will be made available.

But here, since you want to look up entities, there is also another way to go: you can add the path to the associated functions you need to identify, e.g., core::time::Duration::from_secs, in clippy_utils::paths.rs, as:

pub static DURATION_FROM_SECS: PathLookup = value_path!(core::time::Duration::from_secs);

You will then be able to use DURATION_FROM_SECS.matches(def_id) where def_id is the DefId of the function you want to check, and it will tell you whether this is indeed Duration::from_secs(). You shouldn't need to match the type and the associated function separately.

Hope it helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

absurd extreme comparisons not working for std::time::Duration
4 participants