Skip to content

Kick off discussion about extracting bit primitives #112

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

Closed
wants to merge 1 commit into from

Conversation

flavius
Copy link

@flavius flavius commented Jun 25, 2019

This is my first contribution to rust, so feel free to point me to the right direction if I do something unconventionally.

Rust ticket: rust-lang/rust#50592

For context, data types I could find:

bit_set.rs:27:pub struct BitSet<T: Idx> {
bit_set.rs:294:pub struct BitIter<'a, T: Idx> {
bit_set.rs:348:pub struct SparseBitSet<T: Idx> {
bit_set.rs:644:pub struct GrowableBitSet<T: Idx> {
bit_set.rs:695:pub struct BitMatrix<R: Idx, C: Idx> {
bit_set.rs:886:pub struct SparseBitMatrix<R, C>
bit_set.rs:453:pub enum HybridBitSet<T: Idx> {

Decisions to be made:

  • whether it pays off to extract them at all (if not, close the rust ticket?)
  • crate name (the one currently commited is just a placeholder)
  • repository location (the crates included in this document use rust-lang, but the rust ticket mentions rust-lang-nursery)

@rfcbot merge

@pnkfelix pnkfelix self-requested a review June 27, 2019 10:59
@nikomatsakis
Copy link
Contributor

Some thoughts:

First off, I think the set of types is not sufficient. We're going to need to include (for example) the Idx trait itself -- and at that point I think it makes sense to include the IndexVec abstraction.

One concern is that this code uses some internal rustc features for efficiency purposes (in particular, we forbid indices from taking on any value greater than 0xFFFF_FF00 or something like that). I don't think we want to stop doing that, but it also will limit the "general purpose utility" of said crates in terms of having other consumers apart from rustc. We'd have to introduce cargo features, I imagine (e.g., one that switches to use NonZeroUsize instead of the internal attributes we use now).

On the other hand, we probably don't want too many other consumers anyway, because that would imply a certain measure of stability and I'm not sure we're up to maintaining it, nor to making the API particularly nice.

One reason I can see to do this, apart from providing general value to the ecosystem, is that it would help to support crates like chalk, polonius, and so forth, which wind up wanting some of this functionality. The other reason of course is that it enables people to maintain the crates with less overhead.

On the other hand, it will make larger scale changes harder, as one must modify the crate first and then publish to crates.io and so forth.

I guess one question for me is if we have other consumers in mind already. I should check whether (e.g.) chalk might actually make use of these types.

@nikomatsakis
Copy link
Contributor

We discussed this a bit on Zulip, and we had the idea that maybe we should start by just creating the crate in tree before we talk about moving it out of tree. Once the crate boundary is established, we can decide whether to pull it into a separate repository.

I think this is a great first step, therefore I'm going to go ahead and close this issue in favor of that plan -- we can re-open once we have something concrete to talk about!

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

24 similar comments
@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

23 similar comments
@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2019

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@nikomatsakis nikomatsakis reopened this Jul 12, 2019
@nikomatsakis
Copy link
Contributor

Woah, something seems to have upset triagebot. :)

@pietroalbini
Copy link
Member

What the hell.

@rust-lang rust-lang deleted a comment from rustbot Jul 12, 2019
@spastorino
Copy link
Member

It seems like this was reopened because triagebot was mad, if it was reopened on purpose please comment back on the issue.

Trying to close it again ...

@spastorino spastorino closed this Jul 25, 2019
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