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

Tracking Issue for cstr_count_bytes #114441

Closed
3 of 4 tasks
tgross35 opened this issue Aug 3, 2023 · 5 comments · Fixed by #123661
Closed
3 of 4 tasks

Tracking Issue for cstr_count_bytes #114441

tgross35 opened this issue Aug 3, 2023 · 5 comments · Fixed by #123661
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@tgross35
Copy link
Contributor

tgross35 commented Aug 3, 2023

Feature gate: #![feature(cstr_count_bytes)]

This is a tracking issue for adding a .count_bytes() method to CStr.

Public API

impl CStr {
    pub fn count_bytes(&self) -> usize;
}

Steps / History

Unresolved Questions

  • None yet.

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html

@tgross35 tgross35 added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Aug 3, 2023
@tgross35 tgross35 changed the title Tracking Issue for cstr_len Tracking Issue for cstr_count_bytes Aug 29, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 20, 2023
Implement `cstr_count_bytes`

This has not yet been approved via ACP, but it's simple enough to get started on.

- ACP: rust-lang/libs-team#256
- Tracking issue: rust-lang#114441

`@rustbot` label +T-libs-api
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Sep 21, 2023
Implement `cstr_count_bytes`

This has not yet been approved via ACP, but it's simple enough to get started on.

- ACP: rust-lang/libs-team#256
- Tracking issue: rust-lang/rust#114441

`@rustbot` label +T-libs-api
@dtolnay dtolnay added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Mar 24, 2024
@dtolnay
Copy link
Member

dtolnay commented Mar 24, 2024

@rust-lang/libs-api:
@rfcbot fcp merge

I propose stabilizing this signature, including the constness:

impl CStr {
    pub const fn count_bytes(&self) -> usize;
}

Here is the comment where our team made the suggestion to expose this under the name .count_bytes(): rust-lang/libs-team#256 (comment)

New developments since then: the CStr::bytes iterator was added just last week in #104353. With that, an alternative spelling to evaluate would be .bytes().count() using the Iterator trait's count (possibly with a non-default implementation specific to this impl for performance). But this couldn't be const until perhaps the distant future, and wouldn't be straightforwardly locatable in the method list of CStr or in autocomplete..

This method comes with a doc alias that makes it show up in rustdoc search for strlen.

The constness of this signature has so far been tracked separately under #113219, and involves a T-lang matter. I have proposed an FCP in that issue in parallel to this one that would relinquish the constness of count_bytes into a T-libs-api decision only. If both FCPs go through, we stabilize const fn count_bytes. If only the FCP here goes through and not the one in #113219, then we stabilize fn count_bytes and the constness will be left unstable for now.

@rfcbot
Copy link

rfcbot commented Mar 24, 2024

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Mar 24, 2024
@rfcbot
Copy link

rfcbot commented Mar 31, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Mar 31, 2024
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
Implement `cstr_count_bytes`

This has not yet been approved via ACP, but it's simple enough to get started on.

- ACP: rust-lang/libs-team#256
- Tracking issue: rust-lang/rust#114441

`@rustbot` label +T-libs-api
tgross35 added a commit to tgross35/rust that referenced this issue Apr 9, 2024
Newly stable API:

```rust
impl CStr {
    pub fn count_bytes(&self) -> usize;
}
```

Const stabilization has not yet been decided, so that will continue to be
gated under <rust-lang#113219>.

Fixes: <rust-lang#114441>
@tgross35
Copy link
Contributor Author

tgross35 commented Apr 9, 2024

Since FCP here is nearly done but there are quite a few checkboxes left at #113219, I opened #123661 to stabilize count_bytes without const. When #113219 FCP completes, we can const stabilize both.

Thanks @dtolnay for moving this forward!

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Apr 10, 2024
@rfcbot
Copy link

rfcbot commented Apr 10, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Apr 10, 2024
fmease added a commit to fmease/rust that referenced this issue Apr 10, 2024
…s, r=dtolnay

Stabilize `cstr_count_bytes`

Newly stable API:

```rust
impl CStr {
    pub fn count_bytes(&self) -> usize;
}
```

Const stabilization has not yet been decided, so that will continue to be gated under <rust-lang#113219>.

FCP finished at rust-lang#114441 (comment).

Fixes: <rust-lang#114441>
fmease added a commit to fmease/rust that referenced this issue Apr 10, 2024
…s, r=dtolnay

Stabilize `cstr_count_bytes`

Newly stable API:

```rust
impl CStr {
    pub fn count_bytes(&self) -> usize;
}
```

Const stabilization has not yet been decided, so that will continue to be gated under <rust-lang#113219>.

FCP finished at rust-lang#114441 (comment).

Fixes: <rust-lang#114441>
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 11, 2024
Rollup merge of rust-lang#123661 - tgross35:stabilize-cstr_count_bytes, r=dtolnay

Stabilize `cstr_count_bytes`

Newly stable API:

```rust
impl CStr {
    pub fn count_bytes(&self) -> usize;
}
```

Const stabilization has not yet been decided, so that will continue to be gated under <rust-lang#113219>.

FCP finished at rust-lang#114441 (comment).

Fixes: <rust-lang#114441>
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Apr 11, 2024
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Implement `cstr_count_bytes`

This has not yet been approved via ACP, but it's simple enough to get started on.

- ACP: rust-lang/libs-team#256
- Tracking issue: rust-lang/rust#114441

`@rustbot` label +T-libs-api
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 12, 2024
Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes)

Completed the pair of FCPs rust-lang#113219 (comment) + rust-lang#114441 (comment).

`CStr::from_ptr` is covered by just the first FCP on its own. `CStr::count_bytes` requires the approval of both FCPs. The second paragraph of the first link and the last paragraph of the second link explain the relationship between the two FCPs. As both have been approved, we can proceed with stabilizing `const` on both of these already-stable functions.
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jul 12, 2024
Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes)

Completed the pair of FCPs rust-lang#113219 (comment) + rust-lang#114441 (comment).

`CStr::from_ptr` is covered by just the first FCP on its own. `CStr::count_bytes` requires the approval of both FCPs. The second paragraph of the first link and the last paragraph of the second link explain the relationship between the two FCPs. As both have been approved, we can proceed with stabilizing `const` on both of these already-stable functions.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 12, 2024
Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes)

Completed the pair of FCPs rust-lang#113219 (comment) + rust-lang#114441 (comment).

`CStr::from_ptr` is covered by just the first FCP on its own. `CStr::count_bytes` requires the approval of both FCPs. The second paragraph of the first link and the last paragraph of the second link explain the relationship between the two FCPs. As both have been approved, we can proceed with stabilizing `const` on both of these already-stable functions.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 12, 2024
Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes)

Completed the pair of FCPs rust-lang#113219 (comment) + rust-lang#114441 (comment).

`CStr::from_ptr` is covered by just the first FCP on its own. `CStr::count_bytes` requires the approval of both FCPs. The second paragraph of the first link and the last paragraph of the second link explain the relationship between the two FCPs. As both have been approved, we can proceed with stabilizing `const` on both of these already-stable functions.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 12, 2024
Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes)

Completed the pair of FCPs rust-lang#113219 (comment) + rust-lang#114441 (comment).

`CStr::from_ptr` is covered by just the first FCP on its own. `CStr::count_bytes` requires the approval of both FCPs. The second paragraph of the first link and the last paragraph of the second link explain the relationship between the two FCPs. As both have been approved, we can proceed with stabilizing `const` on both of these already-stable functions.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 12, 2024
Rollup merge of rust-lang#127433 - dtolnay:conststrlen, r=workingjubilee

Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes)

Completed the pair of FCPs rust-lang#113219 (comment) + rust-lang#114441 (comment).

`CStr::from_ptr` is covered by just the first FCP on its own. `CStr::count_bytes` requires the approval of both FCPs. The second paragraph of the first link and the last paragraph of the second link explain the relationship between the two FCPs. As both have been approved, we can proceed with stabilizing `const` on both of these already-stable functions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. 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