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

ACP: char::MIN #252

Closed
clarfonthey opened this issue Jul 30, 2023 · 16 comments
Closed

ACP: char::MIN #252

clarfonthey opened this issue Jul 30, 2023 · 16 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@clarfonthey
Copy link

clarfonthey commented Jul 30, 2023

Proposal

Problem statement

Several standard primitive types offer MIN and MAX constants, which have a variety of use cases across the ecosystem. Unfortunately, char only offers a MAX constant, and not MIN.

Motivating examples or use cases

While Rust doesn't offer generic traits for minimum/maximum bounds on types, the standard MIN and MAX constants make implementing such functionality across a wide variety of types easy, since macros can simply expect <$t>::MIN and <$t>::MAX to exist. Unfortunately, char only has MAX, not MIN.

One of the arguments for why char::MAX exists is that it's difficult to remember and easy to mess up. For example, the literal '\u{10FFFF}' could easily be accidentally typed as \u{10FFF} and would be easy to miss. However, char::MAX is much harder to mistype without breaking, and is much clearer about its intent.

Compare this to the minimum value '\0' which is substantially easier to remember and a lot clearer. However, keep in mind that unsigned integers have an even simpler minimum, 0, and these also have MIN constants.

Solution sketch

impl char {
    const MIN: char = '\0';
}

Alternatives

The primary alternative is to simply not offer this constant, if the motivation doesn't justify it.

Links and related work

N/A

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@clarfonthey clarfonthey added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Jul 30, 2023
@BurntSushi
Copy link
Member

SGTM

@ChrisDenton
Copy link
Member

ChrisDenton commented Jul 30, 2023

I'm wondering where this would be used? Are there any more concrete examples?

I get the point about rounding out the API but I don't think that's enough justification in itself. And all the places where maybe char::MIN could be used I think '\0' is clearer, as you say. More so than a 0 integer even because \0 is usually spelled "nul" or "null" in English rather than "min".

I guess macro use would be the strongest argument but it's not really an integer so it doesn't really make sense to me to have it in a macro with integer types. Maybe there is a real world example of where this would be useful?

@BurntSushi
Copy link
Member

I'd probably use it here: https://github.com/rust-lang/regex/blob/a1910244f873e003efbe6e80ad9302c8ea949430/regex-syntax/src/unicode.rs#L708

I suspect there isn't really a case where char::MIN is strictly required.

@ChrisDenton
Copy link
Member

You're not even using char::MAX though, which I think is much more justifiable. And arguably char::NULL would make more sense in that case, especially as it's closer to ascii::char::Null.

@ChrisDenton
Copy link
Member

Or maybe that's an argument for having an ascii min too.

@BurntSushi
Copy link
Member

You're not even using char::MAX though

Because it's IMO weird to write ('\0', char::MAX). See parallelism. (The same concept applies to coding IMO, but because of the confusable word, it's difficult to quickly find a better source.)

@ChrisDenton
Copy link
Member

Ah I see, so you'd also want something like ASCII_MAX.

@clarfonthey
Copy link
Author

Just earlier I was manually implementing a trait for char that worked with a macro for other integer types because of the presence of MIN and MAX. Even the NonZero* types have MIN and MAX but char doesn't.

@ChrisDenton
Copy link
Member

I guess "other integer types" is what I'm not getting. To my mind char is not an integer type.

@clarfonthey
Copy link
Author

clarfonthey commented Jul 31, 2023

True, but there are many traits that you would implement for integers that you might implement for char. In my case, I was allow-listing types for something that could represent the state of an automaton, but I know that a lot of other code encapsulating "something integer-like that has strict bounds" also includes char.

Sure, it's one special case, but it's still not clear it's worth putting your foot down and not having a MIN value if people definitely would use it.

@ChrisDenton
Copy link
Member

Right but that's why I was asking for examples of where people would actually use it.

Also note that char has a hole in the middle so min and max don't adequately describe its bounds.

@BurntSushi
Copy link
Member

Process note: I think you just need sign off from one libs-api member (me in this case) to move forward with creating a PR and a tracking issue. My understanding is that debate carries on there as opposed to here.

(I'm happy to corrected on process. I just wanted to throw this out there as my understanding at present.)

@BurntSushi
Copy link
Member

That is, I agree that this is a plausible addition and that it's worth moving forward. But still fully expect to need consensus before stabilizing.

@scottmcm
Copy link
Member

Also note that char has a hole in the middle so min and max don't adequately describe its bounds.

I think that argument was already lost with the stabilization of https://doc.rust-lang.org/std/num/struct.NonZeroI32.html#associatedconstant.MIN in 1.70.

char::MIN..=char::MAX would be a thing that would iterate all possible chars.

@ChrisDenton
Copy link
Member

ChrisDenton commented Jul 31, 2023

Process note: I think you just need sign off from one libs-api member (me in this case) to move forward with creating a PR and a tracking issue. My understanding is that debate carries on there as opposed to here.

(I'm happy to corrected on process. I just wanted to throw this out there as my understanding at present.)

The current process is that you should add the tag "ACP-accepted" and close this issue.

@programmerjake
Copy link
Member

Right but that's why I was asking for examples of where people would actually use it.

quick example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=2337422cf15e939e0d099b061ae7dc83

@BurntSushi BurntSushi added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Jul 31, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 8, 2023
…tSushi

Add char::MIN

ACP: rust-lang/libs-team#252
Tracking issue: rust-lang#114298

r? `@rust-lang/libs-api`
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Sep 12, 2023
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
Add char::MIN

ACP: rust-lang/libs-team#252
Tracking issue: #114298

r? `@rust-lang/libs-api`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Add char::MIN

ACP: rust-lang/libs-team#252
Tracking issue: #114298

r? `@rust-lang/libs-api`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

5 participants