Skip to content

Index all the collections #16195

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

Merged
merged 5 commits into from
Aug 12, 2014
Merged

Index all the collections #16195

merged 5 commits into from
Aug 12, 2014

Conversation

ftxqxd
Copy link
Contributor

@ftxqxd ftxqxd commented Aug 2, 2014

Implement Index for RingBuf, HashMap, TreeMap, SmallIntMap, and TrieMap.

If there’s anything that I missed or should be removed, let me know.

@Gankra
Copy link
Contributor

Gankra commented Aug 2, 2014

Index operations should probably be #[inline]. Also, you seem to have migrated some things to indexing, and other not. Any particular reason? Don't forget that code in comments should be included in "things to migrate".

@ftxqxd
Copy link
Contributor Author

ftxqxd commented Aug 2, 2014

I’ve updated some (hopefully all) examples to use indexing, and #[inline]d all the index methods.

The reason I only migrated some code is because that was all that seemed to trigger the deprecation warning when I compiled rustc and ran the tests.

@@ -1606,7 +1610,7 @@ impl<K: Eq + Hash<S>, V, S, H: Hasher<S>> HashMap<K, V, H> {
/// use std::collections::HashMap;
///
/// let mut map = HashMap::new();
/// map.insert("a", 1i);
// map.insert("a", 1i);
Copy link
Member

Choose a reason for hiding this comment

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

Accidental drop of a slash?

@alexcrichton
Copy link
Member

cc @aturon, given these guidelines I would like to r+ this, but would like to confirm just to make sure.

@aturon
Copy link
Member

aturon commented Aug 4, 2014

LGTM. We'll eventually want to migrate the get methods to not fail the task in most of these cases, according to the new guidelines, but that can be done separately.

@ftxqxd
Copy link
Contributor Author

ftxqxd commented Aug 7, 2014

Updated; tests should be passing now. r?

@@ -139,6 +139,8 @@ impl<T> RingBuf<T> {
/// # Example
///
/// ```rust
/// #![allow(deprecated)]
Copy link
Member

Choose a reason for hiding this comment

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

Can these attributes be placed only on the items where they're needed -- looks like the Index implementation? That way we don't accidentally start sneaking in other deprecated items.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, nevermind, this is in an example. Sorry!

ftxqxd added 5 commits August 12, 2014 15:32
This also deprecates RingBuf::get. Use indexing instead.
This also deprecates HashMap::get. Use indexing instead.
This also deprecates SmallIntMap::get. Use indexing instead.
@ftxqxd
Copy link
Contributor Author

ftxqxd commented Aug 12, 2014

Updated again; tests pass locally now. (Sorry for the other failures.) re-r?

@aturon
Copy link
Member

aturon commented Aug 12, 2014

No worries, thanks for updating!

bors added a commit that referenced this pull request Aug 12, 2014
Implement `Index` for `RingBuf`, `HashMap`, `TreeMap`, `SmallIntMap`, and `TrieMap`.

If there’s anything that I missed or should be removed, let me know.
@bors bors closed this Aug 12, 2014
@bors bors merged commit 8f71cb0 into rust-lang:master Aug 12, 2014
@ftxqxd ftxqxd deleted the more-index branch August 12, 2014 06:47
lnicola pushed a commit to lnicola/rust that referenced this pull request Jun 2, 2024
…ykril

Add `toggle_async_sugar` assist code action

Implement code action for sugaring and de-sugaring asynchronous functions.

This code action does not import `Future` trait when de-sugaring and does not touch function boby, I guess this can be implemented later if needed. This action also does not take into consideration other bounds because IMO it's usually "let me try to use sugared version here".

Feel free to request changes, that's my first code action implementation 😄

Closes rust-lang#17010
Relates to rust-lang#16195
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