DbTxn must_use doesn't work

[kayabaNerve]

This is presumably somewhat a bug in Rust itself. I just need to make a MRE and evaluate it.

Identified due to #561.

[kayabaNerve]

rust-lang/rust#102238 (comment) for my comment on the relevant issue.

[kayabaNerve]

#[must_use]
trait X {}

fn y<Z: X>(z: Z) -> Z {
  z
}

fn a<Z: X>(z: Z) {
    y(z);
}

#[must_use]
struct A;
impl X for A {}

fn main() {
  a(A);
}

for the MRE.

[kayabaNerve]

We can wrap the DbTxn generic in a struct which is must_use to workaround this. It's ugly but would be safe.

[kayabaNerve]

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=82ba3d8b66d59ac5a9faa415032775e3

must_use only requires use on a branch, not on all branches :///

[kayabaNerve]

The struct (and the correct application of must_use) would only guarantee the txn is bound, not moved out of scope.

Opened an issue calling for a new clippy lint for an expanded interpretation of must_use: rust-lang/rust-clippy#13997

[kayabaNerve]

I did try the idea in rust-lang/rust-clippy#13997 (comment). While it is incomplete, I wanted to see if it was sufficiently complete for our use case. For the remaining edge cases, we could write unsafe_droppable_txn and manually review it with extra scrutiny.

The compiler will generate code to drop the txn if the txn is held across an await point (in case the future is dropped). We'd have to wrap the txn in a droppable struct before each await point, unwrapping it after. That'd be a notable ergonomic pain.

We'd also have to move to panic=abort if the txn is held across code which may panic. I'm actually fine with this regarding production deployments. I believe we only have unwind due to Substrate and we should review why that was needed/preferred and downscope such that only the Substrate crates have it. The issue is that we can't build tests with abort, and this code would only compile under abort, unless we use a nightly flag -Zpanic_abort_tests. The question would be can we have this assert trigger only if compiling with abort, so unwind still works but we get the static verification by a build with abort?