Skip to content

Conversation

ljedrz
Copy link
Contributor

@ljedrz ljedrz commented Oct 16, 2018

This simplifies some of the HybridBitSet code.

cc @nnethercote

@rust-highfive
Copy link
Contributor

r? @matthewjasper

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 16, 2018
Copy link
Contributor

@matthewjasper matthewjasper left a comment

Choose a reason for hiding this comment

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

Nice!

}
let mut new_dense = self_sparse.to_dense();
let changed = new_dense.union(other_dense);
mem::replace(self, HybridBitSet::Dense(new_dense));
Copy link
Contributor

Choose a reason for hiding this comment

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

Likewise

@ljedrz ljedrz force-pushed the simplify_hybridbitset branch from 497e53c to 5d8f0e1 Compare October 16, 2018 18:27
@ljedrz
Copy link
Contributor Author

ljedrz commented Oct 16, 2018

@matthewjasper comment addressed.

}
let mut new_dense = self_sparse.to_dense();
let changed = new_dense.union(other_dense);
*self = HybridBitSet::Dense(new_dense);
Copy link
Contributor

Choose a reason for hiding this comment

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

The indentation on this line is wrong.

let changed = dense.insert(elem);
assert!(changed);
*self = HybridBitSet::Dense(dense);
changed
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Might as well remove this blank line, now.

@nnethercote
Copy link
Contributor

I tried this patch with an oldish (~3 weeks ago) repo and got compile errors, and then I updated and now it works. I figure NLL must have been enabled for rustc in the meantime?

@matthewjasper
Copy link
Contributor

Yes, it got enabled with the beta upgrade

@nnethercote
Copy link
Contributor

Cool, I didn't know it would improve this code. That's nice!

@ljedrz ljedrz force-pushed the simplify_hybridbitset branch from 5d8f0e1 to 1dd92c4 Compare October 16, 2018 20:14
@ljedrz
Copy link
Contributor Author

ljedrz commented Oct 16, 2018

Whitespaces fixed.

@matthewjasper
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 16, 2018

📌 Commit 1dd92c4 has been approved by matthewjasper

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 16, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Oct 18, 2018
…thewjasper

Remove HybridBitSet::dummy

This simplifies some of the `HybridBitSet` code.

cc @nnethercote
bors added a commit that referenced this pull request Oct 18, 2018
Rollup of 18 pull requests

Successful merges:

 - #54646 (improve documentation on std::thread::sleep)
 - #54933 (Cleanup the rest of codegen_llvm)
 - #54964 (Run both lldb and gdb tests)
 - #55016 (Deduplicate some code and compile-time values around vtables)
 - #55031 (Improve verify_llvm_ir config option)
 - #55050 (doc std::fmt: the Python inspiration is already mentioned in precedin…)
 - #55077 (rustdoc: Use dyn keyword when rendering dynamic traits)
 - #55080 (Detect if access to localStorage is forbidden by the user's browser)
 - #55090 (regression test for move out of borrow via pattern)
 - #55102 (resolve: Do not skip extern prelude during speculative resolution)
 - #55104 (Add test for #34229)
 - #55111 ([Rustc Book] Explain --cfg's arguments)
 - #55122 (Cleanup mir/borrowck)
 - #55127 (Remove HybridBitSet::dummy)
 - #55128 (Fix LLVMRustInlineAsmVerify return type mismatch)
 - #55142 (miri: layout should not affect CTFE checks (outside of validation))
 - #55151 (Cleanup nll)
 - #55161 ([librustdoc] Disable spellcheck for search field)
@bors bors merged commit 1dd92c4 into rust-lang:master Oct 18, 2018
@ljedrz ljedrz deleted the simplify_hybridbitset branch October 18, 2018 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants