-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add documentation for function pointers as a primitive #43529
Conversation
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
Thanks to #17104 (comment) i have some impls to try to load, so i'll mark this WIP until i get that done. |
src/libstd/primitive_docs.rs
Outdated
/// assert_eq!(clos(5), 10); | ||
/// ``` | ||
/// | ||
/// All function pointers implement [`Copy`], as well as [`Fn`]/[`FnMut`]/[`FnOnce`] for their |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unsafe
function pointers do not implement Fn
traits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, fn
and unsafe fn
are effectively separate types. I should mention that. Thanks!
/// [`FnMut`]: ops/trait.FnMut.html | ||
/// [`FnOnce`]: ops/trait.FnOnce.html | ||
/// | ||
/// Plain function pointers are obtained by casting either plain functions, or closures that don't |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be useful to mention here somewhere that function pointers are always assumed to be non-null.
This is a common mistake in FFI to use fn()
in nullable callbacks when Option<fn()>
need to be used instead.
Okay, putting function pointer impls into the page was easier than i thought. I have a rendering up, if you want to preview what it looks like. Since i haven't put in any formatting for function pointer types in impls, it's kinda gnarly-looking, tho. On the other hand, i'm ready to call this good! I've expanded the docs out; hopefully i haven't written in something wrong. |
src/libstd/primitive_docs.rs
Outdated
/// ``` | ||
/// | ||
/// On top of that, function pointers can vary based on whether they're external, and then by what | ||
/// ABI they use. For example, `fn()` is different from `extern "C" fn()`, which itself is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On top of that, function pointers can vary based on whether they're external, and then by what ABI they use.
As the extern
keyword is just used to specify the ABI I think this sentence should just be something like "On top of that, function pointers can vary based on what ABI they use.".
src/libstd/primitive_docs.rs
Outdated
/// | ||
/// Extern function declarations can also be *variadic*, allowing them to be called with a variable | ||
/// number of arguments. Normal rust functions, even those with an `extern "ABI"`, cannot be | ||
/// variadic. For more information, see [the nomicon's section on variadic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extern function declarations can also be variadic, allowing them to be called with a variable number of arguments. Normal rust functions, even those with an
extern "ABI"
, cannot be variadic.
It's only functions with the "C"
or "cdecl"
that can be variadic so I think it would be clearer to say just something like Functions with the `"C"` or `"cdecl"` ABI can also be *variadic*, allowing them to be called with a variable number of arguments.
.
src/libstd/primitive_docs.rs
Outdated
/// function pointer over FFI and be able to accomodate null pointers, make your type | ||
/// `Option<fn()>` with your required signature. | ||
/// | ||
/// Function pointers, including `extern` functions with the `"C"` and `"Rust"` ABIs, implement the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These impls are only for "C"
and "Rust"
so this caveat should be with the 12 arguments or less caveat.
It would be nice for rustdoc to automatically generate links to this page by changing this to something like the following: write!(f, "{}{}", UnsafetySpace(decl.unsafety), AbiSpace(decl.abi))?;
primitive_link(f, PrimitiveType::Fn, "fn")?;
write!(f, "{}{}", decl.generics, decl.decl) |
@ollie27 That did the trick! Thanks for pointing that out; i totally didn't think to check it. I've updated the docs with your suggestions, too. |
This looks great! You noted in #43560 that they'll need to be rebased off of each other; in general, r=me, but I'm not sure how you want to tackle that. |
Since #43560 got the r+ first, i guess i'd like to wait until that's in-tree before pushing this one forward. That way we don't get homu's hopes up or feel like i need to scramble to get the rebase in place. |
add docs for references as a primitive Just like #43529 did for function pointers, here is a new primitive page for references. This PR will pull in impls on references if it's a reference to a generic type parameter. Initially i was only able to pull in impls that were re-exported from another crate; crate-local impls got a different representation in the AST, and i had to change how types were resolved when cleaning it. (This is the change at the bottom of `librustdoc/clean/mod.rs`, in `resolve_type`.) I'm unsure the full ramifications of the change, but from what it looks like, it shouldn't impact anything major. Likewise, references to generic type parameters also get the `&'a [mut]` linked to the new page. cc @rust-lang/docs: Is this sufficient information? The listing of trait impls kinda feels redundant (especially if we can get the automated impl listing sorted again), but i still think it's useful to point out that you can use these in a generic context. Fixes #15654
☔ The latest upstream changes (presumably #43560) made this pull request unmergeable. Please resolve the merge conflicts. |
e04c762
to
71751db
Compare
@bors r=steveklabnik |
📌 Commit 71751db has been approved by |
add documentation for function pointers as a primitive This PR adds a new kind of primitive to the standard library documentation: Function pointers. It's useful to be able to discuss them separately from closure-trait-objects, and to have something to point to when discussing function pointers as a *type* and not a *trait*. Fixes #17104
☀️ Test successful - status-appveyor, status-travis |
This PR adds a new kind of primitive to the standard library documentation: Function pointers. It's useful to be able to discuss them separately from closure-trait-objects, and to have something to point to when discussing function pointers as a type and not a trait.
Fixes #17104