-
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
Clarify undefined behaviour in binary heap, btree and hashset docs #87537
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
I don't think this makes it more clear, quite the opposite, at first glance I thought the "behaviour" you added was referring to the "undefined behaviour". You should also change the docs for |
How about wording like this? “The behavior resulting from such a logic error is not specified (it could include panics, incorrect results, aborts, memory leaks, or non-termination) but will not be undefined behavior.” (I think it would also be good to link “undefined behavior” to some page explaining what undefined behavior is, since this might be the first time someone has encountered the concept, and it might not be obvious that “undefined behavior” is a specific formal term.) |
Agreed that this seems like a good clarification. It may make sense to extract the shared language somewhere in the root of the library docs as a general statement we can link to in terms of what logic errors can do; that way we can avoid updating N places each time and potentially expand a little on the paragraph. std's top level doc comment seems like a reasonable place to add that section, I guess. |
Ping from triage: |
86ca0af
to
79c1f8a
Compare
OK, I've updated the docs in all four places using the wording suggested by @kpreid :). The patch isn't identical in all those files as they've used different line lengths, so I followed the convention in each file. |
☔ The latest upstream changes (presumably #89262) made this pull request unmergeable. Please resolve the merge conflicts. |
Previously, it wasn't clear whether "This could include" was referring to logic errors, or undefined behaviour. Tweak wording to clarify this sentence does not relate to UB.
79c1f8a
to
04c1ec5
Compare
@bors r+ rollup Rebased and approved, thanks! |
📌 Commit 04c1ec5 has been approved by |
…rk-Simulacrum Clarify undefined behaviour in binary heap, btree and hashset docs Previously, it wasn't clear whether "This could include" was referring to logic errors, or undefined behaviour. Tweak wording to clarify this sentence does not relate to UB.
…rk-Simulacrum Clarify undefined behaviour in binary heap, btree and hashset docs Previously, it wasn't clear whether "This could include" was referring to logic errors, or undefined behaviour. Tweak wording to clarify this sentence does not relate to UB.
…rk-Simulacrum Clarify undefined behaviour in binary heap, btree and hashset docs Previously, it wasn't clear whether "This could include" was referring to logic errors, or undefined behaviour. Tweak wording to clarify this sentence does not relate to UB.
…rk-Simulacrum Clarify undefined behaviour in binary heap, btree and hashset docs Previously, it wasn't clear whether "This could include" was referring to logic errors, or undefined behaviour. Tweak wording to clarify this sentence does not relate to UB.
Rollup of 14 pull requests Successful merges: - rust-lang#87537 (Clarify undefined behaviour in binary heap, btree and hashset docs) - rust-lang#88624 (Stabilize feature `saturating_div` for rust 1.58.0) - rust-lang#89257 (Give better error for `macro_rules name`) - rust-lang#89665 (Ensure that pushing empty path works as before on verbatim paths) - rust-lang#89895 (Don't mark for loop iter expression as desugared) - rust-lang#89922 (Update E0637 description to mention `&` w/o an explicit lifetime name) - rust-lang#89944 (Change `Duration::[try_]from_secs_{f32, f64}` underflow error) - rust-lang#89991 (rustc_ast: Turn `MutVisitor::token_visiting_enabled` into a constant) - rust-lang#90028 (Reject closures in patterns) - rust-lang#90069 (Fix const qualification when executed after promotion) - rust-lang#90078 (Add a regression test for issue-83479) - rust-lang#90114 (Add some tests for const_generics_defaults) - rust-lang#90115 (Add test for issue rust-lang#78561) - rust-lang#90129 (triagebot: Treat `I-*nominated` like `I-nominated`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Previously, it wasn't clear whether "This could include" was referring to logic errors, or undefined behaviour. Tweak wording to clarify this sentence does not relate to UB.