remove redundant Send impl for references

[RalfJung]

Also explain why the other instance is not redundant, move it next to the trait they are implementing, and out of the redundant module. This seems to go back all the way to 35ca50b, not sure why the module was added.

The instance for &mut is the default instance we get anyway, and we don't have anything similar for Sync, so IMO we should be consistent and not have the redundant instance here, either.

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

[RalfJung]

Oh, the instance is actually needed somehow. But how...?!?

[RalfJung]

Ah no, that's just a harmless error message change.

[joshtriplett]

Was this ever needed (and the compiler just got smarter), or was this always redundant?

[RalfJung]

I don't think the mutable reference instance was ever needed (given that I can find no trace of similar instances for Sync), but I can't really speak for Rust before 1.0 so who knows.

[RalfJung]

r? @thomcc

[thomcc]

Hmm, these are kind of nice to have for documentation reasons, if I'm being honest. I know once upon a time having these explicit impls helped me understand how Send/Sync interacted with references. Perhaps that's a failing of our documentation elsewhere, though...

[RalfJung]

Having them for Send but not Sync seems more confusing than helpful for me.

I'm happy to add back the one for Send and then also the ones for Sync, and explicitly say they are redundant and documentation-only, if you prefer that.

[thomcc]

Hm, that's a pretty good point, I think I'm sold. Doing something like that for documentation should probably be in the actual documentation, rather than in the code.

@bors r+

[bors]

📌 Commit 73d655e has been approved by thomcc

It is now in the queue for this repository.

[RalfJung]

@bors rollup

[thomcc]

This changes code on a blanket impl for all references on a widely used trait. Even if semantically there is no change, I genuinely have no idea how that might impact compiler performance, are you sure rollup=always is appropriate?

[RalfJung]

I didn't think there is any chance of a perf regression here. Also note this has been rolled up before I even added that flag. If you think a PR has a chance of perf regression you need to do bors rollup=iffy or, better, just trigger a timing build.

[thomcc]

Yeah, I have no clue if it matters for perf. I usually just leave it as maybe if that's the case.