SemVer hazard: adding a Drop impl

[djc]

It appears that adding a Drop impl for a type that previously didn't have on can be a semver hazard. Example error from rustls/pki-types#79:

djc-2021 main rustls-pki-types $ cargo c
    Checking rustls-pki-types v1.11.0 (/Users/djc/src/rustls-pki-types)
error[E0505]: cannot move out of `key` because it is borrowed
   --> src/lib.rs:287:52
    |
285 |     fn try_from(key: Vec<u8>) -> Result<Self, Self::Error> {
    |                 --- binding `key` declared here
286 |         Ok(match PrivateKeyDer::try_from(&key[..])? {
    |                  ----------------------------------
    |                  |                        |
    |                  |                        borrow of `key` occurs here
    |                  a temporary with access to the borrow is created here ...
287 |             PrivateKeyDer::Pkcs1(_) => Self::Pkcs1(key.into()),
    |                                                    ^^^ move out of `key` occurs here
...
291 |     }
    |     - ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `ControlFlow<Result<Infallible, &str>, PrivateKeyDer<'_>>`
    |
    = note: the temporary is part of an expression at the end of a block;
            consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block
    |
286 ~         let x = Ok(match PrivateKeyDer::try_from(&key[..])? {
287 |             PrivateKeyDer::Pkcs1(_) => Self::Pkcs1(key.into()),
288 |             PrivateKeyDer::Sec1(_) => Self::Sec1(key.into()),
289 |             PrivateKeyDer::Pkcs8(_) => Self::Pkcs8(key.into()),
290 ~         }); x
    |
help: consider cloning the value if the performance cost is acceptable
    |
286 |         Ok(match PrivateKeyDer::try_from(&key.clone()[..])? {
    |                                              ++++++++

We found that the SemVer reference currently doesn't seem to talk about this, although the Drop reference docs have a bunch of context in https://doc.rust-lang.org/std/ops/trait.Drop.html#drop-check.

cc @obi1kenobi

[ehuss]

Thanks for the report! Yes, I believe it is a major change. This is currently tracked in #8736. I'm on the fence whether or not each individual item should be a separate issue, but I'm concerned that adding 100+ issues tracking each unwritten rule is going to be a little overwhelming at this stage.

[djc]

Sorry, I did search for existing issues but didn't find that one as I was looking for more specific issues. Is the fact that that issue has been open for 5 years without meaningful progress just a matter of resources (time/energy), or are there other blocks? I suppose perhaps automation (in the form of something like cargo-semver-checks) would ultimately be preferrable?

[ehuss]

It's just resources.

The ultimate goal is to also have cargo-semver-checks in-tree, but we still need to write down what the rules are. Someone just needs to write them down. Unfortunately writing the rules tends to be a lot of work to get them correct, clear, and to get consensus.

[obi1kenobi]

Strongly agree that the SemVer reference is a very useful resource even for cargo-semver-checks: whenever possible, we include a link to the reference that explains the breakage in more detail, because that's preferable from an ergonomics perspective over c-s-c giving the user an essay to read whenever breakage is found.

I'd be interested in chatting at RustWeek about expanding the SemVer reference, specifically if there's anything on my end that can make writing it easier like providing examples, describing the edge cases and rules, etc. We already have to figure this stuff out anyway, and I'd love it if that info were useful to you as well.

We have a number of lints where we've figured all that out already but are currently not backed by any SemVer reference page. They instead point to other places as the best available alternative, often not a great one. I'd be happy to trial the above with those lints if that'd be useful.

The Drop impl hazard on the cargo-semver-checks end is tracked by: obi1kenobi/cargo-semver-checks#930