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.

Sign up for GitHub

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

Jump to bottom

Tracking Issue for const_cstr_methods #101719

Closed
3 tasks done
WaffleLapkin opened this issue Sep 12, 2022 · 6 comments
Closed
3 tasks done

Tracking Issue for const_cstr_methods #101719

WaffleLapkin opened this issue Sep 12, 2022 · 6 comments
Labels
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.

Comments

@WaffleLapkin
Copy link
Member

WaffleLapkin commented Sep 12, 2022 •
edited
Loading

Feature gate: #![feature(const_cstr_methods)]

This is a tracking issue for CStr methods as const functions.

Public API

// core::ffi

impl CStr {
    // feature(const_cstr_methods)
    pub const fn from_bytes_with_nul(bytes: &[u8]) -> Result<&Self, FromBytesWithNulError>;
    pub const fn to_bytes(&self) -> &[u8];
    pub const fn to_bytes_with_nul(&self) -> &[u8];
    pub const fn to_str(&self) -> Result<&str, str::Utf8Error>;
}

// Note that those methods are stable, they are only unstable as `const` fn

Steps / History

Unresolved Questions

  • None yet.

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html ↩
@WaffleLapkin WaffleLapkin 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 Sep 12, 2022
@reitermarkus reitermarkus mentioned this issue Oct 12, 2022
Merged
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Oct 28, 2022
@Dylan-DPC
Rollup merge of rust-lang#102961 - reitermarkus:const-cstr-from-ptr, …
6db281a 
…r=oli-obk

Make `CStr::from_ptr` `const`.

Should be included in rust-lang#101719.

cc `@WaffleLapkin`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Oct 28, 2022
@Dylan-DPC
Rollup merge of rust-lang#102961 - reitermarkus:const-cstr-from-ptr, …
db43536 
…r=oli-obk

Make `CStr::from_ptr` `const`.

Should be included in rust-lang#101719.

cc ``@WaffleLapkin``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 29, 2022
@matthiaskrgr
Rollup merge of rust-lang#102961 - reitermarkus:const-cstr-from-ptr, …
5b45bfe 
…r=oli-obk

Make `CStr::from_ptr` `const`.

Should be included in rust-lang#101719.

cc `@WaffleLapkin`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 29, 2022
@matthiaskrgr
Rollup merge of rust-lang#102961 - reitermarkus:const-cstr-from-ptr, …
b3ca68f 
…r=oli-obk

Make `CStr::from_ptr` `const`.

Should be included in rust-lang#101719.

cc ``@WaffleLapkin``
@chrysn chrysn mentioned this issue Jan 25, 2023
Merged
@tgross35 tgross35 mentioned this issue Feb 1, 2023
Merged
@tgross35
Copy link
Contributor

tgross35 commented Feb 1, 2023

@WaffleLapkin are any of these uncontroversial enough that you'd be comfortable const stabilizing them?

For #107429 I had to mark some of the memchr functions const stable, and the methods here largely have the same const dependencies. So, at least a subset of these could probably go const stable in the same release.

@WaffleLapkin
Copy link
Member Author

@tgross35 I personally think that all of these are uncontroversial and I'd be very comfortable stabilizing them, however I do not posses the power to do so. Stabilizing library APIs is a prerogative of T-libs-api.

@tgross35
Copy link
Contributor

tgross35 commented Feb 2, 2023

Rightieo, I just was wondering if you had any bad feeling that any of them would be unmaintainable as const.

I think the FCP could take place on the PR since this is quite small, similarly to #107429. Would you like to make the stabilization PR? If not, I don't mind doing it tomorrow. (in either case, using my commit 877e9f5 as a base is likely a good idea to avoid a smidge of rebasing once #107429 is in)

@WaffleLapkin
Copy link
Member Author

I'll live the PR to you :)

@tgross35 tgross35 mentioned this issue Feb 3, 2023
Merged
@tgross35
Copy link
Contributor

tgross35 commented Feb 3, 2023

Alright, I've opened #107624

thomcc pushed a commit to tcdi/postgrestd that referenced this issue Feb 10, 2023
@matthiaskrgr
Rollup merge of #102961 - reitermarkus:const-cstr-from-ptr, r=oli-obk
478f17e 
Make `CStr::from_ptr` `const`.

Should be included in rust-lang/rust#101719.

cc ``@WaffleLapkin``
compiler-errors added a commit to compiler-errors/rust that referenced this issue Jun 30, 2023
@compiler-errors
Rollup merge of rust-lang#107624 - tgross35:const-cstr-methods, r=dto…
8d15710 
…lnay

Stabilize `const_cstr_methods`

This PR seeks to stabilize `const_cstr_methods`. Fixes most of rust-lang#101719

## New const stable API

```rust
impl CStr {
    // depends: memchr
    pub const fn from_bytes_with_nul(bytes: &[u8]) -> Result<&Self, FromBytesWithNulError> {...}
    // depends: const_slice_index
    pub const fn to_bytes(&self) -> &[u8] {}
    // depends: pointer casts
    pub const fn to_bytes_with_nul(&self) -> &[u8] {}
    // depends: str::from_utf8
    pub const fn to_str(&self) -> Result<&str, str::Utf8Error> {}
}
```

I don't think any of these methods will have any issue when `CStr` becomes a thin pointer as long as `memchr` is const  (which also allows for const `strlen`) .

## Notes

- `from_bytes_until_nul` relies on `const_slice_index`, which relies on `const_trait_impls`, and generally this should be avoided. After talking with Oli, it should be OK in this case because we could replace the ranges with pointer tricks if needed (worst case being those feature gates disappear). rust-lang#107624 (comment)
- Making `from_ptr` const is deferred because it depends on `const_eval_select`. I have moved this under the new flag `const_cstr_from_ptr` rust-lang#107624 (comment)

cc `@oli-obk` I think you're the const expert

`@rustbot` modify labels: +T-libs-api +needs-fcp
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 30, 2023
@matthiaskrgr
Rollup merge of rust-lang#107624 - tgross35:const-cstr-methods, r=dto…
016c306 
…lnay

Stabilize `const_cstr_methods`

This PR seeks to stabilize `const_cstr_methods`. Fixes most of rust-lang#101719

## New const stable API

```rust
impl CStr {
    // depends: memchr
    pub const fn from_bytes_with_nul(bytes: &[u8]) -> Result<&Self, FromBytesWithNulError> {...}
    // depends: const_slice_index
    pub const fn to_bytes(&self) -> &[u8] {}
    // depends: pointer casts
    pub const fn to_bytes_with_nul(&self) -> &[u8] {}
    // depends: str::from_utf8
    pub const fn to_str(&self) -> Result<&str, str::Utf8Error> {}
}
```

I don't think any of these methods will have any issue when `CStr` becomes a thin pointer as long as `memchr` is const  (which also allows for const `strlen`) .

## Notes

- `from_bytes_until_nul` relies on `const_slice_index`, which relies on `const_trait_impls`, and generally this should be avoided. After talking with Oli, it should be OK in this case because we could replace the ranges with pointer tricks if needed (worst case being those feature gates disappear). rust-lang#107624 (comment)
- Making `from_ptr` const is deferred because it depends on `const_eval_select`. I have moved this under the new flag `const_cstr_from_ptr` rust-lang#107624 (comment)

cc ``@oli-obk`` I think you're the const expert

``@rustbot`` modify labels: +T-libs-api +needs-fcp
@tgross35
Copy link
Contributor

tgross35 commented Jul 1, 2023

@WaffleLapkin I think you can close this tracking issue if you would like, since #107624 merged. CStr::from_ptr was under this gate (I don't see it in this description) but moved to const_cstr_from_ptr with the merge, and I opened a new issue to track that #113219

tgross35 added a commit to tgross35/rust that referenced this issue Jul 1, 2023
@tgross35
Update the tracking issue for const_cstr_from_ptr
5ed429f 
Tracking issue rust-lang#101719 was for `const_cstr_methods`, rust-lang#113219 is a new
issue specific for `const_cstr_from_ptr`.
@tgross35 tgross35 mentioned this issue Jul 1, 2023
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 22, 2023
@matthiaskrgr
Rollup merge of rust-lang#113252 - tgross35:const-cstr-from-ptr-track…
bd4c7ab 
…ing-issue, r=ChrisDenton

Update the tracking issue for `const_cstr_from_ptr`

Tracking issue rust-lang#101719 was for `const_cstr_methods`, rust-lang#113219 is a new issue specific for `const_cstr_from_ptr`.

(I believe rust-lang#101719 could also be closed)

`@rustbot` label +T-libs-api +A-docs
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 22, 2023
@matthiaskrgr
Rollup merge of rust-lang#113252 - tgross35:const-cstr-from-ptr-track…
a252840 
…ing-issue, r=ChrisDenton

Update the tracking issue for `const_cstr_from_ptr`

Tracking issue rust-lang#101719 was for `const_cstr_methods`, rust-lang#113219 is a new issue specific for `const_cstr_from_ptr`.

(I believe rust-lang#101719 could also be closed)

``@rustbot`` label +T-libs-api +A-docs
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 22, 2023
@matthiaskrgr
Rollup merge of rust-lang#113252 - tgross35:const-cstr-from-ptr-track…
58a4be1 
…ing-issue, r=ChrisDenton

Update the tracking issue for `const_cstr_from_ptr`

Tracking issue rust-lang#101719 was for `const_cstr_methods`, rust-lang#113219 is a new issue specific for `const_cstr_from_ptr`.

(I believe rust-lang#101719 could also be closed)

```@rustbot``` label +T-libs-api +A-docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
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.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@tgross35 @WaffleLapkin