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

Implement From<&'a [T; N]> for Cow<'a, [T]> #117604

Closed
wants to merge 1 commit into from

Conversation

zachs18
Copy link
Contributor

@zachs18 zachs18 commented Nov 5, 2023

(see also: #116890)

Currently, converting an array reference to a Cow<[T]> requires either slicing and reborrowing (Cow::from(&arr_ref[..])), or explicitly constructing a Cow::Borrowed, sometimes with a type annotation (playground link of some conversions that do and do not compile).

Note that this does not conflict with a possible future impl From<&'a U> for Cow<'_, U>, but could cause inference errors if coexisting with such an impl (where U = [T; N], Cow::from(array_ref) would be ambiguous if not otherwise constrained).

Needs FCP since this is an insta-stable impl.

@rustbot label +T-libs-api

r? @ghost

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 5, 2023
@scottmcm
Copy link
Member

scottmcm commented Nov 6, 2023

Note that inference regressions for things like Cow::from are tolerable, so you don't necessarily need a fresh test for it.

You could consider something like a top-10k-crates check-only crater run to see whether it's majorly impactful. Inference breaks from extra impls like this default to being allowed, actually -- otherwise we could hardly add anything -- and would only be blocked if the issues are particularly bad and particularly common. (The classic example being something like breaking simple code using literals.)

@zachs18
Copy link
Contributor Author

zachs18 commented Nov 11, 2023

Closing in favor of #117342 (which I hadn't seen at the time oops)

@zachs18 zachs18 closed this Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants