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

“may not” was incorrectly changed to “must not” in FromResidual documentation #123566

Closed
taylordotfish opened this issue Apr 6, 2024 · 2 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@taylordotfish
Copy link

Location

Documentation for FromResidual::from_residual

Summary

As part of #87445, the following part of the documentation for FromResidual::from_residual:

/// This should be implemented consistently with the `branch` method such
/// that applying the `?` operator will get back an equivalent residual:
/// `FromResidual::from_residual(r).branch() --> ControlFlow::Break(r)`.
/// (It may not be an *identical* residual when interconversion is involved.)

was changed to (change in bold):

/// This should be implemented consistently with the `branch` method such
/// that applying the `?` operator will get back an equivalent residual:
/// `FromResidual::from_residual(r).branch() --> ControlFlow::Break(r)`.
/// (It must not be an *identical* residual when interconversion is involved.)

The parenthetical now appears to state that the residual is not permitted to be identical when interconversion is involved (i.e., it must be different in some way). However, I believe the intention of the original wording was to convey that the residual is not required to be identical when interconversion is involved, which makes more sense contextually. I believe the emphasis on the word “identical” also supports this interpretation.

I propose that “must not” be changed to “need not”, “is not required to”, “does not have to”, or similar. Even “might not”, the other alternative in #87445, would be closer.

@taylordotfish taylordotfish added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Apr 6, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 6, 2024
@jieyouxu jieyouxu added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-bug Category: This is a bug. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 6, 2024
@taylordotfish
Copy link
Author

It looks like #123595 was created in response to this issue, but doesn't mention or link to it, which is odd considering its description contains verbatim parts of this issue's description without citation.

@GrigorenkoPV
Copy link
Contributor

I guess this issue can be closed now that the (impolite) PR got merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants