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: Recreate a collections module #14538

Closed
wants to merge 2 commits into from

Conversation

alexcrichton
Copy link
Member

As with the previous commit with librand, this commit shuffles around some
collections code. The new state of the world is similar to that of librand:

  • The libcollections crate now only depends on libcore and liballoc.
  • The standard library has a new module, std::collections. All functionality
    of libcollections is reexported through this module.

I would like to stress that this change is purely cosmetic. There are very few
alterations to these primitives.

There are a number of notable points about the new organization:

  • std::{str, slice, string, vec} all moved to libcollections. There is no reason
    that these primitives shouldn't be necessarily usable in a freestanding
    context that has allocation. These are all reexported in their usual places in
    the standard library.

  • The hashmap, and transitively the lru_cache, modules no longer reside in
    libcollections, but rather in libstd. The reason for this is because the
    HashMap::new contructor requires access to the OSRng for initially seeding
    the hash map. Beyond this requirement, there is no reason that the hashmap
    could not move to libcollections.

    I do, however, have a plan to move the hash map to the collections module. The
    HashMap::new function could be altered to require that the H hasher
    parameter ascribe to the Default trait, allowing the entire hashmap module
    to live in libcollections. The key idea would be that the default hasher would
    be different in libstd. Something along the lines of:

    // src/libstd/collections/mod.rs
    
    pub type HashMap<K, V, H = RandomizedSipHasher> =
          core_collections::HashMap<K, V, H>;
    

    This is not possible today because you cannot invoke static methods through
    type aliases. If we modified the compiler, however, to allow invocation of
    static methods through type aliases, then this type definition would
    essentially be switching the default hasher from SipHasher in libcollections
    to a libstd-defined RandomizedSipHasher type. This type's Default
    implementation would randomly seed the SipHasher instance, and otherwise
    perform the same as SipHasher.

    This future state doesn't seem incredibly far off, but until that time comes,
    the hashmap module will live in libstd to not compromise on functionality.

  • In preparation for the hashmap moving to libcollections, the hash module has
    moved from libstd to libcollections. A previously snapshotted commit enables a
    distinct Writer trait to live in the hash module which Hash
    implementations are now parameterized over.

    Due to using a custom trait, the SipHasher implementation has lost its
    specialized methods for writing integers. These can be re-added
    backwards-compatibly in the future via default methods if necessary, but the
    FNV hashing should satisfy much of the need for speedier hashing.

A list of breaking changes:

  • HashMap::{get, get_mut} no longer fails with the key formatted into the error
    message with {:?}, instead, a generic message is printed. With backtraces,
    it should still be not-too-hard to track down errors.
  • The HashMap, HashSet, and LruCache types are now available through
    std::collections instead of the collections crate.
  • Manual implementations of hash should be parameterized over hash::Writer
    instead of just Writer.

[breaking-change]

@alexcrichton
Copy link
Member Author

cc #13851

@brson brson mentioned this pull request May 30, 2014
9 tasks
@huonw
Copy link
Member

huonw commented May 30, 2014

cc me

@@ -1330,7 +1332,7 @@ mod tests {
fn test_from_bytes() {
let bitv = from_bytes([0b10110110, 0b00000000, 0b11111111]);
let str = format!("{}{}{}", "10110110", "00000000", "11111111");
assert_eq!(bitv.to_str(), str);
assert_eq!(bitv.to_str().as_slice(), str.as_slice());
Copy link
Member

Choose a reason for hiding this comment

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

Why're these as_sliceings needed? (String no longer implements Eq?!?)

Copy link
Member Author

Choose a reason for hiding this comment

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

"foo".to_str() is returning realstd::string::String while bitv.to_str() is returning collections::string::String, so it was easier to compare string slices rather than the String type.

The String type definitely still implements Eq

@huonw
Copy link
Member

huonw commented May 31, 2014

I'm not keen on adding more isolated Writer traits, especially with that name (e.g. it could be HashWriter like fmt has FormatWriter). Also, this makes it very feasible for Vec to move under std::collections right?

@alexcrichton
Copy link
Member Author

Thanks @huonw for taking a look at this!

I'm still a bit torn on how to name these Writer instances. We have a number of examples across the board:

fmt::Result
io::IoResult
io::Writer
hash::Writer
fmt::FormatWriter

I tend to lean towards module::Name rather than module::ModuleName, even when duplicating the same Name across separate modules. Renaming to HashWriter is possible, but it always seems odd to import hash::HashWriter (the stuttering).

Aha, it does indeed allow Vec to move under collections! The str, vec, string, and slice modules of libcollections are all intertwined with one another, so they would likely want to move as a unit. At the very least we could reexport Vec under std::collections, however. (and possibly String)?

The manual imports of String and Vec should be quite rare due to their position in the prelude, so they should be pretty easy to change (unless you're importing MoveItems). Decisions, decisions!

As with the previous commit with `librand`, this commit shuffles around some
`collections` code. The new state of the world is similar to that of librand:

* The libcollections crate now only depends on libcore and liballoc.
* The standard library has a new module, `std::collections`. All functionality
  of libcollections is reexported through this module.

I would like to stress that this change is purely cosmetic. There are very few
alterations to these primitives.

There are a number of notable points about the new organization:

* std::{str, slice, string, vec} all moved to libcollections. There is no reason
  that these primitives shouldn't be necessarily usable in a freestanding
  context that has allocation. These are all reexported in their usual places in
  the standard library.

* The `hashmap`, and transitively the `lru_cache`, modules no longer reside in
  `libcollections`, but rather in libstd. The reason for this is because the
  `HashMap::new` contructor requires access to the OSRng for initially seeding
  the hash map. Beyond this requirement, there is no reason that the hashmap
  could not move to libcollections.

  I do, however, have a plan to move the hash map to the collections module. The
  `HashMap::new` function could be altered to require that the `H` hasher
  parameter ascribe to the `Default` trait, allowing the entire `hashmap` module
  to live in libcollections. The key idea would be that the default hasher would
  be different in libstd. Something along the lines of:

      // src/libstd/collections/mod.rs

      pub type HashMap<K, V, H = RandomizedSipHasher> =
            core_collections::HashMap<K, V, H>;

  This is not possible today because you cannot invoke static methods through
  type aliases. If we modified the compiler, however, to allow invocation of
  static methods through type aliases, then this type definition would
  essentially be switching the default hasher from `SipHasher` in libcollections
  to a libstd-defined `RandomizedSipHasher` type. This type's `Default`
  implementation would randomly seed the `SipHasher` instance, and otherwise
  perform the same as `SipHasher`.

  This future state doesn't seem incredibly far off, but until that time comes,
  the hashmap module will live in libstd to not compromise on functionality.

* In preparation for the hashmap moving to libcollections, the `hash` module has
  moved from libstd to libcollections. A previously snapshotted commit enables a
  distinct `Writer` trait to live in the `hash` module which `Hash`
  implementations are now parameterized over.

  Due to using a custom trait, the `SipHasher` implementation has lost its
  specialized methods for writing integers. These can be re-added
  backwards-compatibly in the future via default methods if necessary, but the
  FNV hashing should satisfy much of the need for speedier hashing.

A list of breaking changes:

* HashMap::{get, get_mut} no longer fails with the key formatted into the error
  message with `{:?}`, instead, a generic message is printed. With backtraces,
  it should still be not-too-hard to track down errors.

* The HashMap, HashSet, and LruCache types are now available through
  std::collections instead of the collections crate.

* Manual implementations of hash should be parameterized over `hash::Writer`
  instead of just `Writer`.

[breaking-change]
bors added a commit that referenced this pull request Jun 5, 2014
As with the previous commit with `librand`, this commit shuffles around some
`collections` code. The new state of the world is similar to that of librand:

* The libcollections crate now only depends on libcore and liballoc.
* The standard library has a new module, `std::collections`. All functionality
  of libcollections is reexported through this module.

I would like to stress that this change is purely cosmetic. There are very few
alterations to these primitives.

There are a number of notable points about the new organization:

* std::{str, slice, string, vec} all moved to libcollections. There is no reason
  that these primitives shouldn't be necessarily usable in a freestanding
  context that has allocation. These are all reexported in their usual places in
  the standard library.

* The `hashmap`, and transitively the `lru_cache`, modules no longer reside in
  `libcollections`, but rather in libstd. The reason for this is because the
  `HashMap::new` contructor requires access to the OSRng for initially seeding
  the hash map. Beyond this requirement, there is no reason that the hashmap
  could not move to libcollections.

  I do, however, have a plan to move the hash map to the collections module. The
  `HashMap::new` function could be altered to require that the `H` hasher
  parameter ascribe to the `Default` trait, allowing the entire `hashmap` module
  to live in libcollections. The key idea would be that the default hasher would
  be different in libstd. Something along the lines of:

      // src/libstd/collections/mod.rs

      pub type HashMap<K, V, H = RandomizedSipHasher> =
            core_collections::HashMap<K, V, H>;

  This is not possible today because you cannot invoke static methods through
  type aliases. If we modified the compiler, however, to allow invocation of
  static methods through type aliases, then this type definition would
  essentially be switching the default hasher from `SipHasher` in libcollections
  to a libstd-defined `RandomizedSipHasher` type. This type's `Default`
  implementation would randomly seed the `SipHasher` instance, and otherwise
  perform the same as `SipHasher`.

  This future state doesn't seem incredibly far off, but until that time comes,
  the hashmap module will live in libstd to not compromise on functionality.

* In preparation for the hashmap moving to libcollections, the `hash` module has
  moved from libstd to libcollections. A previously snapshotted commit enables a
  distinct `Writer` trait to live in the `hash` module which `Hash`
  implementations are now parameterized over.

  Due to using a custom trait, the `SipHasher` implementation has lost its
  specialized methods for writing integers. These can be re-added
  backwards-compatibly in the future via default methods if necessary, but the
  FNV hashing should satisfy much of the need for speedier hashing.

A list of breaking changes:

* HashMap::{get, get_mut} no longer fails with the key formatted into the error
  message with `{:?}`, instead, a generic message is printed. With backtraces,
  it should still be not-too-hard to track down errors.

* The HashMap, HashSet, and LruCache types are now available through
  std::collections instead of the collections crate.

* Manual implementations of hash should be parameterized over `hash::Writer`
  instead of just `Writer`.

[breaking-change]
@bors bors closed this Jun 5, 2014
@alexcrichton alexcrichton deleted the libcollections branch June 6, 2014 02:13
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 5, 2023
fix: Fix project linking popup appearing for modules that can be linked to a crate

should fix rust-lang/rust-analyzer#14523
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.

3 participants