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

Add unstable copy_from_slice #31834

Merged
merged 1 commit into from
Feb 26, 2016
Merged

Add unstable copy_from_slice #31834

merged 1 commit into from
Feb 26, 2016

Conversation

strega-nil
Copy link
Contributor

implements rust-lang/rfcs#1419

r? alexcrichton

@rust-highfive
Copy link
Collaborator

r? @aturon

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

@strega-nil
Copy link
Contributor Author

I wrote that comment wrong :P

Hi @aturon :)

@bluss
Copy link
Member

bluss commented Feb 23, 2016

This method should be available in libcore too, so needs to be added to SliceExt, don't you agree?

@bluss
Copy link
Member

bluss commented Feb 23, 2016

Note, this does not fix the tracking issue -- the issue is open until the feature is stable.

@strega-nil
Copy link
Contributor Author

@bluss alright, I'll take out that thing about "fixes".

Also, yes I do think it should go in SliceExt. I'll implement that tonight.

@alexcrichton
Copy link
Member

Thanks @ubsan! In addition to what @bluss mentioned, could you also add a positive test which asserts that the behavior is as expected?

@strega-nil
Copy link
Contributor Author

@alexcrichton will do :)

@bluss
Copy link
Member

bluss commented Feb 24, 2016

Great! Let's go..

@bors r+ 849c4ce

@bluss
Copy link
Member

bluss commented Feb 24, 2016

@bors r-

travis found this one

src/libcollections/lib.rs:35:12: 35:27 error: unused or unknown feature, #[deny(unused_features)] on by default

src/libcollections/lib.rs:35 #![feature(copy_from_slice)]

@bluss
Copy link
Member

bluss commented Feb 24, 2016

Ok that's really weird, probably bug #30209

@strega-nil
Copy link
Contributor Author

@bluss that's weird, it worked on my end... I specifically added an unstable attribute, and then added a feature, and compiled and it worked.

Edit: and... it doesn't work for me if I take out the feature...

src/libcollections/slice.rs:859:9: 859:46 error: use of unstable library feature 'copy_from_slice' (see issue #31755)
src/libcollections/slice.rs:859         core_slice::SliceExt::copy_from_slice(self, src)

@bluss
Copy link
Member

bluss commented Feb 24, 2016

But then the libcollections code changed to just and UFCS trait method call, which is what the bug indicates

@bluss
Copy link
Member

bluss commented Feb 24, 2016

stage1 seems to have no such unused feature warning, but stage2 does when compiling libcollections.

@bluss
Copy link
Member

bluss commented Feb 24, 2016

When compiling libcollectionstest actually.

@alexcrichton
Copy link
Member

I think you'll just need to add this to the top of libcollections:

#![cfg_attr(not(test), feature(...))]

@bluss
Copy link
Member

bluss commented Feb 24, 2016

Yeah that seems to be right. That was a bit confusing.

@bluss
Copy link
Member

bluss commented Feb 26, 2016

Did you mean to have the libcollectionstest tests replace the run-pass/run-fail tests? I.e. remove the other ones?

@strega-nil
Copy link
Contributor Author

@bluss yes. Is that not what our tests should be? I am super confused about our tests :P

@bluss
Copy link
Member

bluss commented Feb 26, 2016

having them in just libcollectionstests seems good.

@strega-nil
Copy link
Contributor Author

cool :)

@bluss
Copy link
Member

bluss commented Feb 26, 2016

Feel free to squash the commits together if you want (to create a cleaner history).

@strega-nil
Copy link
Contributor Author

@bluss will do

@bluss
Copy link
Member

bluss commented Feb 26, 2016

Can we delete the two files in src/tests/ ? They are redundant now.

@bluss
Copy link
Member

bluss commented Feb 26, 2016

r=me without the redundant tests. I'm happy this is finally landing!

@strega-nil
Copy link
Contributor Author

@bluss I thought I had deleted those :/

@strega-nil
Copy link
Contributor Author

@bluss I feel like kind of a dumb right now :P

@alexcrichton
Copy link
Member

@bors: r+ e12b1f9

Thanks @ubsan!

@bors
Copy link
Contributor

bors commented Feb 26, 2016

⌛ Testing commit e12b1f9 with merge 0913004...

bors added a commit that referenced this pull request Feb 26, 2016
@bors bors merged commit e12b1f9 into rust-lang:master Feb 26, 2016
@strega-nil strega-nil deleted the copy_from_slice branch September 11, 2018 05:37
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.

7 participants