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

Make {str,[u8]}::eq_ignore_ascii_case const #104126

Closed

Conversation

coolreader18
Copy link
Contributor

Both of these can trivially be implemented with already-const-stable language constructs/functions, so I figured it would make sense to make them const. I was going to make make_ascii_{upper,lower}case const too, but I realized that requires const_mut_refs and u8/char::make_ascii_*case aren't even unstably const under that, so I figured I'd leave that for after that's stabilized. Also, unfortunately, neither of the existing implementations can just have const added (maybe is_ascii, but it'd need a bunch of extra unstably-const ptr methods), and also eq_ignore_ascii_case codegen'd worse with the const-friendly while loop version, so I had to use const_eval_select for both.

@rustbot
Copy link
Collaborator

rustbot commented Nov 7, 2022

r? @joshtriplett

(rustbot has picked a reviewer for you, use r? to override)

@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 Nov 7, 2022
@rustbot
Copy link
Collaborator

rustbot commented Nov 7, 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

@coolreader18
Copy link
Contributor Author

Failure was just that I forgot to add SAFETY comments for the unsafe blocks.

@bors
Copy link
Contributor

bors commented Nov 19, 2022

☔ The latest upstream changes (presumably #102795) made this pull request unmergeable. Please resolve the merge conflicts.

@ChrisDenton ChrisDenton added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 1, 2023
@ChrisDenton
Copy link
Member

Not much movement on this yet so cc @rust-lang/wg-const-eval in case anyone has any thoughts on this.

@fee1-dead
Copy link
Member

I think this is okay (because it doesn't use const traits) but I think libs-api should have a say on whether use of const_eval_select is okay in terms of readability and maintainability.

@bors
Copy link
Contributor

bors commented May 5, 2023

☔ The latest upstream changes (presumably #111113) made this pull request unmergeable. Please resolve the merge conflicts.

@coolreader18 coolreader18 changed the title Make str::is_ascii and str::eq_ignore_ascii_case (and their equivalent [u8] versions) const Make {str,[u8]}::eq_ignore_ascii_case const May 10, 2023
@coolreader18
Copy link
Contributor Author

Actually, I really don't feel strongly about just eq_ignore_ascii_case (especially since slice == isn't even const stable yet) so I'll close this since #111090 did is_ascii

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants