Specify that packed types must derive, not implement, Copy

[Mark-Simulacrum]

No description provided.

[rust-highfive]

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

[cramertj]

Are all of the autogenerated derive impls already smart enough to Copy out fields of packed Copy types when they use them? I would have assumed they'd still attempt to create references to the fields, which is unsound.

[Mark-Simulacrum]

I'm not sure; the error message though somewhat indicates that they should be... it looks like there's some code in libsyntax_ext which does something to take care of it, but I don't follow it well enough to be sure. It seems somewhat unrelated to this change though.

[cramertj]

I just checked the behavior with --pretty-expanded and it looks like adding Copy causes the compiler to omit references to fields. Thanks!

@bors r+

[bors]

📌 Commit c604df6 has been approved by cramertj

[cramertj]

Ah, wait no-- it only omits the reference if the Copy impl was in derive-- it doesn't omit it in the case of manual Copy impls. Either that should be fixed (hard, I think) or this PR should be amended to check for a derive of Copy (rather than that the type implements Copy).

@bors r-

[Mark-Simulacrum]

There's actually not much point then -- the way derive is structured, if the type derives Copy, the packed derive will be "safe" and as such not generate this warning. I've updated the PR to change the warning message to indicate that the type must derive Copy, not just implement it; and reverted the other changes.

The long-term solution here is probably to drop the requirement that we "see" a Copy derive and generate code as-if the struct is Copy when we see that it's packed. Then we'd somehow ensure that structs who aren't Copy are errored early enough (before borrowck?) that the errors about moving out of a borrowed field don't get emitted. This is far harder though, and I don't have confidence in the correct approach, so I'm not going to work on that in this PR.

[cramertj]

Sounds good to me. Thanks for the updates! r=me with travis passing.

[Mark-Simulacrum]

@bors r=cramertj

[bors]

📌 Commit 033d75d has been approved by cramertj

[bors]

🔒 Merge conflict

[Mark-Simulacrum]

@bors r=cramertj

[bors]

📌 Commit 5c37473 has been approved by cramertj

[bors]

⌛ Testing commit 5c37473 with merge 0bdd29c7e43d60ce5e4e37ee7ce99770a057de65...

[bors]

💔 Test failed - status-travis

[rust-highfive]

The job x86_64-gnu-distcheck of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[02:58:28] Documenting stage2 std (x86_64-unknown-linux-gnu)
[02:58:28] Checking core v0.0.0 (file:///checkout/obj/build/tmp/distcheck/src/libcore)
[02:58:28] Documenting core v0.0.0 (file:///checkout/obj/build/tmp/distcheck/src/libcore)
[02:58:28] Compiling std v0.0.0 (file:///checkout/obj/build/tmp/distcheck/src/libstd)
The job exceeded the maximum time limit for jobs, and has been terminated.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[rust-highfive]

The job x86_64-gnu-distcheck of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[02:58:28] Documenting stage2 std (x86_64-unknown-linux-gnu)
[02:58:28] Checking core v0.0.0 (file:///checkout/obj/build/tmp/distcheck/src/libcore)
[02:58:28] Documenting core v0.0.0 (file:///checkout/obj/build/tmp/distcheck/src/libcore)
[02:58:28] Compiling std v0.0.0 (file:///checkout/obj/build/tmp/distcheck/src/libstd)
The job exceeded the maximum time limit for jobs, and has been terminated.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[Mark-Simulacrum]

@bors retry - timeout

[bors]

⌛ Testing commit 5c37473 with merge 3575be6...

[bors]

💥 Test timed out

[RalfJung]

Per @retep998's comment at #46043 (comment), this breaks WinAPI.

[retep998]

It doesn't break winapi so much as simply document that things actually are broken.