branches_sharing_code gives wrong recommendations

[Matthias247]

Lint name: branches_sharing_code

The lint complains about the following code and wants drop(conn) to be moved outside of the if.
However obviously that doesn't work, because

1. The if condition refers to it. Now obviously the result of the if could be moved into a temporary variable, drop(conn) could be called and another if could be used - but is this really better?
2. conn is a MutexGuard. We can't just randomly move it in different places.

let conn = (self.conn.as_mut().unwrap()).lock("into_0rtt");
if conn.inner.has_0rtt() || conn.inner.side().is_server() {
     drop(conn);
     let conn = self.conn.take().unwrap();
     Ok((NewConnection::new(conn), ZeroRttAccepted(self.connected)))
} else {
    drop(conn);
    Err(self)
}

warning: all if blocks contain the same code at the start
   --> quinn/src/connection.rs:102:9
    |
102 | /         if conn.inner.has_0rtt() || conn.inner.side().is_server() {
103 | |             drop(conn);
    | |_______________________^
    |
    = note: `#[warn(clippy::branches_sharing_code)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#branches_sharing_code
help: consider moving the start statements out like this
    |
102 |         drop(conn);
103 |         if conn.inner.has_0rtt() || conn.inner.side().is_server() {
    |

Meta

• cargo clippy -V: clippy 0.1.54 (770792f 2021-05-07)
• rustc -Vv:

    rustc 1.54.0-nightly (770792ff8 2021-05-07)
    binary: rustc
    commit-hash: 770792ff8d1ec542e78e77876ac936f43ffb8e05
    commit-date: 2021-05-07
    host: x86_64-unknown-linux-gnu
    release: 1.54.0-nightly
    LLVM version: 12.0.0
  

[djc]

From the same project:

        if self.cubic_state.w_max < self.cubic_state.w_last_max {
            self.cubic_state.w_last_max = self.cubic_state.w_max;
            self.cubic_state.w_max = self.cubic_state.w_max as f64 * (1.0 + BETA_CUBIC) / 2.0;
        } else {
            self.cubic_state.w_last_max = self.cubic_state.w_max;
        }

Clippy recommends we move the shared statement before the if, but since it mutates one of the sides of comparison, it would change the meaning of the comparison in a pretty subtle way.

[Pavlov123]

Similar case

if feature_flags.contains(&String::from("exception_reporting")) {
    let _guard = sentry::init((SENTRY_DSN, get_sentry_client_options()));
    ignite();
} else {
    ignite();
}

The sentry "session" will be closed after _guard goes out of scope the call to ignite() has to happen with the guard in scope.