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

constify some CStr methods #100291

Merged
merged 1 commit into from
Sep 13, 2022
Merged

Conversation

WaffleLapkin
Copy link
Member

@WaffleLapkin WaffleLapkin commented Aug 8, 2022

This PR marks the following public APIs as const:

impl CStr {
    // feature(const_cstr_from_bytes)
    pub const fn from_bytes_until_nul(bytes: &[u8]) -> Result<&CStr, FromBytesUntilNulError>;
    pub const fn from_bytes_with_nul(bytes: &[u8]) -> Result<&Self, FromBytesWithNulError>;

    // feature(const_cstr_to_bytes)
    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>;
}

r? @oli-obk (use of const_eval_select :P )
cc @mina86 (you've asked for this <3 )

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Aug 8, 2022
@rust-highfive
Copy link
Collaborator

r? @scottmcm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 8, 2022
@WaffleLapkin
Copy link
Member Author

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned scottmcm Aug 8, 2022
@WaffleLapkin WaffleLapkin marked this pull request as ready for review August 8, 2022 23:44
@rustbot
Copy link
Collaborator

rustbot commented Aug 8, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@mina86
Copy link
Contributor

mina86 commented Aug 9, 2022

Thanks. LGTM.

@reitermarkus reitermarkus mentioned this pull request Sep 9, 2022
Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

Oops forgot to send off the review

@@ -299,7 +299,8 @@ impl CStr {
/// ```
///
#[unstable(feature = "cstr_from_bytes_until_nul", issue = "95027")]
pub fn from_bytes_until_nul(bytes: &[u8]) -> Result<&CStr, FromBytesUntilNulError> {
#[rustc_const_unstable(feature = "const_cstr_from_bytes", issue = "none")]
Copy link
Contributor

Choose a reason for hiding this comment

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

use the same feature gate and issue as the regular unstable attribute, these can be stabilized together.

Copy link
Member Author

@WaffleLapkin WaffleLapkin Sep 11, 2022

Choose a reason for hiding this comment

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

Should I open an issues for from_bytes_with_nul and to_bytes/to_bytes_with_nul/to_str? (they are stable, so I can't reuse the feature gate/issue)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea, I think it's best to track them. You could just munge them into a generic cstr_consts gate and make one issue for all of them

Comment on lines 53 to 55
match bytes {
[] => break None,
&[y, ..] if x == y => break Some(i),
[_, rest @ ..] => {
bytes = rest;
i += 1;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I appreciate the ingenuity, but why not just while let i < bytes.len() and index with i?

Copy link
Member Author

@WaffleLapkin WaffleLapkin Sep 11, 2022

Choose a reason for hiding this comment

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

I do not remember, probably because of a habit, I think there was a time when you couldn't index in const fns, but could use patterns.

@oli-obk
Copy link
Contributor

oli-obk commented Sep 12, 2022

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 12, 2022

📌 Commit cb02b64 has been approved by oli-obk

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 12, 2022
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 12, 2022
…=oli-obk

constify some `CStr` methods

This PR marks the following public APIs as `const`:
```rust
impl CStr {
    // feature(const_cstr_from_bytes)
    pub const fn from_bytes_until_nul(bytes: &[u8]) -> Result<&CStr, FromBytesUntilNulError>;
    pub const fn from_bytes_with_nul(bytes: &[u8]) -> Result<&Self, FromBytesWithNulError>;

    // feature(const_cstr_to_bytes)
    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>;
}
```

r? `@oli-obk` (use of `const_eval_select` :P )
cc `@mina86` (you've asked for this <3 )
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 12, 2022
…=oli-obk

constify some `CStr` methods

This PR marks the following public APIs as `const`:
```rust
impl CStr {
    // feature(const_cstr_from_bytes)
    pub const fn from_bytes_until_nul(bytes: &[u8]) -> Result<&CStr, FromBytesUntilNulError>;
    pub const fn from_bytes_with_nul(bytes: &[u8]) -> Result<&Self, FromBytesWithNulError>;

    // feature(const_cstr_to_bytes)
    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>;
}
```

r? ``@oli-obk`` (use of `const_eval_select` :P )
cc ``@mina86`` (you've asked for this <3 )
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 12, 2022
…llaumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#100185 (Fix `ReErased` leaking into typeck due to `typeof(...)` recovery)
 - rust-lang#100291 (constify some `CStr` methods)
 - rust-lang#101677 (Add test for rust-lang#101211)
 - rust-lang#101723 (Impove diagnostic for `.await`ing non-futures)
 - rust-lang#101724 (Allow unauthenticated users to add the `const-hack` label)
 - rust-lang#101731 (rustdoc: improve rustdoc HTML suggestions handling of nested generics)
 - rust-lang#101732 (Feature gate the `rustdoc::missing_doc_code_examples` lint)
 - rust-lang#101735 (rustdoc: fix treatment of backslash-escaped HTML)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7fc3183 into rust-lang:master Sep 13, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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