-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Added traits implemented by FnPtr to fn docs with example function #112106
base: master
Are you sure you want to change the base?
Conversation
r? @thomcc (rustbot has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
library/core/src/primitive_docs.rs
Outdated
@@ -1564,6 +1564,36 @@ mod prim_ref {} | |||
/// | |||
/// In addition, all *safe* function pointers implement [`Fn`], [`FnMut`], and [`FnOnce`], because | |||
/// these traits are specially known to the compiler. | |||
/// | |||
/// All function pointers implement the trait [`FnPtr`], which implements the following 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.
I'm not sure we should mention FnPtr
as it's currently unstable.
library/core/src/primitive_docs.rs
Outdated
/// | ||
/// All function pointers implement the trait [`FnPtr`], which implements the following traits: | ||
/// | ||
/// * [`ConstParamTy`] |
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.
We should probably leave the unstable traits out of this list, no?
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.
I think the only stable trait that is added is Sized
, should I just add that to the list of traits on line 1552 and not mention anything about FnPtr
?
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.
and then should I delete the dummy impl for FnPtr
and add one for Sized
?
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.
hey @thomcc, just following up on this
@rustbot author |
@mj10021 any updates on this? |
ad1d772
to
35c9608
Compare
Hi! Sorry it took me a bit to respond. I just removed the references to unstable features and pushed the update |
f788cf6
to
fb52b28
Compare
@rustbot review |
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.
other than the phrasing of the docs, it looks great! thanks!
library/core/src/primitive_docs.rs
Outdated
reason = "internal trait for implementing various traits for all function pointers" | ||
)] | ||
#[doc(fake_variadic)] | ||
/// This trait is implemented on function pointers with any number of arguments. |
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.
function pointers with any number of arguments
is probably better phrased as "all function pointers" for consistency and simplicity's sake
library/std/src/primitive_docs.rs
Outdated
reason = "internal trait for implementing various traits for all function pointers" | ||
)] | ||
#[doc(fake_variadic)] | ||
/// This trait is implemented on function pointers with any number of arguments. |
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.
same applies here
@rustbot label -S-waiting-on-review +S-waiting-on-author |
☔ The latest upstream changes (presumably #115940) made this pull request unmergeable. Please resolve the merge conflicts. |
4c48923
to
5220231
Compare
I'm going to be away for a few months, so I'm rerolling my PRs so that folks don't have to wait for me. Sorry/thanks. r? libs |
☔ The latest upstream changes (presumably #120558) made this pull request unmergeable. Please resolve the merge conflicts. |
Sorry to bounce it back yet again, but can you resolve the conflicts? @rustbot author |
5220231
to
343f955
Compare
This comment has been minimized.
This comment has been minimized.
343f955
to
ed59fd9
Compare
The job Click to see the possible cause of the failure (guessed by this bot)
|
I'm not exactly sure how to work around that error, should the FnPtr be implemented differently? iirc this is a new error |
Sorry, I don't know either! |
@GuillaumeGomez This PR tries to make trait impls appear in rustdoc for function pointer types. Do you have a better idea on how we can make this work? Or some suggestions? |
It seems to be a limitation coming from how rustdoc uses the compiler. We might need to get help from the compiler team here to figure how to solve this issue. Maybe you might have an idea @fmease ? |
@rustbot blocked |
☔ The latest upstream changes (presumably #123065) made this pull request unmergeable. Please resolve the merge conflicts. |
Sorry for not getting back to you. I've discussed this with Guillaume and a T-compiler member a while ago. This error is intentional to uphold some soundness invariants of the trait solver and caused by the While we could theoretically turn it into a An alternative to that would be patching the trait solver(s) to skip the I'm not super happy about any of these options (there are more sophisticated ones like introducing more internal attributes etc.). |
That seems easiest to me 🤔 |
As noted in #111182, the fn-ptr docs were not updated to include the FnPtr trait, so I added the implemented traits with an example fn