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

Primitive docs relevant #48152

Merged
merged 5 commits into from Feb 17, 2018

Conversation

@antoyo
Copy link
Contributor

commented Feb 12, 2018

This fixes the documentation to show the right types in the examples for many integer methods.

I need to check if the result is correct before we merge.

@antoyo

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2018

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Feb 12, 2018

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

```
let n = 0b01001100", stringify!($SelfT), ";
assert_eq!(n.count_zeros(), 5);

This comment has been minimized.

Copy link
@joshtriplett

joshtriplett Feb 12, 2018

Member

This doesn't seem right; wouldn't the number of zeroes depend on the size of the type? It should be the number of bits in the type minus 3.

This comment has been minimized.

Copy link
@GuillaumeGomez

GuillaumeGomez Feb 12, 2018

Member

It would indeed!

@@ -1436,33 +1446,35 @@ macro_rules! uint_impl {
unsafe { intrinsics::ctlz(self as $ActualT) as u32 }

This comment has been minimized.

Copy link
@joshtriplett

joshtriplett Feb 12, 2018

Member

Shouldn't the example of leading_zeroes get the same treatment, rather than hardcoding u16?

This comment has been minimized.

Copy link
@joshtriplett

joshtriplett Feb 12, 2018

Member

(And ditto for other methods here whose docs still list specific types in their examples.)

This comment has been minimized.

Copy link
@antoyo

antoyo Feb 12, 2018

Author Contributor

Some methods were left as is because it was hard to make the example generic.

This comment has been minimized.

Copy link
@QuietMisdreavus

QuietMisdreavus Feb 14, 2018

Member

If you can't convert all the examples, I would like to see the "examples are shared between primitive integer types" notice kept on the primitive docs, since it still applies.

@GuillaumeGomez GuillaumeGomez force-pushed the antoyo:primitive-docs-relevant branch 3 times, most recently from 4f202bf to dcc9f74 Feb 12, 2018

@GuillaumeGomez GuillaumeGomez force-pushed the antoyo:primitive-docs-relevant branch from dcc9f74 to 9a30673 Feb 14, 2018

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

commented Feb 14, 2018

Should be good now.

@GuillaumeGomez GuillaumeGomez changed the title [WIP] Primitive docs relevant Primitive docs relevant Feb 14, 2018

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

commented Feb 14, 2018

Tests passed!

@QuietMisdreavus
Copy link
Member

left a comment

Just a couple notes. Thanks so much for doing this!

int_impl! { i128, i128, u128, 128, -170141183460469231731687303715884105728,
170141183460469231731687303715884105727, "#![feature(i128_type)]
#![feature(i128)]
# fn main() {

This comment has been minimized.

Copy link
@QuietMisdreavus

QuietMisdreavus Feb 14, 2018

Member

Wait, does this actually need the manual fn main? Rustdoc should move those feature flags outside its generated main function.

This comment has been minimized.

Copy link
@GuillaumeGomez

GuillaumeGomez Feb 14, 2018

Member

Apparently it fails otherwise. I was quite surprised as well...

This comment has been minimized.

Copy link
@QuietMisdreavus

QuietMisdreavus Feb 14, 2018

Member

I wonder if it has to do with line breaks. Did your previous attempt have a line break after the feature flags? I think rustdoc doesn't do anything fancier than "if the line starts with #![, splice out the whole line".

@@ -1436,33 +1446,35 @@ macro_rules! uint_impl {
unsafe { intrinsics::ctlz(self as $ActualT) as u32 }

This comment has been minimized.

Copy link
@QuietMisdreavus

QuietMisdreavus Feb 14, 2018

Member

If you can't convert all the examples, I would like to see the "examples are shared between primitive integer types" notice kept on the primitive docs, since it still applies.

@GuillaumeGomez GuillaumeGomez force-pushed the antoyo:primitive-docs-relevant branch from 57ca76a to 57498e1 Feb 14, 2018

@QuietMisdreavus

This comment has been minimized.

Copy link
Member

commented Feb 14, 2018

[00:03:47] error: this file contains an un-closed delimiter
[00:03:47]     --> libcore/num/mod.rs:3756:4
[00:03:47]      |
[00:03:47] 3756 | ];
[00:03:47]      |    ^
[00:03:47]      |
[00:03:47] help: did you mean to close this delimiter?
[00:03:47]     --> libcore/num/mod.rs:1390:24
[00:03:47]      |
[00:03:47] 1390 | macro_rules! uint_impl {
[00:03:47]      |                        ^
[00:03:47] 
[00:03:47] error: no rules expected the token `#`
[00:03:47]     --> libcore/num/mod.rs:2481:1
[00:03:47]      |
[00:03:47] 2481 | #[lang = "u8"]
[00:03:47]      | ^
[00:03:47] 
[00:03:47] error: Could not compile `core`.

@GuillaumeGomez GuillaumeGomez force-pushed the antoyo:primitive-docs-relevant branch from 57498e1 to 4380672 Feb 14, 2018

@QuietMisdreavus

This comment has been minimized.

Copy link
Member

commented Feb 15, 2018

[01:12:33]    Doc-tests core
[01:12:36] 
[01:12:36] running 1324 tests
<snip>
[01:14:46] failures:
[01:14:46] 
[01:14:46] ---- num/mod.rs - num::u128::leading_zeros (line 107) stdout ----
[01:14:46] 	error[E0658]: 128-bit type is unstable (see issue #35118)
[01:14:46]  --> num/mod.rs:108:9
[01:14:46]   |
[01:14:46] 4 | let n = u128::max_value() >> 2;
[01:14:46]   |         ^^^^^^^^^^^^^^^
[01:14:46]   |
[01:14:46]   = help: add #![feature(i128_type)] to the crate attributes to enable
[01:14:46] 
[01:14:46] thread 'rustc' panicked at 'couldn't compile the test', librustdoc/test.rs:298:13
[01:14:46] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:14:46] 
[01:14:46] 
[01:14:46] failures:
[01:14:46]     num/mod.rs - num::u128::leading_zeros (line 107)

@GuillaumeGomez GuillaumeGomez force-pushed the antoyo:primitive-docs-relevant branch from 4380672 to 4a42042 Feb 15, 2018

@QuietMisdreavus

This comment has been minimized.

Copy link
Member

commented Feb 15, 2018

[01:19:36]    Doc-tests core
[01:19:40] 
[01:19:40] running 1324 tests
<snip>
[01:22:06] failures:
[01:22:06] 
[01:22:06] ---- num/mod.rs - num::u128::leading_zeros (line 107) stdout ----
[01:22:06] 	error: this file contains an un-closed delimiter
[01:22:06]  --> num/mod.rs:114:35
[01:22:06]   |
[01:22:06] 9 | assert_eq!(n.leading_zeros(), 2);
[01:22:06]   |                                   ^
[01:22:06]   |
[01:22:06] help: did you mean to close this delimiter?
[01:22:06]  --> num/mod.rs:111:11
[01:22:06]   |
[01:22:06] 6 | fn main() {
[01:22:06]   |           ^
[01:22:06] 
[01:22:06] thread 'rustc' panicked at 'couldn't compile the test', librustdoc/test.rs:298:13
[01:22:06] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:22:06] 
[01:22:06] 
[01:22:06] failures:
[01:22:06]     num/mod.rs - num::u128::leading_zeros (line 107)

@GuillaumeGomez GuillaumeGomez force-pushed the antoyo:primitive-docs-relevant branch from 4a42042 to d692e94 Feb 15, 2018

@QuietMisdreavus

This comment has been minimized.

Copy link
Member

commented Feb 15, 2018

Looks like we're all set! Thanks so much!

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2018

📌 Commit d692e94 has been approved by QuietMisdreavus

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 16, 2018

Rollup merge of rust-lang#48152 - antoyo:primitive-docs-relevant, r=Q…
…uietMisdreavus

Primitive docs relevant

This fixes the documentation to show the right types in the examples for many integer methods.

I need to check if the result is correct before we merge.

bors added a commit that referenced this pull request Feb 16, 2018

Auto merge of #48261 - GuillaumeGomez:rollup, r=GuillaumeGomez
Rollup of 8 pull requests

- Successful merges: #47799, #48095, #48123, #48152, #48234, #48236, #48239, #48243
- Failed merges:
@kennytm

This comment has been minimized.

Copy link
Member

commented Feb 16, 2018

@bors r-

Unfortunately this PR failed on 32-bit platforms.

[00:03:20] error: unexpected end of macro invocation
[00:03:20]     --> src/libcore/num/mod.rs:1380:42
[00:03:20]      |
[00:03:20] 1380 |     int_impl! { isize, i32, u32, 32, "", "" }
[00:03:20]      |                                          ^^
}

#[cfg(target_pointer_width = "32")]
#[lang = "isize"]
impl isize {
int_impl! { isize, i32, u32, 32 }
int_impl! { isize, i32, u32, 32, "", "" }

This comment has been minimized.

Copy link
@kennytm

kennytm Feb 16, 2018

Member

You are missing -2147483648, 2147483647 here. Similar for the 16-bit code at line 1374 above.

This comment has been minimized.

Copy link
@GuillaumeGomez

GuillaumeGomez Feb 16, 2018

Member

Indeed, thanks!

@GuillaumeGomez GuillaumeGomez force-pushed the antoyo:primitive-docs-relevant branch from d692e94 to a58409d Feb 16, 2018

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

commented Feb 16, 2018

Fixed.

@bors: r=QuietMisdreavus

@bors

This comment has been minimized.

Copy link
Contributor

commented Feb 16, 2018

📌 Commit a58409d has been approved by QuietMisdreavus

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 17, 2018

Rollup merge of rust-lang#48152 - antoyo:primitive-docs-relevant, r=Q…
…uietMisdreavus

Primitive docs relevant

This fixes the documentation to show the right types in the examples for many integer methods.

I need to check if the result is correct before we merge.

bors added a commit that referenced this pull request Feb 17, 2018

Auto merge of #48294 - GuillaumeGomez:rollup, r=GuillaumeGomez
Rollup of 8 pull requests

- Successful merges: #48095, #48152, #48234, #48239, #48243, #48260, #48284, #48286
- Failed merges:

@bors bors merged commit a58409d into rust-lang:master Feb 17, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.