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

Rollup of 7 pull requests #31006

Merged
merged 12 commits into from
Jan 19, 2016
Merged

Rollup of 7 pull requests #31006

merged 12 commits into from
Jan 19, 2016

Conversation

boblehest and others added 6 commits January 17, 2016 04:36
The space between `T` and `Bound` is the typical style used in code and
produced by rustdoc's rendering. Fixed first in Reflect's docs and then
I fixed all occurrences in docs I could find.
I'm using clang 7.2 which works just fine to compile Rust with, but was disallowed.
@Manishearth
Copy link
Member Author

@bors r+ p=10

@bors
Copy link
Contributor

bors commented Jan 18, 2016

📌 Commit a9c76a1 has been approved by Manishearth

@bors
Copy link
Contributor

bors commented Jan 18, 2016

⌛ Testing commit a9c76a1 with merge 6e57826...

@bors
Copy link
Contributor

bors commented Jan 18, 2016

💔 Test failed - auto-mac-64-opt

for bit in 0..len {
tester.insert(Zst, ());
}
assert_eq!(tester.len(), 1);
Copy link
Member

Choose a reason for hiding this comment

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

assertion here

thread '<main>' panicked at 'assertion failed: `(left == right)` (left: `0`, right: `1`)', /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/test/run-pass/zero-sized-btreemap-insert.rs:40

@KiChjang I think the test is buggy, it should be length len right?

Copy link
Member

Choose a reason for hiding this comment

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

It should really be an if-expression , if len is 0 then 0 else 1.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I'm not sure what it would be, based on the peculiar PartialOrd impl. Shouldn't it be inserting each Zst as a new value?

Copy link
Member

Choose a reason for hiding this comment

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

According to @gankro here we never get pass 1 at all as it's not a multi-map.

Copy link
Member

Choose a reason for hiding this comment

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

You didn't tell him about how you wrote PartialOrd.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, you're right, I didn't get my bearings correct yet.

Original: rust-lang#30968 (My first PR was targeting the wrong branch)
Sorry for nitpicking, but I think the example of the expanded macro should be wrapped inside a pair of curly braces to match the macro definition. Also the current example triggers a variable redefinition error.
Fix spacing style of `T: Bound` in docs

The space between `T` and `Bound` is the typical style used in code and
produced by rustdoc's rendering. Fixed first in Reflect's docs and then
I fixed all occurrences in docs I could find.
I'm using clang 7.2 which works just fine to compile Rust with, but was disallowed.
@Manishearth
Copy link
Member Author

@bors r+

@bors
Copy link
Contributor

bors commented Jan 18, 2016

📌 Commit 652fa5c has been approved by Manishearth

bors added a commit that referenced this pull request Jan 18, 2016
@bors bors merged commit 652fa5c into rust-lang:master Jan 19, 2016
@Centril Centril added the rollup A PR which is a rollup label Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants