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

clippy::suspicious_else_formatting triggered by fake_return_edge #2410

Open
str4d opened this issue Dec 9, 2022 · 2 comments
Open

clippy::suspicious_else_formatting triggered by fake_return_edge #2410

str4d opened this issue Dec 9, 2022 · 2 comments

Comments

@str4d
Copy link

str4d commented Dec 9, 2022

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;
}
};
let block = quote! {
{
#fake_return_edge
#block
}
};

This generates code (with Rust 1.56.0) like:

        #[allow(clippy::suspicious_else_formatting)]
        {
            let __tracing_attr_span;
            let __tracing_attr_guard;
            if tracing::Level::INFO <= ::tracing::level_filters::STATIC_MAX_LEVEL
                && tracing::Level::INFO <= ::tracing::level_filters::LevelFilter::current()
            {
                // span
            }
            #[warn(clippy::suspicious_else_formatting)]
            {
                #[allow(
                    unreachable_code,
                    clippy::diverging_sub_expression,
                    clippy::let_unit_value
                )]
                if false {
                    let __tracing_attr_fake_return = ...;
                    return __tracing_attr_fake_return;
                }
                {
                    // Function content
                }
            }
        }

We can see that the code added to fix #1613 looks correct, but we need to do the same thing for fake_return_edge.

@str4d
Copy link
Author

str4d commented Dec 9, 2022

We noticed this because we tried to fix it ourselves in our code, but because the new instance is nested inside the first one, the fix to #1613 insulates the new instance and prevents external attributes from affecting it.

@str4d
Copy link
Author

str4d commented Dec 9, 2022

Alternatively, we could move the #[warn(clippy::suspicious_else_formatting)] onto where the user's block now occurs. Given that fake_return_edge is used to redefine block, I presume that is how the issue slipped in.

str4d added a commit to str4d/tracing that referenced this issue Jan 9, 2023
The fake return edge was added in tokio-rs#2270 to improve type
errors in instrumented async functions. However, it meant that the user
block was being modified outside of `gen_block`. This created a negative
interaction with tokio-rs#1614, which suppressed a clippy lint
internally while explicitly enabling it on the user block. The installed
fake return edge generated this same lint, but the user had no way to
suppress it: lint directives above the instrumentation were ignored
because of the internal suppression, and lints inside the user block
could not influence the fake return edge's scope.

We now avoid modifying the user block outside of `gen_block`, and
restrict the fake return edge to async functions. We also apply the same
clippy lint suppression technique to the installed fake return edge.

Closes tokio-rs#2410.
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

1 participant