Better document the implementors of Clone and Copy #48171
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
pietroalbini
added
the
S-waiting-on-review
Centril
added
T-lang
|
Sorry for the delayed response. This looks plausible to me, and I like the idea of moving this out of compiler magic and into the library itself. I'd like to get at least one other reviewer to check this. |
Should we solicit opinions of people who might be affected by this? |
|
I believe there are practically no legitimate use cases of no_core other than compiler developers trying to minimize a problem occurring in libcore. Using it is inherently very deeply entangled with compiler internals. Many routine changes can already break no_core usage, such as changing anything about any lang item (including adding or removing them), or even just changing something about the compiler such that compiling your code requires a lang item that previously wasn't required. Basically, using no_core out of tree is not supported. Edit: Indeed the only use of no_core (other than by compiler developers) I've ever seen was in this twitter thread and that thread exists precisely because using no_core is so insane. |
I know that I've needed to use it when attempting to get AVR-Rust off the ground, I kind of assumed that other embedded folk have needed to do the same. |
|
Probably couldn't hurt to raise attention on this via TWiR and/or an internals thread. |
|
@shepmaster Good point, I forgot about AVR-Rust, sorry! I have reason to suspect AVR-Rust is pretty unique in this regard, though. Most embedded stuff is happening with more mature LLVM backends which have no problem (or not nearly as big a problem, e.g., they may just be missing support for some obscure intrinsics) getting libcore to compile. For example, MPS430 seems to have working libcore . |
|
And to be clear, "getting a new target off the ground" is a totally legitimate use of no_core (I've even used it for that myself), but I count that under "usage by compiler developers" and it should usually be very temporary (AVR apparently being an exception). Since there are few compiler developers and even fewer targets being added, I don't think it's particularly likely that this PR will land after someone copied something from libcore, yet before they scrapped that copy and started compiling libcore as-is. |
|
To be clear, I don't have a problem with this change affecting AVR-Rust, it's simply the only reason I know about
I think this is where AVR-Rust is at as well; it's just taking longer because the LLVM support is being worked on in parallel.
Not to worry; it's not anywhere near feature complete ;-) |
Can you say a bit more about the motivation here? Is it rustdoc or something else? |
|
@nikomatsakis several comments on the original issue stem from Stack Overflow question-askers not knowing that e.g. |
src/librustc_typeck/diagnostics.rs
Outdated
| @@ -1973,16 +1973,16 @@ differs from the behavior for `&T`, which is always `Copy`). | |||
|
|
|||
| E0206: r##" | |||
| You can only implement `Copy` for a struct or enum. Both of the following | |||
| examples will fail, because neither `i32` (primitive type) nor `&'static Bar` | |||
| (reference to `Bar`) is a struct or enum: | |||
| examples will fail, because neither `fn() -> i32` nor `&'static mut Bar` | |||
There was a problem hiding this comment.
I don't think we should include fn() -> i32 as a sample type. &'static mut Bar is ok I guess.
There was a problem hiding this comment.
What about [u8; 256]?
There was a problem hiding this comment.
Also, can you add a new test for what happens when we do implement for u32 (outside of libcore)? I guess we get a standard overlap failure, huh? Seems ok.
|
@nikomatsakis Addressed nits. I'll squashed and rebase when it's approved. |
|
☔ The latest upstream changes (presumably #48449) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
|
@FraGag looks good =) |
FraGag
force-pushed
the
doc-copy-clone-impls
branch
from
6b1dbc6 to
556128b
Compare
|
@nikomatsakis Squashed and rebased. The merge conflicts were just line numbers in |
src/libcore/marker.rs
Outdated
| bool char | ||
| } | ||
|
|
||
| #[stable(feature = "rust1", since = "1.0.0")] |
There was a problem hiding this comment.
I guess it doesn't matter, but ! has not been present since 1.0.0. In fact, the type itself isn't even stable yet!
|
OK, I updated to 1.25.0 |
The libc crate is used as a dependency of the Rust compiler. Its build system passes `--cfg dox` to all crates when generating their documentation. libc's documentation is generated when the build system is asked to generate the compiler documentation because `cargo doc` automatically documents all dependencies. When the dox configuration option is enabled, libc disables its dependency on the core crate and provides the necessary definitions itself. The dox configuration option is meant for generating documentation for a multitude of targets even if the core crate for that target is not installed. However, when documenting the compiler, it's not necessary to do that; we can just use core or std as usual. This change is motivated by the changes made to the compiler in rust-lang/rust#48171. With these changes, it's necessary to provide implementations of the Clone and Copy traits for some primitive types in the library that defines these traits (previously, these implementations were provided by the compiler). Normally, these traits (and thus the implementations) are provided by core, so any crate that uses `#![no_core]` must now provide its own copy of the implementations. Because libc doesn't provide its own copy of the implementations yet, and because the compiler's build system passes `--cfg dox` to libc, generating the documentation for the compiler fails when generating documentation for libc. By renaming the configuration option, libc will use core or std and will thus have the necessary definitions for the documentation to be generated successfully.
The libc crate is used as a dependency of the Rust compiler. Its build system passes `--cfg dox` to all crates when generating their documentation. libc's documentation is generated when the build system is asked to generate the compiler documentation because `cargo doc` automatically documents all dependencies. When the dox configuration option is enabled, libc disables its dependency on the core crate and provides the necessary definitions itself. The dox configuration option is meant for generating documentation for a multitude of targets even if the core crate for that target is not installed. However, when documenting the compiler, it's not necessary to do that; we can just use core or std as usual. This change is motivated by the changes made to the compiler in rust-lang/rust#48171. With these changes, it's necessary to provide implementations of the Clone and Copy traits for some primitive types in the library that defines these traits (previously, these implementations were provided by the compiler). Normally, these traits (and thus the implementations) are provided by core, so any crate that uses `#![no_core]` must now provide its own copy of the implementations. Because libc doesn't provide its own copy of the implementations yet, and because the compiler's build system passes `--cfg dox` to libc, generating the documentation for the compiler fails when generating documentation for libc. By renaming the configuration option, libc will use core or std and will thus have the necessary definitions for the documentation to be generated successfully.
Rename the dox configuration option to cross_platform_docs The libc crate is used as a dependency of the Rust compiler. Its build system passes `--cfg dox` to all crates when generating their documentation. libc's documentation is generated when the build system is asked to generate the compiler documentation because `cargo doc` automatically documents all dependencies. When the dox configuration option is enabled, libc disables its dependency on the core crate and provides the necessary definitions itself. The dox configuration option is meant for generating documentation for a multitude of targets even if the core crate for that target is not installed. However, when documenting the compiler, it's not necessary to do that; we can just use `core` or `std` as usual. This change is motivated by the changes made to the compiler in rust-lang/rust#48171. With these changes, it's necessary to provide implementations of the `Clone` and `Copy` traits for some primitive types in the library that defines these traits (previously, these implementations were provided by the compiler). Normally, these traits (and thus the implementations) are provided by core, so any crate that uses `#![no_core]` must now provide its own copy of the implementations. Because libc doesn't provide its own copy of the implementations yet, and because the compiler's build system passes `--cfg dox` to libc, generating the documentation for the compiler fails when generating documentation for libc. By renaming the configuration option, libc will use `core` or `std` and will thus have the necessary definitions for the documentation to be generated successfully. **Note:** rust-lang/rust#48171 is blocked on this PR and on a release of libc including this change on crates.io. (Some crates in the compiler use libc as a submodule, while others use a version from crates.io.)
Rename the dox configuration option to cross_platform_docs The libc crate is used as a dependency of the Rust compiler. Its build system passes `--cfg dox` to all crates when generating their documentation. libc's documentation is generated when the build system is asked to generate the compiler documentation because `cargo doc` automatically documents all dependencies. When the dox configuration option is enabled, libc disables its dependency on the core crate and provides the necessary definitions itself. The dox configuration option is meant for generating documentation for a multitude of targets even if the core crate for that target is not installed. However, when documenting the compiler, it's not necessary to do that; we can just use `core` or `std` as usual. This change is motivated by the changes made to the compiler in rust-lang/rust#48171. With these changes, it's necessary to provide implementations of the `Clone` and `Copy` traits for some primitive types in the library that defines these traits (previously, these implementations were provided by the compiler). Normally, these traits (and thus the implementations) are provided by core, so any crate that uses `#![no_core]` must now provide its own copy of the implementations. Because libc doesn't provide its own copy of the implementations yet, and because the compiler's build system passes `--cfg dox` to libc, generating the documentation for the compiler fails when generating documentation for libc. By renaming the configuration option, libc will use `core` or `std` and will thus have the necessary definitions for the documentation to be generated successfully. **Note:** rust-lang/rust#48171 is blocked on this PR and on a release of libc including this change on crates.io. (Some crates in the compiler use libc as a submodule, while others use a version from crates.io.)
|
@FraGag weekly ping from the triage team! What's the status on this? |
Add implementations of `Clone` and `Copy` for some primitive types to libcore so that they show up in the documentation. The concerned types are the following: * All primitive signed and unsigned integer types (`usize`, `u8`, `u16`, `u32`, `u64`, `u128`, `isize`, `i8`, `i16`, `i32`, `i64`, `i128`); * All primitive floating point types (`f32`, `f64`) * `bool` * `char` * `!` * Raw pointers (`*const T` and `*mut T`) * Shared references (`&'a T`) These types already implemented `Clone` and `Copy`, but the implementation was provided by the compiler. The compiler no longer provides these implementations and instead tries to look them up as normal trait implementations. The goal of this change is to make the implementations appear in the generated documentation. For `Copy` specifically, the compiler would reject an attempt to write an `impl` for the primitive types listed above with error `E0206`; this error no longer occurs for these types, but it will still occur for the other types that used to raise that error. The trait implementations are guarded with `#[cfg(not(stage0))]` because they are invalid according to the stage0 compiler. When the stage0 compiler is updated to a revision that includes this change, the attribute will have to be removed, otherwise the stage0 build will fail because the types mentioned above no longer implement `Clone` or `Copy`. For type variants that are variadic, such as tuples and function pointers, and for array types, the `Clone` and `Copy` implementations are still provided by the compiler, because the language is not expressive enough yet to be able to write the appropriate implementations in Rust. The initial plan was to add `impl` blocks guarded by `#[cfg(dox)]` to make them apply only when generating documentation, without having to touch the compiler. However, rustdoc's usage of the compiler still rejected those `impl` blocks. This is a [breaking-change] for users of `#![no_core]`, because they will now have to supply their own implementations of `Clone` and `Copy` for the primitive types listed above. The easiest way to do that is to simply copy the implementations from `src/libcore/clone.rs` and `src/libcore/marker.rs`. Fixes rust-lang#25893
There are types that implement `Clone` and `Copy` but are not mentioned in the documentation, because the implementations are provided by the compiler. They are types of variants that cannot be fully covered by trait implementations in Rust code, because the language is not expressive enough.
Simply checking for the presence of `llvm.memset` is too brittle because this instrinsic can be used for seemingly trivial operations, such as zero-initializing a `RawVec`.
FraGag
force-pushed
the
doc-copy-clone-impls
branch
from
5905c9a to
87c08f9
Compare
|
Now that rust-lang/libc#951 has been merged and libc 0.2.40 published, I can progress. I just rebased and tweaked my branch to account for the changes made in #49299, which made some changes to the documentation for |
pietroalbini
added
S-waiting-on-review
|
Ping from triage, @nikomatsakis ! |
|
@bors r+ |
|
📌 Commit 87c08f9 has been approved by |
bors
added
S-waiting-on-bors
Better document the implementors of Clone and Copy There are two parts to this change. The first part is a change to the compiler and to the standard library (specifically, libcore) to allow implementations of `Clone` and `Copy` to be written for a subset of builtin types. By adding these implementations to libcore, they now show up in the documentation. This is a [breaking-change] for users of `#![no_core]`, because they will now have to supply their own copy of the implementations of `Clone` and `Copy` that were added in libcore. The second part is purely a documentation change to document the other implementors of `Clone` and `Copy` that cannot be described in Rust code (yet) and are thus provided by the compiler. Fixes #25893
|
☀️ Test successful - status-appveyor, status-travis |
There are two parts to this change. The first part is a change to the compiler and to the standard library (specifically, libcore) to allow implementations of
CloneandCopyto be written for a subset of builtin types. By adding these implementations to libcore, they now show up in the documentation. This is a [breaking-change] for users of#![no_core], because they will now have to supply their own copy of the implementations ofCloneandCopythat were added in libcore.The second part is purely a documentation change to document the other implementors of
CloneandCopythat cannot be described in Rust code (yet) and are thus provided by the compiler.Fixes #25893