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

Ensure Extend is implemented and renamed Extendable to Extend #18475

Merged
merged 2 commits into from
Nov 8, 2014

Conversation

gamazeps
Copy link
Contributor

Ensured that Extend & FromIterator are implemented for the libcollection.

Removed the fact that FromIterator had to be implemented in order to implement Extend, as it did not make sense for LruCache (it needs to be given a size and there are no Default for LruCache).

Changed the name from Extend to Extendable.

Part of #18424

@huonw
Copy link
Member

huonw commented Oct 31, 2014

This seems to also add IntoIterator to the prelude?

@huonw
Copy link
Member

huonw commented Oct 31, 2014

Oh, no, I'm just dumb: that's the vim syntax file. sidles away

@gamazeps
Copy link
Contributor Author

:p

@@ -74,8 +74,8 @@ pub trait FromIterator<A> {
}

/// A type growable from an `Iterator` implementation
pub trait Extendable<A>: FromIterator<A> {
/// Extend a container with the elements yielded by an iterator
pub trait Extend<A>: {
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to drop the extra colon here

@alexcrichton
Copy link
Member

Awesome work, thanks @gamazeps! Could you also do these two things?

  1. Flag the commit message as a [breaking-change]
  2. Add a #[deprecated] pub use for retaining Extendable for now

Other than that I think this is good to go!

@alexcrichton alexcrichton mentioned this pull request Oct 31, 2014
24 tasks
@gamazeps gamazeps force-pushed the toExtend branch 5 times, most recently from 23c2119 to cd3c88b Compare October 31, 2014 19:39
@gamazeps
Copy link
Contributor Author

r? @alexcrichton

@alexcrichton
Copy link
Member

Could you leave the #[experimental] annotations for now actually? We'll be switching those to #[stable] in a final sweep.

@gamazeps
Copy link
Contributor Author

gamazeps commented Nov 1, 2014

Done :)

@gamazeps
Copy link
Contributor Author

gamazeps commented Nov 5, 2014

@alexcrichton does it need a rebase, or can I simply ask bors to retry ?

@alexcrichton
Copy link
Member

Yeah this just needs to be rebased against master and then it's god to go again.

@gamazeps
Copy link
Contributor Author

gamazeps commented Nov 8, 2014

@alexcrichton Done :) (teh rebase was awfull ^^)

@gamazeps
Copy link
Contributor Author

gamazeps commented Nov 8, 2014

@bors retry

@Gankra
Copy link
Contributor

Gankra commented Nov 8, 2014

@gamazeps the error looks legit:

C:\bot\slave\auto-win-32-nopt-c\build\src\libcore\iter.rs:82:37: 82:72 error: `use` and `extern crate` declarations must precede items
C:\bot\slave\auto-win-32-nopt-c\build\src\libcore\iter.rs:82 #[deprecated = "renamed to Extend"] pub use self::Extend as Extendable;

@gamazeps
Copy link
Contributor Author

gamazeps commented Nov 8, 2014

@gankro, yep I realised it just after the retry ^^
It seems like the syntax changed since last week ...

In order to upgrade, simply rename the Extendable trait to Extend in
your code

Part of rust-lang#18424

[breaking-change]
@gamazeps
Copy link
Contributor Author

gamazeps commented Nov 8, 2014

Fixed it :)

bors added a commit that referenced this pull request Nov 8, 2014
Ensured that Extend & FromIterator are implemented for the libcollection.

Removed the fact that FromIterator had to be implemented in order to implement Extend, as it did not make sense for LruCache (it needs to be given a size and there are no Default for LruCache).

Changed the name from Extend to Extendable.

Part of #18424
@bors bors closed this Nov 8, 2014
@bors bors merged commit a11f167 into rust-lang:master Nov 8, 2014
@forkloop
Copy link

Extend will not inherit from FromIterator as specified in the RFC? Shall we update the RFC there?

@Gankra
Copy link
Contributor

Gankra commented Nov 10, 2014

CC @aturon, this is a change we made because not all Extendables can be constructed FromIterators (need extra args to be constructed). Not sure if the RFC needs to be updated, but I'm also not sure if we ever ran this past you.

@aturon
Copy link
Member

aturon commented Nov 10, 2014

This is a fine change. @forkloop, can you please add a comment to the tracking issue about this change? We'll file an amendment to the RFC after we've collected all the implementation tweaks.

@forkloop
Copy link

I added a comment in the tracking issue, thanks a lot for the work!

@gamazeps gamazeps deleted the toExtend branch May 7, 2017 14:52
lnicola pushed a commit to lnicola/rust that referenced this pull request Nov 12, 2024
Support new #[rustc_intrinsic] attribute and fallback bodies
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.

8 participants