Skip to content
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

Merged
merged 1 commit into from
Aug 1, 2017

Conversation

QuietMisdreavus
Copy link
Member

@QuietMisdreavus QuietMisdreavus commented Jul 28, 2017

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

@rust-highfive
Copy link
Collaborator

r? @aturon

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

@QuietMisdreavus QuietMisdreavus changed the title add documentation for function pointers as a primitive [WIP] add documentation for function pointers as a primitive Jul 28, 2017
@QuietMisdreavus
Copy link
Member Author

Thanks to #17104 (comment) i have some impls to try to load, so i'll mark this WIP until i get that done.

/// assert_eq!(clos(5), 10);
/// ```
///
/// All function pointers implement [`Copy`], as well as [`Fn`]/[`FnMut`]/[`FnOnce`] for their
Copy link
Contributor

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.

Copy link
Member Author

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
Copy link
Contributor

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.

@QuietMisdreavus QuietMisdreavus changed the title [WIP] add documentation for function pointers as a primitive add documentation for function pointers as a primitive Jul 29, 2017
@QuietMisdreavus
Copy link
Member Author

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.

/// ```
///
/// 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
Copy link
Member

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.".

///
/// 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
Copy link
Member

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..

/// 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
Copy link
Member

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.

@ollie27
Copy link
Member

ollie27 commented Jul 29, 2017

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)

@QuietMisdreavus
Copy link
Member Author

@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.

@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 31, 2017
@steveklabnik
Copy link
Member

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.

@QuietMisdreavus
Copy link
Member Author

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.

bors added a commit that referenced this pull request Aug 1, 2017
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
@bors
Copy link
Contributor

bors commented Aug 1, 2017

☔ The latest upstream changes (presumably #43560) made this pull request unmergeable. Please resolve the merge conflicts.

@QuietMisdreavus
Copy link
Member Author

@bors r=steveklabnik

@bors
Copy link
Contributor

bors commented Aug 1, 2017

📌 Commit 71751db has been approved by steveklabnik

@bors
Copy link
Contributor

bors commented Aug 1, 2017

⌛ Testing commit 71751db with merge dd53dd5...

bors added a commit that referenced this pull request Aug 1, 2017
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
@bors
Copy link
Contributor

bors commented Aug 1, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: steveklabnik
Pushing dd53dd5 to master...

@bors bors merged commit 71751db into rust-lang:master Aug 1, 2017
@QuietMisdreavus QuietMisdreavus deleted the fn-docs branch August 1, 2017 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants