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

New lint affecting tracing::instrument macro when returning an impl #2613

Open
damccull opened this issue Jun 10, 2023 · 3 comments
Open

New lint affecting tracing::instrument macro when returning an impl #2613

damccull opened this issue Jun 10, 2023 · 3 comments

Comments

@damccull
Copy link

When I updated to the latest clippy via rustup update, I ran into a new lint that doesn't seem to like returning impl SomeTrait: https://rust-lang.github.io/rust-clippy/master/index.html#/let_with_type_underscore

So tracing, when used like this is causing this lint to trigger.

#[tracing::instrument(name = "Login form", skip(flashes))]
pub async fn login_form(flashes: IncomingFlashes) -> impl IntoResponse {
//...
}
@sandhose
Copy link

The code which triggers this lint is generated here:

// Install a fake return statement as the first thing in the function
// body, so that we eagerly infer that the return type is what we
// declared in the async fn signature.
// The `#[allow(..)]` is given because the return statement is
// unreachable, but does affect inference, so it needs to be written
// exactly that way for it to do its magic.
let fake_return_edge = quote_spanned! {return_span=>
#[allow(unreachable_code, clippy::diverging_sub_expression, clippy::let_unit_value, clippy::unreachable)]
if false {
let __tracing_attr_fake_return: #return_type =
unreachable!("this is just for type inference, and is unreachable code");
return __tracing_attr_fake_return;
}
};

When the return type is an impl Trait, the #return_type here is _.

I think there are two solutions for this:

  • either add the clippy::let_with_type_underscore to the allowed lints
  • or completely skip this block when the return type is an impl Trait, because that block doesn't help with inference in those cases?

@mladedav
Copy link
Contributor

This issue seems to have been fixed #2609.

However, what this did to us is clippy now fails for us because we're using an older rust version, where this lint is not present yet.

I don't think adding a warning to compilation is a breaking change, but it might be worthwhile to also add #![allow(unknown_lints)] for this and future similar cases where we do not want to break all compilations, right?

@mladedav
Copy link
Contributor

This issue has been fixed and can be closed now.

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

3 participants