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

[std::cmp] add missing docs and provide an example #12956

Merged
merged 1 commit into from
Apr 4, 2014

Conversation

killerswan
Copy link
Contributor

No description provided.


```rust
// Our type.
struct SketchyNum(int);
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer this to be a normal struct like struct SketchyNum { num: int } so that the implementation below can be

(self.num - other.num).abs() < 5

for clarity. (It does makes the assertions slightly uglier, but not extremely so.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, not bad at all.

@alexcrichton
Copy link
Member

This looks great! Could you squash the commits into one?

Also, feel free to comment on the PR whenever you force-push, sadly github doesn't send out updates when that happens!

@killerswan
Copy link
Contributor Author

Squished!

@killerswan
Copy link
Contributor Author

Am I supposed to do something here?

@alexcrichton
Copy link
Member

The test run failed on bors. It looks like some of the new documentation examples failed when tested.

You can run the tests yourself via make check-stage1-doc-crate-std and then after that you can pass NO_REBUILD=1 to avoid the full bootstrap.

@killerswan
Copy link
Contributor Author

How do I see which tests failed on bors? None of the logs I see has anything to do with what I changed here...

@alexcrichton
Copy link
Member

Bors left a comment on the commit (#12956 (comment)) and it's the link marked failure:

@killerswan
Copy link
Contributor Author

FWIW, I've rebased to the latest master. There, somebody'd added a special case for cfg0 and so it was missing a doc comment, and I've added one for it.

Still compiling, though.

@killerswan
Copy link
Contributor Author

Well, so much for that:

$ make check-stage1-doc-crate-std
...
oxidize: x86_64-apple-darwin/stage0/lib/rustlib/x86_64-apple-darwin/bin/compiletest
/Users/kevin/code/rust/src/compiletest/compiletest.rs:1:1: 1:1 error: can't find crate for `native`
/Users/kevin/code/rust/src/compiletest/compiletest.rs:1 // Copyright 2012 The Rust Project Developers. See the COPYRIGHT
                                                        ^
error: aborting due to previous error
make: *** [x86_64-apple-darwin/stage0/lib/rustlib/x86_64-apple-darwin/bin/compiletest] Error 101

@alexcrichton
Copy link
Member

Ah sorry about that, looks like the dependencies are a bit off for the makefiles. Regardless, the travis build passed, which is now running tests, so I think this is good to go.

@killerswan
Copy link
Contributor Author

/me crosses fingers

@killerswan
Copy link
Contributor Author

This latest failure boils down to this, for example:

    <anon>:8:13: 8:23 error: multiple applicable methods in scope
    <anon>:8 assert_eq!( 5.cmp(&10), Less);     // because 5 < 10
                         ^~~~~~~~~~
    <std macros>:1:1: 12:2 note: in expansion of assert_eq!
    <anon>:8:1: 8:31 note: expansion site
    <anon>:8:13: 8:23 note: candidate #1 is `std::cmp::u8.TotalOrd::cmp`
    <anon>:8 assert_eq!( 5.cmp(&10), Less);     // because 5 < 10
                         ^~~~~~~~~~
    <std macros>:1:1: 12:2 note: in expansion of assert_eq!
    <anon>:8:1: 8:31 note: expansion site
    <anon>:8:13: 8:23 note: candidate #2 is `std::cmp::u16.TotalOrd::cmp`
    <anon>:8 assert_eq!( 5.cmp(&10), Less);     // because 5 < 10
                         ^~~~~~~~~~

Is there an easy way to tell rustc what kind of integer literal each of these should be by default?

@huonw
Copy link
Member

huonw commented Mar 27, 2014

Just add a suffix, e.g. 5u.cmp(&10) to force the 5 to be a uint (and then inference works for the 10).

@killerswan
Copy link
Contributor Author

What have I changed, though, which caused this to break?

@killerswan
Copy link
Contributor Author

Also still having trouble compiling locally:

/Users/kevin/code/rust/src/compiletest/compiletest.rs:1:1: 1:1 error: found possibly newer version of crate `std` which `native` depends on
/Users/kevin/code/rust/src/compiletest/compiletest.rs:1 // Copyright 2012 The Rust Project Developers. See the COPYRIGHT
                                                        ^
/Users/kevin/code/rust/src/compiletest/compiletest.rs:1:1: 1:1 note: perhaps this crate needs to be recompiled?
/Users/kevin/code/rust/src/compiletest/compiletest.rs:1 // Copyright 2012 The Rust Project Developers. See the COPYRIGHT
                                                        ^
error: aborting due to previous error
make: *** [x86_64-apple-darwin/stage0/lib/rustlib/x86_64-apple-darwin/bin/compiletest] Error 101

So I'm about to send in another change and we'll see...

@killerswan
Copy link
Contributor Author

From the looks of that error message I didn't push my last commit. I'll check in tomorrow.

@killerswan
Copy link
Contributor Author

Just rebased again (now: 971c90248e978bf7). Still seeing that "native" error and haven't necessarily sorted everything out, yet, sadly...

@alexcrichton
Copy link
Member

I would recommend running make check before we r+ it again to make sure that it's passing. Alternatively, you can build rustdoc and then run rustdoc --test src/libstd/lib.rs

@@ -55,6 +76,19 @@ pub trait TotalEq: Eq {
fn assert_receiver_is_total_eq(&self) {}
}

/*
Copy link
Contributor

Choose a reason for hiding this comment

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

This block comment doesn't really make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, true, but I'd been confused why the other had been inserted and then swapped with this. (Now I get it: we were waiting on another snapshot.)

@killerswan
Copy link
Contributor Author

Rebased and pushed 99fa22515, which went through make check OK, but obviously has failed elsewhere for Travis CI: possibly something to do with LLVM 3.4? But I'm going to try again for good measure with 02c9c94.

@killerswan
Copy link
Contributor Author

/me crosses fingers

@bors bors merged commit 02c9c94 into rust-lang:master Apr 4, 2014
@killerswan
Copy link
Contributor Author

Yay! Thanks @alexcrichton and @huonw and @thestinger. And @bors.

sanxiyn added a commit to sanxiyn/rust that referenced this pull request Apr 11, 2019
Remove strange formatting in `Ordering` docs.

I can't really fathom what the intent of the brackets is. The [original PR](rust-lang#12956) doesn't give any hints. I think it seems fine without them.
cramertj added a commit to cramertj/rust that referenced this pull request Apr 11, 2019
Remove strange formatting in `Ordering` docs.

I can't really fathom what the intent of the brackets is. The [original PR](rust-lang#12956) doesn't give any hints. I think it seems fine without them.
Centril added a commit to Centril/rust that referenced this pull request Apr 12, 2019
Remove strange formatting in `Ordering` docs.

I can't really fathom what the intent of the brackets is. The [original PR](rust-lang#12956) doesn't give any hints. I think it seems fine without them.
lnicola pushed a commit to lnicola/rust that referenced this pull request Aug 9, 2022
More methods and traits for `la_arena::ArenaMap`

Continue of rust-lang#12931. Seems that I forgot some methods in the previous PR :(

I also changed `ArenaMap::insert` to return the old value, to match the map-like collection API of std. **So this is a breaking change.**

r? `@lnicola`
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.

5 participants