Implement From for more types on Cow #50170

burtonageo · 2018-04-22T22:00:07Z

This is basically #48191, except that it should be implemented in a way that doesn't break third party crates.

rust-highfive · 2018-04-22T22:00:10Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @KodrAus (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

KodrAus · 2018-04-25T21:20:26Z

Thanks @burtonageo! At first glance this looks good to me, there are a few more concrete implementations like From<Cow<'a, T>> for T::Owned that we could add here too:

From<Cow<'a, Path>> for PathBuf
From<Cow<'a, CStr>> for CString
From<Cow<'a, OsStr>> for OsString

These probably aren't strictly necessary, but the From<Cow<'a, [T]>> for Vec<T> was also only added for consistency with the String implementation.

burtonageo · 2018-04-27T19:43:31Z

pathbuf_from_cow_path, cstring_from_cow_cstr and osstring_from_cow_osstr all added

KodrAus · 2018-05-01T23:31:30Z

Thanks @burtonageo! This looks good to me.

@rfcbot fcp merge

These conversions are insta-stable, but I don't think they're contentious. We now have a pattern with our From conversions for Cow between T and T::Owned:

impl<'a, T> From<Cow<'a, T>> for T::Owned
impl<'a, T> From<&'a T> for Cow<'a, T> 
impl<'a, T> From<T::Owned> for Cow<'a, T> 
impl<'a, T> From<&'a T::Owned> for Cow<'a, T>

This brings our set of From conversions to:

  impl<'a, T> From<Cow<'a, [T]>> for Vec<T>
  impl<'a, T> From<&'a [T]> for Cow<'a, [T]> 
  impl<'a, T> From<Vec<T>> for Cow<'a, [T]> 
+ impl<'a, T> From<&'a Vec<T>> for Cow<'a, [T]>

  impl<'a> From<Cow<'a, str>> for String
  impl<'a> From<&'a str> for Cow<'a, str>
  impl<'a> From<String> for Cow<'a, str>
+ impl<'a> From<&'a String> for Cow<'a, str>

+ impl<'a> From<Cow<'a, Path>> for PathBuf
  impl<'a> From<&'a Path> for Cow<'a, Path>
  impl<'a> From<PathBuf> for Cow<'a, Path>
+ impl<'a> From<&'a PathBuf> for Cow<'a, Path>

+ impl<'a> From<Cow<'a, CStr>> for CString
+ impl<'a> From<&'a CStr> for Cow<'a, CStr>
+ impl<'a> From<CString> for Cow<'a, CStr>
+ impl<'a> From<&'a CString> for Cow<'a, CStr>

+ impl<'a> From<Cow<'a, OsStr>> for OsString
+ impl<'a> From<&'a OsStr> for Cow<'a, OsStr>
+ impl<'a> From<OsString> for Cow<'a, OsStr>
+ impl<'a> From<&'a OsString> for Cow<'a, OsStr>

KodrAus · 2018-05-06T06:57:54Z

@rfcbot fcp merge

KodrAus · 2018-05-06T06:59:27Z

One more time...

@rfcbot fcp merge

KodrAus · 2018-05-06T07:02:32Z

@rfcbot we are no longer friends.

I could just r+ this but figured I'd use rfcbot since these impls won't have an unstable period.

cc @rust-lang/libs

SimonSapin · 2018-05-06T07:37:42Z

@rfcbot fcp merge

rfcbot · 2018-05-06T07:37:42Z

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

kennytm · 2018-05-08T21:14:26Z

src/liballoc/string.rs

@@ -2239,6 +2239,14 @@ impl<'a> From<String> for Cow<'a, str> {
    }
 }

+#[stable(feature = "cow_from_string_ref", since = "1.27.0")]


These should all become 1.28.0

burtonageo · 2018-05-11T13:06:27Z

@KodrAus Can this now enter final comment period?

kennytm · 2018-05-11T19:09:55Z

@burtonageo

It needs one more vote from either @Kimundi, @aturon or @withoutboats.

TimNN · 2018-05-15T17:08:55Z

Ping, @Kimundi, @aturon or @withoutboats: This PR has an open FCP request that needs your input.

rfcbot · 2018-05-16T17:37:46Z

🔔 This is now entering its final comment period, as per the review above. 🔔

alexcrichton · 2018-05-16T20:18:12Z

@bors: r+

bors · 2018-05-16T20:18:13Z

📌 Commit 7c0f664 has been approved by alexcrichton

…chton Implement From for more types on Cow This is basically rust-lang#48191, except that it should be implemented in a way that doesn't break third party crates.

Rollup of 17 pull requests Successful merges: - #50170 (Implement From for more types on Cow) - #50638 (Don't unconditionally set CLOEXEC twice on every fd we open on Linux) - #50656 (Fix `fn main() -> impl Trait` for non-`Termination` trait) - #50669 (rustdoc: deprecate `#![doc(passes, plugins, no_default_passes)]`) - #50726 (read2: Use inner function instead of closure) - #50728 (Fix rustdoc panic with `impl Trait` in type parameters) - #50736 (env: remove unwrap in examples in favor of try op) - #50740 (Remove LazyBTreeMap.) - #50752 (Add missing error codes in libsyntax-ext asm) - #50779 (Make mutable_noalias and arg_align_attributes be tracked) - #50787 (Fix run-make wasm tests) - #50788 (Fix an ICE when casting a nonexistent const) - #50789 (Ensure libraries built in stage0 have unique metadata) - #50793 (tidy: Add a check for empty UI test files) - #50797 (fix a typo in signed-integer::from_str_radix()) - #50808 (Stabilize num::NonZeroU*) - #50809 (GitHub: Stop treating Cargo.lock as a generated file.) Failed merges:

oli-obk · 2018-06-28T06:45:03Z

This PR did still cause breakage: #51844

Implement From for more types on Cow

1133a14

rust-highfive assigned KodrAus Apr 22, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 22, 2018

burtonageo added 2 commits April 27, 2018 20:27

Add cstring_from_cow_cstr and osstring_from_cow_osstr

ea8131d

Add pathbuf_from_cow_path

d87b039

Update the stable attributes to use the current nightly version number

f3e858a

KodrAus added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label May 6, 2018

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 6, 2018

kennytm reviewed May 8, 2018

View reviewed changes

burtonageo added 2 commits May 9, 2018 07:23

Update features to 1.28.0

17e2628

Fix typo

7c0f664

TimNN added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 15, 2018

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels May 16, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels May 16, 2018

kennytm added the relnotes Marks issues that should be documented in the release notes of the next release. label May 16, 2018

kennytm mentioned this pull request May 16, 2018

Rollup of 17 pull requests #50807

Merged

bors merged commit 7c0f664 into rust-lang:master May 17, 2018

burtonageo deleted the more_cow_from branch May 17, 2018 04:56

leoyvens mentioned this pull request May 22, 2018

cannot resolve std::borrow::Cow<'_, str>: std::borrow::Borrow<_> #50954

Closed

rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 26, 2018

Mark-Simulacrum mentioned this pull request Jun 27, 2018

"type annotations required" errors in 1.28 beta #51844

Closed

7 tasks

jonas-schievink mentioned this pull request Oct 16, 2018

Type inference regression in Rust 1.28.0 #55129

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement From for more types on Cow #50170

Implement From for more types on Cow #50170

burtonageo commented Apr 22, 2018

rust-highfive commented Apr 22, 2018

KodrAus commented Apr 25, 2018

burtonageo commented Apr 27, 2018

KodrAus commented May 1, 2018

KodrAus commented May 6, 2018

KodrAus commented May 6, 2018

KodrAus commented May 6, 2018

SimonSapin commented May 6, 2018

rfcbot commented May 6, 2018 •

edited by Kimundi

Loading

kennytm May 8, 2018

burtonageo commented May 11, 2018

kennytm commented May 11, 2018

TimNN commented May 15, 2018

rfcbot commented May 16, 2018

alexcrichton commented May 16, 2018

bors commented May 16, 2018

oli-obk commented Jun 28, 2018

Implement From for more types on Cow #50170

Implement From for more types on Cow #50170

Conversation

burtonageo commented Apr 22, 2018

rust-highfive commented Apr 22, 2018

KodrAus commented Apr 25, 2018

burtonageo commented Apr 27, 2018

KodrAus commented May 1, 2018

KodrAus commented May 6, 2018

KodrAus commented May 6, 2018

KodrAus commented May 6, 2018

SimonSapin commented May 6, 2018

rfcbot commented May 6, 2018 • edited by Kimundi Loading

kennytm May 8, 2018

Choose a reason for hiding this comment

burtonageo commented May 11, 2018

kennytm commented May 11, 2018

TimNN commented May 15, 2018

rfcbot commented May 16, 2018

alexcrichton commented May 16, 2018

bors commented May 16, 2018

oli-obk commented Jun 28, 2018

rfcbot commented May 6, 2018 •

edited by Kimundi

Loading