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

Vec::extend_from_slice’s documentation forgot about the difference of T: Clone vs. T: Copy bounds. #97119

Open
steffahn opened this issue May 17, 2022 · 4 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-slice Area: [T] T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@steffahn
Copy link
Member

Quoting from https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#method.extend_from_slice

Note that this function is same as extend except that it is specialized to work with slices instead.

But Vec::extend_from_slice supports &[T] for T: Clone, whereas Vec::extend only supports impl Iterator<Item = &T> if T: Copy, so it’s not the same.

@steffahn steffahn changed the title Vec::extend_from_slice forgot about the difference of T: Clone vs. T: Copy bounds. Vec::extend_from_slice’s documentation forgot about the difference of T: Clone vs. T: Copy bounds. May 17, 2022
@cuviper
Copy link
Member

cuviper commented May 17, 2022

FWIW, rust-lang/rfcs#839 debated whether Extend<&T> should require T: Clone or T: Copy, and the consensus was to start with Copy and consider Clone later. But I think that would be a breaking change to expand that blanket impl, because downstream impl Extend<&Foo> for Vec<Foo> is allowed if Foo is not Copy -- but it may be Clone.

@steffahn
Copy link
Member Author

steffahn commented May 17, 2022

Interesting discussion to read, thanks for the link. I agree with your observation that the supposedly backwards compatible change that it ended on would be breaking; at least since 1.41.0, before that release it was probably actually still backwards compatible edit.. nevermind it always was breaking, 1.41 just introduced to possibility to have generic arguments in the foreign Self type, too.

@jyn514 jyn514 added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-slice Area: [T] labels Apr 20, 2023
@jyn514
Copy link
Member

jyn514 commented Apr 20, 2023

I thought we have a policy that adding new impls isn't considered a breaking change? I guess relaxing a bound is technically not the same as adding an impl but it seems effectively the same ...

@cuviper
Copy link
Member

cuviper commented Apr 20, 2023

There's a difference between adding new impls for concrete types, which can't conflict downstream but may cause call ambiguity, versus adding or expanding a generic impl that could conflict with downstream impls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-slice Area: [T] T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants