prevent deref coercions in pin!

[dianne]

Fixes #153438 using a (hopefully temporary!) typed macro idiom to ensure that when pin! produces a Pin<&mut T>, its argument is of type T. See #153438 (comment) for my ideas on how this could be changed in the future.

[rustbot]

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 69 candidates
• Random selection from 15 candidates

[dianne]

I expect this needs crater, so: @bors try

[dianne]

Just realized I could format things more cleanly (diff). This shouldn't affect the correctness of the try build.

[eggyal]

A more explicit approach could be to do something like:

super let mut pinned = $value;

#[allow(unreachable_code)]
'p: {
    fn unreachable_type_constraint<'a, T>(_: T) -> Pin<&'a mut T> {
        unreachable!()
    }

    break 'p unsafe { Pin::new_unchecked(&mut pinned) };
    unreachable_type_constraint(pinned)
}

[dianne]

So that's how to add types to macros! Funky idiom, but I do prefer the explicitness of actually having types, yeah. I think that'd be less likely to break inference too if anything's started relying on it; iirc type expectations are propagated from the block to the break expression (via the block's coercion target type). But it's fine because we'll encounter an error instead of silently adding a coercion if things don't match up.. I think.

Would you prefer to make your own PR @eggyal, or should I just work that into this one? I'm not a library reviewer so I can't say which would be preferable from that perspective, but from the perspective I do have, I think adding a type constraint is a better fix.

[eggyal]

By all means work it into this one. This idiom was originally suggested to me by lcnr some years ago, so I claim no credit for it!

[Kivooeo]

this change requires someone from libs team to be approved, so i'm reassigning

r? libs

[rust-bors]

☀️ Try build successful (CI)
Build commit: 048f484 (048f484e31141edc1fa71e20406300e92a9c16ba, parent: 64b72a1fa5449d928d5f553b01a596b78ee255d2)

[dianne]

Went ahead with the unreachable type constraint version of this (diff). There's unfortunately a bit of a diagnostics regression due to the extra type checking. I still think it's worth the lower likelihood of breakage, but it's a tradeoff. I'll be firing off a fresh try build, but if the diagnostics are a sticking point, maybe it'd be worth comparing crater results for both approaches?

[dianne]

@bors try

[rust-bors]

☀️ Try build successful (CI)
Build commit: 5d96fa0 (5d96fa0e954d77528204a1ba3b8847ec083c779b, parent: 64b72a1fa5449d928d5f553b01a596b78ee255d2)

[dianne]

I'll crater the type constraint approach first since that's my preferred quick fix at the moment.
@craterbot check

Also going to try re-running pr-check-2. It never restarted after being canceled it looks like?

[craterbot]

👌 Experiment pr-153457 created and queued.
🤖 Automatically detected try build 5d96fa0
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more