Pedantic lint: unhandled_must_use

[kayabaNerve]

What it does

#[must_use]
struct MyStruct;
fn x() -> MyStruct {
  MyStruct
}

fn main() {
  let x = x();
  if core::hint::black_box(false) {
    return;
  }
  drop(x);
}

For the above snippet, must_use will not be emitted as a warning because the variable binding is considered usage. The proposed lint would consider any bound must_use types at the termination of scope unused. The above example includes the edge-case where the scope has multiple points of potential termination and the binding is used at one location but not the other.

I will note this may be hard to implement on any must_use type which also implements Copy. This either needs to not trigger on must_use + Copy, ignoring that class of types, or determine if the variable was copied, and if so, only continue evaluation for the copies' bindings (if any exist).

Alternative name suggestions for the lint welcome. in_scope_must_use? unmoved_must_use?

Advantage

• Ensures must_use types are 'handled', not just bound.
• My provocation for this is a DbTxn type I have. I once lost days of developer time because I forgot to call commit, despite the must_use present. I would like my clippy to error if I ever don't call commit or explicitly drop any DbTxn.

Drawbacks

• Pedantic lint, making it low priority

Example

  let x: Result<(), ()> = Err(());
  if core::hint::black_box(false) {
    return;
  }
  drop(x);

Could be written as:

  let x: Result<(), ()> = Err(());
  if core::hint::black_box(false) {
    drop(x);
    return;
  }
  drop(x);

[samueltardieu]

Note that other solutions exist for the problem you expose. For example, you might want to put a flag inside your structure (or wrap it in another one along that uses Deref if you can't modify it) and set the flag just before the transaction is committed or properly abandoned. The flag can then be checked at drop time, and panic or log an error if it has not been set, meaning that the wrapper transaction has not been properly "terminated".

As a general remark, if this lint ever exists, it should belong to restriction IMO, as calling drop explicitly on all must_use variables is not idiomatic Rust.

[kayabaNerve]

Making this a runtime panic wouldn't be preferred or even viable IMO. It means any missed uses go from stalls (lack of progress over the database) to full crashes. I appreciate the note though.

I suggested this for pedantic as I thought pedantic was for explicit opt-in lints which don't make sense to enable as a class due to highly specific/contradictory lints. Apologies if I was in error there.

[samueltardieu]

I understand. For info, the various categories are detailed here. From what I've seen in many projects, pedantic lints are often enabled as a whole, with some being opted out, while restriction lints are usually individually opted in.

[kayabaNerve]

While further researching solutions, I noted this topic apparently has had discussion before re: linear types (potentially making this of wider value): https://faultlore.com/blah/linear-rust/#the-checker

https://jack.wrenn.fyi/blog/undroppable suggests having the drop fn have a const assert so build fails if it's ever code-gen'd. Not only is that hell to debug, as it doesn't identify the location of code generation, solely the location of the failing assert, it's incomplete. For whatever reason, Rust can't observe that this value will never have its drop fn called.

struct Undroppable<T>(T);
impl<T> Drop for Undroppable<T> {
  fn drop(&mut self) {
    const {
      assert!(false);
    }
  }
}

struct MyStruct;

fn main() {
  let x = Undroppable(MyStruct);
  let branch = core::hint::black_box(true);
  if branch {
    core::mem::forget(x);
  } else {
    core::mem::forget(x);
  }
}

Setting the inner value of black_box(...) to either true or false causes a compilation error. Removing the call to black_box causes it to compile for either true or false.

I write this here for the sake of completeness.