Skip to content

Audit integer types in libunicode, libcore/(char, str) and libstd/ascii #22339

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

Merged
merged 2 commits into from
Feb 16, 2015

Conversation

petrochenkov
Copy link
Contributor

Some function signatures have changed, so this is a [breaking-change].
In particular, radixes and numerical values of digits are represented by u32 now.

Part of #22240

@rust-highfive
Copy link
Contributor

r? @huonw

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

@@ -119,16 +119,16 @@ pub fn from_u32(i: u32) -> Option<char> {
/// ```
#[inline]
#[unstable(feature = "core", reason = "pending integer conventions")]
pub fn from_digit(num: uint, radix: uint) -> Option<char> {
pub fn from_digit(num: u32, radix: u32) -> Option<char> {
Copy link
Member

Choose a reason for hiding this comment

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

radix could be as small as u8 here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See the guideline here #22240

Actually u32 here is not as arbitrary as it may seem, radixes, for example, are directly tied to the textual representation of numbers, and u32 can be viewed as value of unicode code point. And numeric values of unicode digits (e.g. Roman numerals) can be larger than 256 (not in this example, but in others). Regardless, I think u8 would be as good as u32 and the choice doesn't really matter in practice :)

@huonw
Copy link
Member

huonw commented Feb 15, 2015

@bors r+ b1cd

Thanks!

Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 15, 2015
 Some function signatures have changed, so this is a [breaking-change].
In particular, radixes and numerical values of digits are represented by `u32` now.

Part of rust-lang#22240
@bors bors merged commit b1cd769 into rust-lang:master Feb 16, 2015
@petrochenkov petrochenkov deleted the int branch May 9, 2015 11:59
@petrochenkov
Copy link
Contributor Author

@behnam
I don't remember :(
I didn't choose types by random for sure, so there was probably some logic, but probably nothing important.
The constant is still unstable, so it can be changed.

@behnam
Copy link
Contributor

behnam commented Jun 30, 2017

No worries, @petrochenkov. I'm submitting a PR to update and will CC people who may have a comment. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants