Expand docs for TryFrom and TryInto.
#58015
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (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. |
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
ping from triage @icefoxen you need to resolve the failing tests. |
|
Thanks @dpc I will gnaw on them when I get a chance. |
|
Wrong dpc. @Dylan-DPC :) . But I like what's going on here, too. 🎉 |
|
There's too many dpc's around! |
|
Ok it's time to finish pushing this through. It currently conflicts with #58302 but the actual differences are trivial. If that one gets accepted I'll update the docs in this PR to match what we need. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Thank you for helping push this forward! (I’ve edited the PR description, |
|
@SimonSapin Thanks! I knew about that behavior but am never 100% sure what does and does not trigger it. Alas. |
|
It’s documented in https://help.github.com/articles/closing-issues-using-keywords/. And, since… more recently than the behavior has existed, those keywords have a dotted underline and an explanation tooltip when they are interpreted specially. See for example #58407. |
icefoxen
changed the title
TryFrom and TryInto.TryFrom and TryInto.
|
Ok. Docs done, tidy has no complaints, doctests run successfully. r? |
|
Ping from triage, @sfackler :) |
|
r? @SimonSapin |
|
☔ The latest upstream changes (presumably #58302) made this pull request unmergeable. Please resolve the merge conflicts. |
src/libcore/convert.rs
Outdated
| /// - `TryFrom<T> for U` implies [`TryInto<U>`]` for T` | ||
| /// - [`try_from`] is reflexive, which means that `TryFrom<T> for T` | ||
| /// is implemented and cannot fail -- the associated `Error` type for | ||
| /// calling `T::try_from()` on a value of type `T` is `!`. |
There was a problem hiding this comment.
As of #58302 this error type is Infallible (a new empty enum), but we plan to make it ! when the ! type is stabilized and make Infallible a type alias.
Could you add a sentence here?
src/libcore/num/mod.rs
Outdated
| @@ -4449,6 +4449,9 @@ macro_rules! try_from_unbounded { | |||
| impl TryFrom<$source> for $target { | |||
| type Error = TryFromIntError; | |||
|
|
|||
| /// Try to create the target type from the source type. | |||
| /// This particular variant will never fail, but is included | |||
| /// for completeness's sake. | |||
There was a problem hiding this comment.
For some TryFrom conversions that involve usize or isize, details of how the conversion is implemented depend on the target platform (’s pointer size). But we typically only generate one set of documentation for the standard library, so I think we shouldn’t document details that only apply to some platforms.
The easiest way would be to have the same doc-comment for all impl blocks that use TryFromIntError.
|
Made the changes you asked for, or at least tried to... it's late and I'm a little loopy, but I don't want you to have to wait on this. |
|
This is better, thanks. I think the word “disjoint” is not what you meant here: two ranges are disjoint if they do not overlap at all: if their intersection is empty. (And failed conversions returns
|
|
Ahahaha, wow I was really loopy. Should now be much better, thank you. Do I have to rebase or anything to handle the merge conflict? My git-fu is, as many will tell you, not great. |
The examples are still lacking for now, both for module docs and for methods/impl's.
They're not as good as `From` 'cause they don't stringify the types and generate examples and so on, but it's a start.
Expand docs for `TryFrom` and `TryInto`. The examples are still lacking for now, both for module docs and for methods/impl's. Will be adding those in further pushes. Should hopefully resolve the doc concern in #33417 when finished?
|
☀️ Test successful - checks-travis, status-appveyor |
Centril
added
the
T-release
|
@rust-lang/libs Can you decide if you're fine with this being backported to beta? We need a decision in the next several days so it can make it in time. |
| @@ -380,7 +386,55 @@ pub trait TryInto<T>: Sized { | |||
| fn try_into(self) -> Result<T, Self::Error>; | |||
| } | |||
|
|
|||
| /// Attempt to construct `Self` via a conversion. | |||
| /// Simple and safe type conversions that may fail in a controlled | |||
| /// way under some circumstances. It is the reciprocal of [`TryInto`]. | |||
There was a problem hiding this comment.
This is a bit too much information for a summary line; they should be quite short. That said, this PR has already been merged; @icefoxen any interest in a follow-on?
There was a problem hiding this comment.
Not this time I'm afraid. As suggested by how long it took me to notice this, life is pretty hectic at the moment. 😞
|
I'm okay with this being backported. Ther'es one thing that's not ideal about it but it's not a dealbreaker. :) |
alexcrichton
added
beta-accepted
|
The libs team discussed this today and decided it's ok to backport to beta |
|
@alexcrichton Btw, when you |
[beta] Rollup backports Cherry-picked: * #58021: Fix fallout from #57667 * #59599: Updated RELEASES.md for 1.34.0 * #59587: Remove #[doc(hidden)] from Error::type_id * #58994: Hide deprecation warnings inside derive expansions * #58015: Expand docs for `TryFrom` and `TryInto`. * #59770: ci: pin android emulator to 28.0.23 * #59704: ci: Update FreeBSD tarball downloads * #59257: Update CI configuration for building Redox libraries * #59724: Function arguments should never get promoted r? @ghost
[beta] Rollup backports Cherry-picked: * #58021: Fix fallout from #57667 * #59599: Updated RELEASES.md for 1.34.0 * #59587: Remove #[doc(hidden)] from Error::type_id * #58994: Hide deprecation warnings inside derive expansions * #58015: Expand docs for `TryFrom` and `TryInto`. * #59770: ci: pin android emulator to 28.0.23 * #59704: ci: Update FreeBSD tarball downloads * #59257: Update CI configuration for building Redox libraries * #59724: Function arguments should never get promoted * #59499: Fix broken download link in the armhf-gnu image * #58330: Add rustdoc JS non-std tests * #58848: Prevent cache issues on version updates r? @ghost
The examples are still lacking for now, both for module docs and for methods/impl's. Will be adding those in further pushes.
Should hopefully resolve the doc concern in #33417 when finished?