Skip to content

Conversation

@tkr-sh
Copy link
Contributor

@tkr-sh tkr-sh commented May 13, 2025

Some methods that were taking some range in parameter were a bit inconsistent / unclear in the panic documentation.

Here is the recap:

  • Replaced "start/end point" by "start/end bound" to be coherent with RangeBounds naming (it's also easier to understand I think)
  • Previously, it was written "[...] or if the end point is greater than the length of [...]", but this is not entirely true! Actually, you can have a start bound that is greater than the length, with an end bound that is unbounded and it will also panic. Therefore I think that "[...] one of the range bound is bounded and greater than the length of [...]" is better!
  • String methods weren't mentionning that the method panics if start_bound > end_bound but it actually does! It uses slice::range which panics when start > end. (https://doc.rust-lang.org/stable/src/alloc/string.rs.html#1932-1934, https://doc.rust-lang.org/stable/src/core/slice/index.rs.html#835-837).
    You can also test it with:
struct MyRange;
impl std::ops::RangeBounds<usize> for MyRange {
    fn start_bound(&self) -> std::ops::Bound<&usize> {
        std::ops::Bound::Included(&3usize)
    }
    fn end_bound(&self) -> std::ops::Bound<&usize> {
        std::ops::Bound::Included(&1usize)
    }
}

fn main() {
    let mut s = String::from("I love Rust!");
    s.drain(MyRange); // panics!
}

@rustbot
Copy link
Collaborator

rustbot commented May 13, 2025

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 13, 2025
@tkr-sh
Copy link
Contributor Author

tkr-sh commented May 13, 2025

@rustbot label +A-docs

@rustbot rustbot added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label May 13, 2025
@tkr-sh
Copy link
Contributor Author

tkr-sh commented May 27, 2025

r? @hkBst

@rustbot
Copy link
Collaborator

rustbot commented May 27, 2025

Failed to set assignee to hkBst: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

Copy link
Member

@hkBst hkBst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with this.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 15, 2025
@alex-semenyuk
Copy link
Member

@tkr-sh
Thanks for your contribution.
Form wg-triage. Any updates on this PR?

@rustbot

This comment has been minimized.

@tgross35
Copy link
Contributor

Please no emojis in commit messages

@tkr-sh
Copy link
Contributor Author

tkr-sh commented Aug 10, 2025

hello @alex-semenyuk !
I just commited changes that should make this PR approved I think.
Sorry for the delay btw.

@rustbot review

@rustbot
Copy link
Collaborator

rustbot commented Aug 10, 2025

Requested reviewer is already assigned to this pull request.

Please choose another assignee.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 10, 2025
@tkr-sh
Copy link
Contributor Author

tkr-sh commented Aug 10, 2025

@tgross35 is there a guideline in the rustc dev book that says to not do that ? If not, I don't see any reason why I should not do it.

@ibraheemdev
Copy link
Member

ibraheemdev commented Aug 18, 2025

Could you please squash the commits? This looks good after that's done.

@tgross35
Copy link
Contributor

@tgross35 is there a guideline in the rustc dev book that says to not do that ? If not, I don't see any reason why I should not do it.

Ideally everything like that would be documented, but we assume that if things aren't mentioned then existing conventions will be followed. This is where reviewers come in :)

@ibraheemdev ibraheemdev added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 19, 2025
@Dylan-DPC
Copy link
Member

@tkr-sh any updates on this? the only item pending is to squash the commits.

@rustbot

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Sep 20, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@tkr-sh
Copy link
Contributor Author

tkr-sh commented Sep 20, 2025

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 20, 2025
@ibraheemdev
Copy link
Member

Thanks. @bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 20, 2025

📌 Commit ac3c480 has been approved by ibraheemdev

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 20, 2025
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 20, 2025
Improve doc of some methods that take ranges

Some methods that were taking some range in parameter were a bit inconsistent / unclear in the panic documentation.

Here is the recap:
- Replaced "start/end point" by "start/end bound" to be coherent with [`RangeBounds`](https://doc.rust-lang.org/stable/std/ops/trait.RangeBounds.html) naming (it's also easier to understand I think)
- Previously, it was written "_[...] or if the end point is greater than the length of [...]_", but this is not entirely true! Actually, you can have a start bound that is greater than the length, with an end bound that is unbounded and it will also panic. Therefore I think that "_[...] one of the range bound is bounded and greater than the length of [...]_" is better!
- `String` methods weren't mentionning that the method panics if `start_bound > end_bound` but it actually does! It uses `slice::range` which panics when `start > end`.  (https://doc.rust-lang.org/stable/src/alloc/string.rs.html#1932-1934, https://doc.rust-lang.org/stable/src/core/slice/index.rs.html#835-837).
You can also test it with:
```rs
struct MyRange;
impl std::ops::RangeBounds<usize> for MyRange {
    fn start_bound(&self) -> std::ops::Bound<&usize> {
        std::ops::Bound::Included(&3usize)
    }
    fn end_bound(&self) -> std::ops::Bound<&usize> {
        std::ops::Bound::Included(&1usize)
    }
}

fn main() {
    let mut s = String::from("I love Rust!");
    s.drain(MyRange); // panics!
}
```
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 20, 2025
Improve doc of some methods that take ranges

Some methods that were taking some range in parameter were a bit inconsistent / unclear in the panic documentation.

Here is the recap:
- Replaced "start/end point" by "start/end bound" to be coherent with [`RangeBounds`](https://doc.rust-lang.org/stable/std/ops/trait.RangeBounds.html) naming (it's also easier to understand I think)
- Previously, it was written "_[...] or if the end point is greater than the length of [...]_", but this is not entirely true! Actually, you can have a start bound that is greater than the length, with an end bound that is unbounded and it will also panic. Therefore I think that "_[...] one of the range bound is bounded and greater than the length of [...]_" is better!
- `String` methods weren't mentionning that the method panics if `start_bound > end_bound` but it actually does! It uses `slice::range` which panics when `start > end`.  (https://doc.rust-lang.org/stable/src/alloc/string.rs.html#1932-1934, https://doc.rust-lang.org/stable/src/core/slice/index.rs.html#835-837).
You can also test it with:
```rs
struct MyRange;
impl std::ops::RangeBounds<usize> for MyRange {
    fn start_bound(&self) -> std::ops::Bound<&usize> {
        std::ops::Bound::Included(&3usize)
    }
    fn end_bound(&self) -> std::ops::Bound<&usize> {
        std::ops::Bound::Included(&1usize)
    }
}

fn main() {
    let mut s = String::from("I love Rust!");
    s.drain(MyRange); // panics!
}
```
bors added a commit that referenced this pull request Sep 20, 2025
Rollup of 6 pull requests

Successful merges:

 - #140983 (Improve doc of some methods that take ranges)
 - #144091 (Stabilize `new_zeroed_alloc`)
 - #145664 (Stabilize `std::panic::Location::file_as_c_str`)
 - #146744 (Deref related cleanups in ref_prop)
 - #146793 (naked_asm: emit a label starting with `func_end`)
 - #146822 (Fix old typo in lang_start_internal comment)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Sep 21, 2025
Rollup of 6 pull requests

Successful merges:

 - #140983 (Improve doc of some methods that take ranges)
 - #144091 (Stabilize `new_zeroed_alloc`)
 - #145664 (Stabilize `std::panic::Location::file_as_c_str`)
 - #146744 (Deref related cleanups in ref_prop)
 - #146793 (naked_asm: emit a label starting with `func_end`)
 - #146822 (Fix old typo in lang_start_internal comment)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Sep 21, 2025
Rollup of 8 pull requests

Successful merges:

 - #140983 (Improve doc of some methods that take ranges)
 - #144091 (Stabilize `new_zeroed_alloc`)
 - #145664 (Stabilize `std::panic::Location::file_as_c_str`)
 - #146551 (fix issue with `cmse-nonsecure-entry` ABI being both async and c-variadic)
 - #146744 (Deref related cleanups in ref_prop)
 - #146793 (naked_asm: emit a label starting with `func_end`)
 - #146820 (Add unstable attribute to BTreeMap-related allocator generics)
 - #146822 (Fix old typo in lang_start_internal comment)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d253318 into rust-lang:master Sep 21, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 21, 2025
rust-timer added a commit that referenced this pull request Sep 21, 2025
Rollup merge of #140983 - tkr-sh:master, r=ibraheemdev

Improve doc of some methods that take ranges

Some methods that were taking some range in parameter were a bit inconsistent / unclear in the panic documentation.

Here is the recap:
- Replaced "start/end point" by "start/end bound" to be coherent with [`RangeBounds`](https://doc.rust-lang.org/stable/std/ops/trait.RangeBounds.html) naming (it's also easier to understand I think)
- Previously, it was written "_[...] or if the end point is greater than the length of [...]_", but this is not entirely true! Actually, you can have a start bound that is greater than the length, with an end bound that is unbounded and it will also panic. Therefore I think that "_[...] one of the range bound is bounded and greater than the length of [...]_" is better!
- `String` methods weren't mentionning that the method panics if `start_bound > end_bound` but it actually does! It uses `slice::range` which panics when `start > end`.  (https://doc.rust-lang.org/stable/src/alloc/string.rs.html#1932-1934, https://doc.rust-lang.org/stable/src/core/slice/index.rs.html#835-837).
You can also test it with:
```rs
struct MyRange;
impl std::ops::RangeBounds<usize> for MyRange {
    fn start_bound(&self) -> std::ops::Bound<&usize> {
        std::ops::Bound::Included(&3usize)
    }
    fn end_bound(&self) -> std::ops::Bound<&usize> {
        std::ops::Bound::Included(&1usize)
    }
}

fn main() {
    let mut s = String::from("I love Rust!");
    s.drain(MyRange); // panics!
}
```
Muscraft pushed a commit to Muscraft/rust that referenced this pull request Sep 24, 2025
Improve doc of some methods that take ranges

Some methods that were taking some range in parameter were a bit inconsistent / unclear in the panic documentation.

Here is the recap:
- Replaced "start/end point" by "start/end bound" to be coherent with [`RangeBounds`](https://doc.rust-lang.org/stable/std/ops/trait.RangeBounds.html) naming (it's also easier to understand I think)
- Previously, it was written "_[...] or if the end point is greater than the length of [...]_", but this is not entirely true! Actually, you can have a start bound that is greater than the length, with an end bound that is unbounded and it will also panic. Therefore I think that "_[...] one of the range bound is bounded and greater than the length of [...]_" is better!
- `String` methods weren't mentionning that the method panics if `start_bound > end_bound` but it actually does! It uses `slice::range` which panics when `start > end`.  (https://doc.rust-lang.org/stable/src/alloc/string.rs.html#1932-1934, https://doc.rust-lang.org/stable/src/core/slice/index.rs.html#835-837).
You can also test it with:
```rs
struct MyRange;
impl std::ops::RangeBounds<usize> for MyRange {
    fn start_bound(&self) -> std::ops::Bound<&usize> {
        std::ops::Bound::Included(&3usize)
    }
    fn end_bound(&self) -> std::ops::Bound<&usize> {
        std::ops::Bound::Included(&1usize)
    }
}

fn main() {
    let mut s = String::from("I love Rust!");
    s.drain(MyRange); // panics!
}
```
Muscraft pushed a commit to Muscraft/rust that referenced this pull request Sep 24, 2025
Rollup of 8 pull requests

Successful merges:

 - rust-lang#140983 (Improve doc of some methods that take ranges)
 - rust-lang#144091 (Stabilize `new_zeroed_alloc`)
 - rust-lang#145664 (Stabilize `std::panic::Location::file_as_c_str`)
 - rust-lang#146551 (fix issue with `cmse-nonsecure-entry` ABI being both async and c-variadic)
 - rust-lang#146744 (Deref related cleanups in ref_prop)
 - rust-lang#146793 (naked_asm: emit a label starting with `func_end`)
 - rust-lang#146820 (Add unstable attribute to BTreeMap-related allocator generics)
 - rust-lang#146822 (Fix old typo in lang_start_internal comment)

r? `@ghost`
`@rustbot` modify labels: rollup
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Oct 9, 2025
Improve doc of some methods that take ranges

Some methods that were taking some range in parameter were a bit inconsistent / unclear in the panic documentation.

Here is the recap:
- Replaced "start/end point" by "start/end bound" to be coherent with [`RangeBounds`](https://doc.rust-lang.org/stable/std/ops/trait.RangeBounds.html) naming (it's also easier to understand I think)
- Previously, it was written "_[...] or if the end point is greater than the length of [...]_", but this is not entirely true! Actually, you can have a start bound that is greater than the length, with an end bound that is unbounded and it will also panic. Therefore I think that "_[...] one of the range bound is bounded and greater than the length of [...]_" is better!
- `String` methods weren't mentionning that the method panics if `start_bound > end_bound` but it actually does! It uses `slice::range` which panics when `start > end`.  (https://doc.rust-lang.org/stable/src/alloc/string.rs.html#1932-1934, https://doc.rust-lang.org/stable/src/core/slice/index.rs.html#835-837).
You can also test it with:
```rs
struct MyRange;
impl std::ops::RangeBounds<usize> for MyRange {
    fn start_bound(&self) -> std::ops::Bound<&usize> {
        std::ops::Bound::Included(&3usize)
    }
    fn end_bound(&self) -> std::ops::Bound<&usize> {
        std::ops::Bound::Included(&1usize)
    }
}

fn main() {
    let mut s = String::from("I love Rust!");
    s.drain(MyRange); // panics!
}
```
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 S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants