- Feature Name: `typeck_docs` - Start Date: (fill me in with today's date, YYYY-MM-DD) - RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) - Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) # Summary [summary]: #summary Changing rustdoc to not accept broken code any more: * Type checking function bodies (the starting point), to avoid ICEs, to support auto traits properly, to make features like intra-doc links and source page links more tractable and less buggy and weird. * Exporting `doc.rmeta` files from rustdoc, instead of using rustc's, to improve support for these sorts of docs. * The transition plan. # Motivation [motivation]: #motivation Rustdoc claims to not run type checking on function bodies, and accepts a lot of broken code besides that. This is how the standard library generates a single pile of HTML docs that covers multiple supported platforms. Using the compiler's API this way doesn't work well: it causes Internal Compiler Errors, inconsistent output, and it doesn't work consistently. This problem dates back to [before 1.0][#1998], and it's gotten worse. This proposal aims to make the problem more tractable, and to make rustdoc's behavior more consistent, at the cost of making the conditional compilation story a bit worse and encouraging library authors to use platform switchers like docs.rs defaults to. [#1998]: https://github.com/rust-lang/rust/issues/1998 ## A short introduction to conditional compilation and rustdoc (the status quo) To port code to multiple platforms, Rust provides a "conditional compilation" feature closely related to macros. It operates on AST nodes before type checking. ```rust #[cfg(windows)] pub const fn number() -> i32 { 1 } #[cfg(unix)] pub const fn number() -> u32 { 2 } ``` The above setup is fine, as long as a platform can't both be Windows and Unix at the same time. If that happened, it would fail, because you can't have multiple functions with the same name. To allow doc authors to write multi-platform docs (docs where multiple platforms are included in the same HTML output), rustdoc lets them include items with the `doc` pseudo-platform, like this: ```rust // This example is synthetic, but similar code can be found in reality. // For example, `OpenOptions::custom_flag` has two versions on Tokio. // https://github.com/tokio-rs/tokio/blob/a58beb3aca18e6ec4cb444b6c78d5a3373742998/tokio/src/fs/open_options.rs#L463 // And `std::sys::args::args` is created through a bunch of re-exports like this: // https://github.com/rust-lang/rust/blob/master/library/std/src/sys/windows/args.rs // https://github.com/rust-lang/rust/blob/master/library/std/src/sys/mod.rs#L28 #[cfg(all(not(doc), windows))] pub const fn number() -> i32 { 1 } #[cfg(any(doc, unix))] pub const fn number() -> u32 { 2 } ``` You can't just show both functions in the same documentation, because Rustdoc generates `fn.number.html` for both functions, and you can't have more than one *file* with the same name on most filesystems. Also, the above example won't look right. We really want the function to say "supported on both windows and unix". To make that work, use `doc(cfg())`. ```rust #[cfg(all(not(doc), windows))] pub const fn number() -> i32 { 1 } #[cfg(any(doc, unix))] // turn off auto cfg, since it's not really indicative // also, show the "real" support matrix by manually specifying it with doc(cfg) #[doc(no_auto_cfg, cfg(any(unix, windows)))] pub const fn number() -> u32 { 2 } ``` The standard library does this, because it's convenient for readers. So do other Big(tm) libraries, like Tokio. Smaller libraries don't usually do this, because it requires extra work, so docs.rs offers a platform picker drop down menu and builds the docs more than once instead. ### How a function's body affects rustdoc, or, more specifically, how it does not Let's instead consider an example like this, with two functions that both get documented[^incomplete]. I'll call this `foobar_core`: <a name="foobar_core" id="foobar_core"></a> [`foobar_core`]: #foobar_core ```rust use std::io; use std::ffi::c_void; use std::path::Path; #[cfg(any(doc, unix))] pub fn permission_bitmask(path: &Path) -> io::Result<u32> { Ok(std::os::unix::fs::PermissionsExt::mode(&std::fs::metadata(path)?.permissions())) } #[cfg(any(doc, windows))] pub fn security_information(path: &Path) -> io::Result<(Box<c_void>, Box<c_void>, Box<c_void>)> { let mut dacl = Box::new(windows_sys::um::winnt::ACL { AclRevision: 0, bz1: 0, AclSize: 0, AceCount: 0, Sbz2: 0, }); let mut sacl = Box::new(windows_sys::um::winnt::ACL { AclRevision: 0, bz1: 0, AclSize: 0, AceCount: 0, Sbz2: 0, }); let mut security_descriptor = Box::new(ptr::null_mut()); let result = windows_sys::um::aclapi::GetSecurityInfo( std::os::windows::io::IntoRawHandle::into_raw_handle(std::fs::File::open(path)?), windows_sys::SE_FILE_OBJECT, 0, ptr_null(), ptr_null(), &mut dacl, &mut sacl, &mut security_descriptor, ); if result != 0 { return Err(std::io::Error::new()); } Ok((dacl, sacl, security_descriptor)) } ``` Should the above code work? It *does* on current versions of rustdoc, because all of the references to platform-specific code are inside the function bodies, and rustdoc doesn't type check them. [^incomplete]: The above example makes Windows look a lot more complicated than "Unix," but that's a bit deceptive. Unix is a family of operating systems, and the above code supports a least common denominator subset. Adding support for SELinux ACLs and the macOS app sandbox would reveal the complexity that actually exists in UNIX-land. Also, the above example was never tested: it compiles, but probably doesn't run. Fixing this, or adding more complete support for the extended ACLs offered by modern Unixes, would be a distraction. This *should* be okay, because Rust doesn't have global type inference, so a function's signature completely describes its API. Here's where that's specifically pointed out in [the old Rust FAQ]: [the old Rust FAQ]: https://prev.rust-lang.org/en-US/faq.html#why-arent-function-signatures-inferred > #### Why aren't function signatures inferred? > > In Rust, declarations tend to come with explicit types, while actual code has its types inferred. There are several reasons for this design: > > * Mandatory declaration signatures help enforce interface stability at both the module and crate level. > * Signatures improve code comprehension for the programmer, eliminating the need for an IDE running an inference algorithm across an entire crate to be able to guess at a function's argument types; it's always explicit and nearby. > * Mechanically, it simplifies the inference algorithm, as inference only requires looking at one function at a time. This design choice was made load-bearing in rustdoc, and code like the above example can be found in real-world crates like Tokio and the standard library. In particular, the above example code could use `windows-sys` as a platform-specific Cargo dependency, which means all those references to the crate within the function wouldn't resolve. ```toml [target.x86_64-pc-windows-msvc.dependencies] windows-sys = "0.48" ``` This forces cross-platform docs to write their code in a bit of a convoluted style, since referencing these platform-specific types in the wrong place will cause rustdoc to fail, but it can be done. ### Code that rustdoc accepts, and what it promises to accept As described in [the rustdoc book], it does not run type checking on function bodies. This is a documented promise that this RFC plans to break. [the rustdoc book]: https://doc.rust-lang.org/rustdoc/advanced-features.html#interactions-between-platform-specific-docs > To document your crate, Rustdoc only needs to know the public signature of your functions. In particular, it doesn't have to know how any of your functions are implemented, so it ignores all type errors and name resolution errors with function bodies. Note that this does not work for anything outside a function body: since Rustdoc documents your types, it has to know what those types are! Currently, rustdoc accepts code that is broken in ways other than its function bodies. For example, multiple inherent methods with the same name, as shown in the below code example: <a name="multi_methods" id="multi_methods"></a> [`multi_methods`]: #multi_methods ```rust // Call this example multi_methods pub struct MultiMethods; impl MultiMethods { pub fn foo() {} pub fn foo(&self) {} } ``` ```text notriddle:multi_methods$ cargo +nightly doc Documenting multi_methods v0.1.0 (/home/notriddle/multi_methods) Finished dev [unoptimized + debuginfo] target(s) in 0.18s notriddle:multi_methods$ cargo +nightly run Compiling multi_methods v0.1.0 (/home/notriddle/multi_methods) error[E0592]: duplicate definitions with name `foo` --> multi_methods/src/main.rs:4:5 | 3 | pub fn foo() {} | ------------ other definition for `foo` 4 | pub fn foo(&self) {} | ^^^^^^^^^^^^^^^^^ duplicate definitions for `foo` For more information about this error, try `rustc --explain E0592`. error: could not compile `multi_methods` (bin "multi_methods") due to previous error ``` If function bodies are going to be checked, then method resolution needs to work, and this kind of error also needs fixed. This one will cause rustdoc to behave strangly: <a name="multi_methods_const" id="multi_methods_const"></a> [`multi_methods_const`]: #multi_methods_const ```rust // Call this example multi_methods_const pub struct MultiMethods; impl MultiMethods { pub const fn foo() {} pub const fn foo(self) {} } pub const MULTI_METHODS: () = MultiMethods.foo(); ``` ```text error[E0599]: no method named `foo` found for struct `MultiMethods` in the current scope --> multi_methods_const/src/main.rs:7:44 | 2 | pub struct MultiMethods; | ----------------------- method `foo` not found for this struct ... 7 | pub const MULTI_METHODS: () = MultiMethods.foo(); | -------------^^^-- | | | | | this is an associated function, not a method | help: use associated function syntax instead: `MultiMethods::foo()` | = note: found the following associated functions; to be used as methods, functions must have a `self` parameter note: the candidate is defined in an impl for the type `MultiMethods` --> sync_ck/src/main.rs:4:5 | 4 | pub const fn foo() {} | ^^^^^^^^^^^^^^^^^^ ``` Removing the static method fixes it. ## Why this doesn't work very well for writing cross-platform docs ### Using platform-specific items in a function's signature doesn't work Consider the [`foobar_core`] example again. If I dropped the use of `c_void` in the previous example, instead using the Windows-defined type aliases, it would fail to compile on Linux. ```rust #[cfg(any(doc, windows))] pub fn security_information(path: &Path) -> io::Result<( Box<windows_sys::um::winnt::ACL>, Box<windows_sys::um::winnt::ACL>, Box<windows_sys::um::winnt::PSECURITY_DESCRIPTOR> )> { ``` ```text error[E0433]: failed to resolve: use of undeclared crate or module `windows_sys` --> foobar_core/src/lib.rs:10:61 | 10 | ... -> io::Result<(Box<windows_sys::um::winnt::ACL>, Box<windows_sys::um::winnt::ACL>, Box<winapi::um:... | ^^^^^^^^^^^ use of undeclared crate or module `windows_sys` error[E0433]: failed to resolve: use of undeclared crate or module `windows_sys` --> foobar_core/src/lib.rs:10:95 | 10 | ...m::winnt::ACL>, Box<windows_sys::um::winnt::ACL>, Box<winapi::um::winnt::PSECURITY_DESCRIPTOR>)> { | ^^^^^^^^^^^ use of undeclared crate or module `windows_sys` error[E0433]: failed to resolve: use of undeclared crate or module `windows_sys` --> foobar_core/src/lib.rs:10:129 | 10 | ...ys::um::winnt::ACL>, Box<windows_sys::um::winnt::PSECURITY_DESCRIPTOR>)> { | ^^^^^^^^^^^ use of undeclared crate or module `windows_sys` ``` The rustdoc book mentions this, and it definitely is needed to actually show the links in the generate HTML, but it's not very useful. To make this work better, the `windows-sys` crate would need to be pulled in by Cargo and supplied when building the docs. ### Cannot have doc-specific dependencies Unfortunately, while the naive solution to the above problem is that Cargo needs to pull in the windows crates for generating cross-platform docs... that's not a feature that Cargo has. Since `doc` isn't an actual platform feature, this won't work: ```toml # The `doc` "target" isn't respected, and doesn't work [target.'cfg(any(doc, windows))'.dependencies] windows-sys = "0.48" ``` [Tokio works around this][tokio-workaround] by hand-copying chunks of the Windows API into their own docs. The standard library doesn't have this problem, since it has no public, platform-specific dependencies. [tokio-workaround]: https://docs.rs/tokio/latest/tokio/doc/index.html ### Rustdoc's "inlining" feature, and how it also doesn't work with cross-platform docs Rust has converged on a design pattern called a "facade crate." The standard library is an example, since it re-exports items directly from `core` and `alloc`. To make this work better, rustdoc allows exports to be marked as "inlining," where a `use` declaration gets shown as if it were the original item. If we wanted to make a facade crate for `foobar_core` (called `foobar`), we might re-export these items like this: <a name="foobar" id="foobar"></a> [`foobar`]: #foobar ```rust #[doc(inline)] #[cfg(any(doc, unix))] use foobar_core::permission_bitmask; #[doc(inline)] #[cfg(any(doc, windows))] use foobar_core::security_information; ``` This looks like it ought to work, but it doesn't: ```text error[E0432]: unresolved import `foobar_core::security_information` --> src/lib.rs:6:5 | 6 | use foobar_core::security_information; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `security_information` in the root ``` This happens because Rustdoc actually uses the normal crate metadata for inlining, rather than pulling from the docs. Things that are only present in `cfg(doc)` aren't available to inline. ## Problems caused by not checking function bodies ### Auto traits, `impl Trait` return values As described in [RFC 1522], auto traits are structurally inferred for Return Position Impl Traits. > The type would not be known to implement any other trait, *with the exception of OIBITS (aka "auto traits") and default traits like Sized.* [RFC 1522]: https://rust-lang.github.io/rfcs/1522-conservative-impl-trait.html This also includes async functions, which desugar to impl Trait. Consider the default example, called `sync_ck`: <a name="sync_ck" id="sync_ck"></a> [`sync_ck`]: #sync_ck ```rust use std::fmt::Debug; use std::ptr; pub fn yes_sync() -> impl Debug { 1 } pub fn not_sync() -> impl Debug { ptr::null::<()>() } fn main() { fn need_sync(_: impl std::marker::Sync) {} need_sync(yes_sync()); // Uncomment this line, and get an error. //need_sync(not_sync()); } ``` ```text error[E0277]: `*const ()` cannot be shared between threads safely --> sync_ck/src/main.rs:13:15 | 6 | pub fn not_sync() -> impl Debug { | ---------- within this `impl Debug` ... 13 | need_sync(not_sync()); | --------- ^^^^^^^^^^ `*const ()` cannot be shared between threads safely | | | required by a bound introduced by this call | = help: within `impl Debug`, the trait `Sync` is not implemented for `*const ()` note: required because it appears within the type `impl Debug` --> sync_ck/src/main.rs:6:22 | 6 | pub fn not_sync() -> impl Debug { | ^^^^^^^^^^ note: required by a bound in `need_sync` --> sync_ck/src/main.rs:10:26 | 10 | fn need_sync(_: impl std::marker::Sync) {} | ^^^^^^^^^^^^^^^^^ required by this bound in `need_sync` ``` This is a part of the function's API (since it determines when you're allowed to call it), but figuring it out requires type-checking its body. Rustdoc doesn't do it, so generated docs don't indicate auto traits on return-position impl trait types. `impl Trait` is relatively rare in public APIs right now, since they can't be named, but [RFC 2515] proposes to name them, so I expect them to become more popular and this missing feature in rustdoc to become more starkly felt. [RFC 2515]: https://rust-lang.github.io/rfcs/2515-type_alias_impl_trait.html This feature didn't exist at the time [the old Rust FAQ] was written: arguably, it's a form of global type inferrence, and violates the assumptions behind the original design decision for rustdoc to ignore function bodies. For example, if we give `yes_sync`'s return type a name, then use it in a struct, as shown in the below example called `sync_ck_with_struct`: <a name="sync_ck_with_struct" id="sync_ck_with_struct"></a> [`sync_ck_with_struct`]: #sync_ck_with_struct ```rust #![feature(type_alias_impl_trait)] mod nested { use std::fmt::Debug; pub type YesSync = impl Debug; pub fn yes_sync() -> YesSync { 1 } //pub fn not_sync() -> YesSync { // ptr::null::<()>() //} } pub struct Info { _yes_sync: nested::YesSync, } fn main() { fn need_sync(_: impl std::marker::Sync) {} need_sync(Info { _yes_sync: nested::yes_sync() }); } ``` This will result in rustdoc type-checking the `yes_sync` function's body, in order to determine that `struct Info` is thread-safe. ```text Documenting sync_ck_with_struct v0.1.0 (/home/notriddle/sync_ck_with_struct) error[E0433]: failed to resolve: could not resolve path `windows_sys::test` --> sync_ck_with_struct/src/main.rs:6:9 | 6 | windows_sys::test | ^^^^^^^^^^^^^^^^^ could not resolve path `windows_sys::test` | = note: this error was originally ignored because you are running `rustdoc` = note: try running again with `rustc` or `cargo check` and you may get a more detailed error ``` ### Other unchecked invariants cause ICE <https://github.com/rust-lang/rust/pull/107289> > I worry that we are allowing code to be even more broken than before, in ways that are not tested or guaranteed, but people will still depend on. The code paths that rustdoc has to follow are different from rustc. Because most people don't take advantage of rustdoc's ability to document broken code, it's not very well tested, and often suffers ICEs or accidental breakage. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation Are you arriving at this doc from a compatibility warning like this one? ```text warning[E9999]: duplicate definitions with name `foo` --> sync_ck/src/main.rs:4:5 | 3 | pub fn foo() {} | ------------ other definition for `foo` 4 | pub fn foo(&self) {} | ^^^^^^^^^^^^^^^^^ duplicate definitions for `foo` Note: this used to be accepted in older versions of rustdoc, even though it is invalid Rust, and will be rejected in newer editions ``` This is because the new edition of rustdoc runs more type inference on your code, and this requires the function body to pass type checking. * If this is a name resolution error on a platform-specific dependency, add it as a `doc-dependency` to Cargo.toml * If this is a duplicate method error, where multiple methods with the same name are defined as `cfg(doc)`, replace the `cfg(doc)` with `cfg(not(doc))` on all but one of them. Alternatively, if the two methods have different type signatures and are platform-specific, move them to extension traits like the ones in the standard library. * If this is a type checking error caused by hardcoding a type, where a type alias is used in the API definition (such as using `u32` instead of `windows::DWORD`), use the type alias instead of the concrete name. This may require adding a `doc-dependency` to Cargo.toml. * If this is a type checking error caused by platform differences, and there's no type alias that you can use to avoid it, the *last resort* is to wrap the "real" implementation in `cfg(not(doc))`, like this: ```rust fn my_fn() { #[cfg(doc)] { unimplemented!() } #[cfg(not(doc))] { // implementation here! } } ``` # Reference-level explanation [reference-level-explanation]: #reference-level-explanation ## The new edition This RFC proposes two changes, together because they're interrelated: * The `doc.rmeta` file, which will carry essentially the same data as regular `rmeta`, but built with `cfg(doc)`. Rustdoc-specific metadata in the future can be used specifically in the doc.rmeta files, but for now the proposal is to not change stuff (third-parties that don't use Cargo are going to want lots of warnings and general ways to plan head). * Type-checking the bodies of functions. This depends on the first change in order to make the feature work transitively; if my dependencies (such as `libstd`) can use `cfg(doc)` to write cross-platform docs, then I should be able to use these platform-specific items in my own platform-specific docs. ## Case studies ### How this is fixed when generating rustdoc pages for the standard library The standard library suffered the most breakage to make doc.rmeta work. The fixes to `stdarch` can be found here: https://github.com/notriddle/stdarch/commits/typeck-docs The fixes to the standard library can be found here: https://github.com/notriddle/rust/commits/notriddle/typeck-docs ### How about cargo? https://github.com/notriddle/cargo/commits/notriddle/doc-rmeta Generating docs for it worked out of the box, but since rustdoc is now responsible for generating doc.rmeta instead of rustc, Cargo needs changed to actually do that. ### How this is fixed when generating rustdoc pages for the compiler itself The compiler itself required no changes, and the only transitive dependencies that caused problems are `quote`, `syn`, and `smallvec`. `syn` used to require a cursed workaround for a rustdoc bug, but this was already fixed. The compiler just didn't use a new enough version. I bumped the version from 2.0.8 to 2.0.28. `smallvec` ignores the const generics feature when `cfg(doc)` was active. I can't find any info on why in [servo/smallvec#265] where the line was introduced, but it was easy enough to fix: [servo/smallvec#265]: https://github.com/servo/rust-smallvec/pull/265/files#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759R2067 ```diff diff --git a/src/lib.rs b/src/lib.rs index 3aa673f..265a180 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2378,7 +2378,7 @@ impl<T, const N: usize> SmallVec<[T; N]> { } } -#[cfg(all(feature = "const_generics", not(doc)))] +#[cfg(feature = "const_generics")] #[cfg_attr(docsrs, doc(cfg(feature = "const_generics")))] unsafe impl<T, const N: usize> Array for [T; N] { type Item = T; @@ -2388,7 +2388,7 @@ unsafe impl<T, const N: usize> Array for [T; N] { } } -#[cfg(any(not(feature = "const_generics"), doc))] +#[cfg(not(feature = "const_generics"))] macro_rules! impl_array( ($($size:expr),+) => { $( @@ -2401,7 +2401,7 @@ macro_rules! impl_array( } ); -#[cfg(any(not(feature = "const_generics"), doc))] +#[cfg(not(feature = "const_generics"))] impl_array!( 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 36, 0x40, 0x60, 0x80, 0x100, 0x200, 0x400, 0x600, 0x800, 0x1000, ``` `quote` is the one with a serious problem, because they do this, for the sake of cleaner, more readable docs: ```rust /// Docs here. #[cfg(doc)] #[macro_export] macro_rules quote! { (($x:tt)*) => { ... } } #[cfg(not(doc))] macro_rules quote! { // real implementation ... ``` Yes, line 6 is a literal `...`. By generating the rmeta file based on the docs, the non-working "doc version" gets used instead of the real one. This is not obviously fixable on rustdoc's side, because it leaves us in a double bind: * If doc.rmeta doesn't contain cfg(doc)-ed out items, then multiplatform docs don't work right. They compile, but inlining behaves weirdly (consider what happens in the current version of Rust, when someone writes `#[doc(inline)] pub use quote::quote` and they get the ugly, real version instead of the doc version). CC [#114952] * If doc.rmeta does contain cfg(doc)-ed out items, then `quote` is broken. [#114952]: https://github.com/rust-lang/rust/issues/114952 The short-term solution is the edition system. The old edition still needs to generate doc.rmeta files (see the [edition] section for more info on what's going on), but it should be generated with `cfg(doc)` turned *off*. This the actual fix, as a patch to the `quote` crate, is to make the "pretty" version of `quote!` publicly visible, and have a `doc(hidden)` implementation that it directly calls out to: ```diff #[macro_export] macro_rules quote! { (($x:tt)*) => { $crate::quote_i!($x) } } #[macro_export] #[doc(hidden)] macro_rules quote_i! { ``` ### How this is fixed in tokio and mio Tokio conditionally depends on the `windows_sys` crate, and also has a comflicting method definition. Both are fixed with the following patch. ```diff diff --git a/tokio/Cargo.toml b/tokio/Cargo.toml index 05157cdb..cef0d174 100644 --- a/tokio/Cargo.toml +++ b/tokio/Cargo.toml @@ -120,7 +120,7 @@ signal-hook-registry = { version = "1.1.1", optional = true } libc = { version = "0.2.149" } nix = { version = "0.27.1", default-features = false, features = ["fs", "socket"] } -[target.'cfg(windows)'.dependencies.windows-sys] +[dependencies.windows-sys] version = "0.48" optional = true diff --git a/tokio/src/fs/open_options.rs b/tokio/src/fs/open_options.rs index 6e2aa0cd..86a004bb 100644 --- a/tokio/src/fs/open_options.rs +++ b/tokio/src/fs/open_options.rs @@ -458,6 +458,7 @@ feature! { /// Ok(()) /// } /// ``` + #[cfg(not(doc))] pub fn custom_flags(&mut self, flags: i32) -> &mut OpenOptions { self.as_inner_mut().custom_flags(flags); self diff --git a/tokio/src/lib.rs b/tokio/src/lib.rs index 21d19bac..5f21d580 100644 --- a/tokio/src/lib.rs +++ b/tokio/src/lib.rs @@ -631,11 +631,6 @@ pub mod stream {} #[cfg(docsrs)] pub mod doc; -#[cfg(docsrs)] -#[allow(unused)] -pub(crate) use self::doc::os; - -#[cfg(not(docsrs))] #[allow(unused)] pub(crate) use std::os; diff --git a/tokio/src/net/windows/named_pipe.rs b/tokio/src/net/windows/named_pipe.rs index a03e1d0a..8fe63282 100644 --- a/tokio/src/net/windows/named_pipe.rs +++ b/tokio/src/net/windows/named_pipe.rs @@ -16,28 +16,14 @@ cfg_io_util! { use bytes::BufMut; } -// Hide imports which are not used when generating documentation. -#[cfg(not(docsrs))] -mod doc { - pub(super) use crate::os::windows::ffi::OsStrExt; - pub(super) mod windows_sys { - pub(crate) use windows_sys::{ - Win32::Foundation::*, Win32::Storage::FileSystem::*, Win32::System::Pipes::*, - Win32::System::SystemServices::*, - }; - } - pub(super) use mio::windows as mio_windows; -} - -// NB: none of these shows up in public API, so don't document them. -#[cfg(docsrs)] -mod doc { - pub(super) mod mio_windows { - pub type NamedPipe = crate::doc::NotDefinedHere; - } +use crate::os::windows::ffi::OsStrExt; +pub(crate) mod windows_sys { + pub(crate) use windows_sys::{ + Win32::Foundation::*, Win32::Storage::FileSystem::*, Win32::System::Pipes::*, + Win32::System::SystemServices::*, + }; } - -use self::doc::*; +use mio::windows as mio_windows; // NOTE: mio needs patched for this to work /// A [Windows named pipe] server. /// ``` It also depends on socket2, which needs a workaround for an Apple-specific feature. While socket2 and libstd both have the features they need to generate platform-agnostic docs, libc does not, so even when doing a doc build, you still need to cope with missing symbols: ```diff --- /a/src/sys/unix.rs +++ /b/src/sys/unix.rs @@ -1173,8 +1173,9 @@ self._set_nosigpipe(nosigpipe) } - #[cfg(target_vendor = "apple")] + #[cfg(all(feature = "all", any(doc, target_vendor = "apple")))] pub(crate) fn _set_nosigpipe(&self, nosigpipe: bool) -> io::Result<()> { + #[cfg(target_vendor = "apple")] unsafe { setsockopt( self.as_raw(), @@ -1183,6 +1184,9 @@ nosigpipe as c_int, ) } + #[cfg(not(target_vendor = "apple"))] { + unimplemented!() + } } /// Gets the value of the `TCP_MAXSEG` option on this socket. ``` To build with the `docsrs` feature flag set, tokio also requires the duplicate method `custom_flags` to be fixed, and needs `mio` to support multi-platform docs. It also depends on `quote`, which has the problem mentioned above. # Drawbacks [drawbacks]: #drawbacks ## Overall complexity, and a huge refactor Working through these implications, it's obvious that we're at a local maximum right now. Fixing one thing requires fixing a dozen other things to support the fix, which means this change is *hard*. There are a few cheats available to us, like adjusting the older editions so that they turn *all* errors into warnings and so that `delay_span_bug`-type diagnostics are swallowed, but that's a *cheat*. The goal of this change is to make a new, shiny future where Rustdoc's behavior is easily described in the documentation, but the old edition where it's not still exists. ## Transitivity of platform-independent docs A major factor in this design is that doc.rmeta can expose all of the supported platforms. In particular, the `libstd.doc.rmeta` that ships with the standard library contains all supported platforms, so you can do intra-doc links to *Linux* stuff *even when you run `cargo doc` on Windows*. Just `use std::os::linux::WHATEVER`. It's great. That can only work if the library you link to makes their platform-specific stuff `cfg(doc)`-available. In particular, to make `tokio` docs work across platforms, I either need to shim `mio` (which is how tokio does it right now) or I need to fix mio to support multiplatform docs, too. This is a downside to how docs and conditional compilation work in Rust. This proposal doesn't solve it. It just pushes it onto library maintainers to solve it instead. # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives ## Stop using the compiler for documentation This only brings us marginally closer to the [holy grail]. Presumably, the holy grail would involve rustdoc-specific metadata (since making it work would almost certainly be a perf regression, and we don't want to push that onto the compiler), so this is a small part of it. Things get nasty once the type checker is involved, though. Conditional compilation is allowed to do things, like [`multi_methods`], that play badly with consts and const generics that make the type checker call out to the miri evaluator. At that point, having two methods with the same name is bad. This doesn't actually prove that the holy grail is a myth. Ain't no rule that says every item you see in the docs has to be fed into the type checker. But what doesn't exist in the eyes of the type checker can't be called in a `const`. Making conditionally compiled items visible without `cfg(doc)` on them could get confusing in narrow circumstances like that (though the compiler [already stashes] the names of cfg()-ed out items, so we could at least guide the user towards a workaround, that's annoying if you're a dependency of the crate using conditional compilation). Anything that can make this work would, presumably, also be great for IDEs that want to be smart about platform-specific code, like [matklad's blog] gripes about. > But of course we shouldn’t implement conditional compilation by barbarically cutting the AST, and instead should push conditional compilation to after the type checking, so that you at least can check, on Linux, that the windows version of your code wouldn’t fail due to some stupid typos in the name of `#[cfg(windows)]` functions. Alas, I don’t know how to design such conditional compilation system. [holy grail]: https://github.com/rust-lang/rust/issues/1998 [already stashes]: https://github.com/rust-lang/rust/pull/109005 [matklad's blog]: https://matklad.github.io/2021/11/27/notes-on-module-system.html # Prior art [prior-art]: #prior-art The thing that makes me question the wisdom of this approach is that nobody else seems to do it this way. Many languages just don't have this problem. Most dynamically-typed languages either write docs entirely separately or use a simple parser to extract docs (like how [sphinx parses tokens directly to extract docstrings from python](https://github.com/sphinx-doc/sphinx/blob/master/sphinx/pycode/parser.py)). The main exception I know of is Elixir, which loads the module to build docs, and has a strong anti-conditional-compilation culture to avoid this problem. This also isn't a problem if you just don't have conditional compilation. For example, in Java, calling a function with [operating system specific semantics](https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#getPosixFilePermissions(java.nio.file.Path,%20java.nio.file.LinkOption...) on the wrong operating system gets a runtime `UnsupportedOperationException`. ## Haddock https://haskell-haddock.readthedocs.io/ This one looks like it has an implementation closer to Rustdoc. Including the same downsides, * They're talking about [having GHC prepare doc comments for haddock to use](https://github.com/haskell/haddock/issues/1566). Older versions of Haddock used the GHC API itself. * They're storing doc comments in [Haskell Interface `.hi` files](https://ghc.gitlab.haskell.org/ghc/doc/users_guide/separate_compilation.html), which are normally carried alongside the `.o` files and store type definitions. Again, very familiar sounding. * These two facts, together, imply that if you're [using CPP](https://discourse.haskell.org/t/what-is-the-accepted-solution-for-conditionnal-compilation/7376) to do conditional compilation on Haskell code, you're as bad off with Haddock as you are with Rustdoc. And if `cabal`, their package manager, is used for conditional compilation instead, then GHC, and by extension Haddock, can't see it. ## DDoc / DDox Be forewarned: I know even less about D than I do about Haskell. Two major factors seem relevant here. * https://dlang.org/spec/ddoc.html * https://dlang.org/spec/version.html DMD DDoc seems to extract doc comments from a `Module` https://github.com/dlang/dmd/blob/fdc0c96dfa7b02919958cf3f6914286635a3734c/compiler/src/dmd/doc.d#L400 which, at least, means symbol resolution needs to work. As a result, they've written some of their stdlib code twice: once as a doc, once as actual code, when OS-specific interfaces are needed, since they want multi-platform docs and need something that can compile: https://github.com/dlang/phobos/blob/f41348cda418561ed381f2815e06940a09d03f24/std/windows/charset.d DDox, their next-gen documenter, has similar limitations because of how it interfaces with the compiler: https://github.com/dlang/ddox#known-issues # Unresolved questions [unresolved-questions]: #unresolved-questions ## Do doc.rmeta files get generated in the same compilation session as docs? Does it work like this? (single command, pseudocode) rustdoc src/lib.rs --write-metadata-to target/debug/libfoo.doc.rmeta --write-html-to target/doc Or like this? (two commands, pseudocode) rustc --cfg doc src/lib.rs -o target/debug/libfoo.doc.rmeta rustdoc src/lib.rs --write-html-to target/doc They're supposed to be subject to as-similar-as-possible type checking logic, but are they actually done in the same command? If it's a single command, presumably there's an option that lets you write the metadata but not html. Doing them in a single session probably has better performance. There's a lot of queries that get stored in memory and not written to disk, and doing these two steps in a single session allows them to be reused. But doing them in the same session means that these two artifacts (html and rmeta) are generated by the same command, unless you only want the rmeta, which has a separate command. This is probably too complicated for build systems that aren't Cargo and don't want to write custom logic just for rustdoc, so the toolchain needs to support doing them separately. # Future possibilities [future-possibilities]: #future-possibilities ## Report auto traits on `impl Trait` If function bodies can be evaluated, then support for impl Trait can improve. > `pub fn async_function() -> impl Future` [^i] > > [^i]: > > ## Auto traits for `impl Future` > > ``` > impl Send for impl Future > ``` > > ``` > impl Sync for impl Future > ```