Skip to content
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

fake return edge triggers clippy::panic_in_result_fn lint for functions returning Result #2498

Closed
SanchithHegde opened this issue Mar 5, 2023 · 2 comments

Comments

@SanchithHegde
Copy link

Bug Report

Version

(Version number from Rust playground as of creating this issue)

tracing-attributes: 0.1.23

Platform

I've noticed this on both Linux (CI builds) and MacOS (development machines).

Crates

tracing-attributes: 0.1.23

Description

Sample code: (Rust playground)

#![warn(clippy::panic_in_result_fn)]

#[tracing_attributes::instrument]
fn main() -> Result<(), String> {
    Ok(())
}

The fake return edge added in #2270 triggers the clippy::panic_in_result_fn lint for the above sample code (and async functions which return Result). The only way of ignoring the lint is to annotate the entire function with #[allow(clippy::panic_in_result_fn)], which I'd like to avoid as it allows calling panicking code elsewhere in the function.

I tried to allow the lint only for the fake return edge block on a fork of tracing but didn't succeed. Let me know if I should open an issue on the clippy repository for ignoring this lint on macro generated code (this might also be related to rust-lang/rust-clippy#407 in that case).

I'd be happy to open a PR to fix this, I'm just not sure how this can be solved though, and whether a change must be made in tracing-attributes or in the clippy codebase.

@hawkw
Copy link
Member

hawkw commented Mar 6, 2023

Hmm, it's surprising that adding an allow attribute for the lint on just the if for the fake return edge doesn't work, especially because we already have allow attributes for other lints. I guess the lint is evaluated at the level of the whole function, rather than on individual blocks within the function, or something? It seems like it might be worth opening an upstream issue against clippy that an allow attribute for that lint doesn't work when placed on a block within a function?

If we can't allow the unreachable! just in that block, I think the only way tracing can make the lint go away is to suppress it for the entire #[instrument]ed function, which I agree is unfortunate...the best course of action is probably to figure out why we can't allow it just on the if block.

@SanchithHegde
Copy link
Author

With the Rust 1.73 clippy update, the panic_in_result_fn lint does not fire on unreachable!() macro invocations (see rust-lang/rust-clippy#11123, thanks @panosfol).

I've confirmed that this issue no longer occurs on Rust 1.73 on our codebase, will be closing this issue as completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants