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

string: implement From<&String> for String #59825

Merged
merged 1 commit into from
May 16, 2019
Merged

Conversation

jsgf
Copy link
Contributor

@jsgf jsgf commented Apr 9, 2019

Allow Strings to be created from borrowed Strings. This is mostly
to make things like passing &String to an impl Into<String>
parameter frictionless.

Fixes #59827.

@rust-highfive
Copy link
Collaborator

r? @kennytm

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

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

This comment has been minimized.

@jonas-schievink jonas-schievink added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 9, 2019
@kennytm kennytm added the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Apr 9, 2019
@@ -2179,6 +2179,14 @@ impl From<&str> for String {
}
}

#[unstable(feature="from_ref_string", issue="59827")]
Copy link
Member

Choose a reason for hiding this comment

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

new impls are insta-stable, unfortunately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, OK. What version should I put then?

Copy link
Member

Choose a reason for hiding this comment

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

1.36.0. I don't think this PR could be merged in a day (before the coming release )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.34 would be ambitious yes; I guessed .35, but .36 is fine too.

Allow Strings to be created from borrowed Strings. This is mostly
to make things like passing &String to an `impl Into<String>`
parameter frictionless.
@Centril Centril added the relnotes Marks issues that should be documented in the release notes of the next release. label Apr 10, 2019
@WiSaGaN
Copy link
Contributor

WiSaGaN commented Apr 10, 2019

What is so special about String? Compare to things like OsString or even all owned types? Should they all implement From<&T> for T?

@meithecatte
Copy link
Contributor

I'm not familiar with the ffi types, but the special thing about String is that it can Deref to str, which already implements From for String. This makes &string and &string[..] have the same semantics most of the time, but not always.

@WiSaGaN
Copy link
Contributor

WiSaGaN commented Apr 10, 2019

@NieDzejkob do you mean something similar to impl AsRef< OsStr> for OsString and impl<'a, T: ?Sized + AsRef< OsStr>> From<&'a T> for OsString ?

@jsgf
Copy link
Contributor Author

jsgf commented Apr 10, 2019

@WiSaGaN I agree, there's nothing particularly special about String other than it was the case I ran into. It would make equal sense for a lot of types (and if we had specialization I might suggest a blanket default impl<T: Clone> From<&T> for T, but that might be going too far).

@Dylan-DPC-zz
Copy link

ping from triage @jsgf @kennytm any updates on this?

@jsgf
Copy link
Contributor Author

jsgf commented Apr 22, 2019

@Dylan-DPC I wasn't planning any changes. I'm happy to do similar implementations for Vec and/or OsString if that would unblock it, but it's not clear to me that those missing are blocking objections.

@WiSaGaN
Copy link
Contributor

WiSaGaN commented Apr 23, 2019

I am wondering whether libs team have thought about implementing impl<T: Clone> From<&T> for T before. If we did, and the decision was not to, then what has changed since?

@Centril
Copy link
Contributor

Centril commented May 11, 2019

r? @sfackler

@rust-highfive rust-highfive assigned sfackler and unassigned kennytm May 11, 2019
@sfackler
Copy link
Member

I've definitely run into this impl missing, so I'm in favor!

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented May 11, 2019

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

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), 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.

@rfcbot 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 11, 2019
@Centril Centril removed the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label May 11, 2019
@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label May 15, 2019
@rfcbot
Copy link

rfcbot commented May 15, 2019

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

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label May 15, 2019
@sfackler
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 15, 2019

📌 Commit 08b0aca has been approved by sfackler

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels May 15, 2019
@Centril
Copy link
Contributor

Centril commented May 15, 2019

@bors rollup

Centril added a commit to Centril/rust that referenced this pull request May 15, 2019
string: implement From<&String> for String

Allow Strings to be created from borrowed Strings. This is mostly
to make things like passing `&String` to an `impl Into<String>`
parameter frictionless.

Fixes rust-lang#59827.
bors added a commit that referenced this pull request May 15, 2019
Rollup of 9 pull requests

Successful merges:

 - #59825 (string: implement From<&String> for String)
 - #59923 (Fix convert module's documentation links)
 - #60691 (Include expression to wait for to the span of Await)
 - #60763 (Move token tree related lexer state to a separate struct)
 - #60769 (Update rustc book CLI docs.)
 - #60811 (Bump measureme dependency to 0.3)
 - #60816 (README.md: Mention MSVC 2017+, not 2013(!))
 - #60827 (Use `Symbol` more in lint APIs)
 - #60851 (Move `box` from the stable keyword to unstable keywords list)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request May 15, 2019
string: implement From<&String> for String

Allow Strings to be created from borrowed Strings. This is mostly
to make things like passing `&String` to an `impl Into<String>`
parameter frictionless.

Fixes rust-lang#59827.
Centril added a commit to Centril/rust that referenced this pull request May 16, 2019
string: implement From<&String> for String

Allow Strings to be created from borrowed Strings. This is mostly
to make things like passing `&String` to an `impl Into<String>`
parameter frictionless.

Fixes rust-lang#59827.
bors added a commit that referenced this pull request May 16, 2019
Rollup of 6 pull requests

Successful merges:

 - #59825 (string: implement From<&String> for String)
 - #59923 (Fix convert module's documentation links)
 - #60691 (Include expression to wait for to the span of Await)
 - #60769 (Update rustc book CLI docs.)
 - #60816 (README.md: Mention MSVC 2017+, not 2013(!))
 - #60851 (Move `box` from the stable keyword to unstable keywords list)

Failed merges:

r? @ghost
@bors bors merged commit 08b0aca into rust-lang:master May 16, 2019
@rfcbot 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 25, 2019
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jul 5, 2019
Pkgsrc changes:
 * NetBSD/sparc64 disabling of "packed" removed ("packed" removed upstream)
 * Adapt src_libstd_build.rs patch, update sed'ing of .cargo-checksum.json

Build verified on NetBSD 8.0/amd64.

Upstream changes:

Version 1.36.0 (2019-07-04)
==========================

Language
--------
- [Non-Lexical Lifetimes are now enabled on the 2015 edition.][59114]
- [The order of traits in trait objects no longer affects the semantics of that
  object.][59445] e.g. `dyn Send + fmt::Debug` is now equivalent to
  `dyn fmt::Debug + Send`, where this was previously not the case.

Libraries
---------
- [`HashMap`'s implementation has been replaced with `hashbrown::HashMap` implem
entation.][58623]
- [`TryFromSliceError` now implements `From<Infallible>`.][60318]
- [`mem::needs_drop` is now available as a const fn.][60364]
- [`alloc::Layout::from_size_align_unchecked` is now available as a const fn.][6
0370]
- [`String` now implements `BorrowMut<str>`.][60404]
- [`io::Cursor` now implements `Default`.][60234]
- [Both `NonNull::{dangling, cast}` are now const fns.][60244]
- [The `alloc` crate is now stable.][59675] `alloc` allows you to use a subset
  of `std` (e.g. `Vec`, `Box`, `Arc`) in `#![no_std]` environments if the
  environment has access to heap memory allocation.
- [`String` now implements `From<&String>`.][59825]
- [You can now pass multiple arguments to the `dbg!` macro.][59826] `dbg!` will
  return a tuple of each argument when there is multiple arguments.
- [`Result::{is_err, is_ok}` are now `#[must_use]` and will produce a warning if
  not used.][59648]

Stabilized APIs
---------------
- [`VecDeque::rotate_left`]
- [`VecDeque::rotate_right`]
- [`Iterator::copied`]
- [`io::IoSlice`]
- [`io::IoSliceMut`]
- [`Read::read_vectored`]
- [`Write::write_vectored`]
- [`str::as_mut_ptr`]
- [`mem::MaybeUninit`]
- [`pointer::align_offset`]
- [`future::Future`]
- [`task::Context`]
- [`task::RawWaker`]
- [`task::RawWakerVTable`]
- [`task::Waker`]
- [`task::Poll`]

Cargo
-----
- [Cargo will now produce an error if you attempt to use the name of a required
dependency as a feature.][cargo/6860]
- [You can now pass the `--offline` flag to run cargo without accessing the netw
ork.][cargo/6934]

You can find further change's in [Cargo's 1.36.0 release notes][cargo-1-36-0].

Clippy
------
There have been numerous additions and fixes to clippy, see [Clippy's 1.36.0 rel
ease notes][clippy-1-36-0] for more details.

Misc
----

Compatibility Notes
-------------------
- [`std::arch::x86::_rdtsc` returns `u64` instead of `i64`][stdsimd/559]
- [`std::arch::x86_64::_mm_shuffle_ps` takes an `i32` instead of `u32` for `mask
`][stdsimd/522]
- With the stabilisation of `mem::MaybeUninit`, `mem::uninitialized` use is no
  longer recommended, and will be deprecated in 1.38.0.

[60318]: rust-lang/rust#60318
[60364]: rust-lang/rust#60364
[60370]: rust-lang/rust#60370
[60404]: rust-lang/rust#60404
[60234]: rust-lang/rust#60234
[60244]: rust-lang/rust#60244
[58623]: rust-lang/rust#58623
[59648]: rust-lang/rust#59648
[59675]: rust-lang/rust#59675
[59825]: rust-lang/rust#59825
[59826]: rust-lang/rust#59826
[59445]: rust-lang/rust#59445
[59114]: rust-lang/rust#59114
[cargo/6860]: rust-lang/cargo#6860
[cargo/6934]: rust-lang/cargo#6934
[`VecDeque::rotate_left`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.rotate_left
[`VecDeque::rotate_right`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.rotate_right
[`Iterator::copied`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#tymethod.copied
[`io::IoSlice`]: https://doc.rust-lang.org/std/io/struct.IoSlice.html
[`io::IoSliceMut`]: https://doc.rust-lang.org/std/io/struct.IoSliceMut.html
[`Read::read_vectored`]: https://doc.rust-lang.org/std/io/trait.Read.html#method.read_vectored
[`Write::write_vectored`]: https://doc.rust-lang.org/std/io/trait.Write.html#method.write_vectored
[`str::as_mut_ptr`]: https://doc.rust-lang.org/std/primitive.str.html#method.as_mut_ptr
[`mem::MaybeUninit`]: https://doc.rust-lang.org/std/mem/union.MaybeUninit.html
[`pointer::align_offset`]: https://doc.rust-lang.org/std/primitive.pointer.html#method.align_offset
[`future::Future`]: https://doc.rust-lang.org/std/future/trait.Future.html
[`task::Context`]: https://doc.rust-lang.org/beta/std/task/struct.Context.html
[`task::RawWaker`]: https://doc.rust-lang.org/beta/std/task/struct.RawWaker.html
[`task::RawWakerVTable`]: https://doc.rust-lang.org/beta/std/task/struct.RawWakerVTable.html
[`task::Waker`]: https://doc.rust-lang.org/beta/std/task/struct.Waker.html
[`task::Poll`]: https://doc.rust-lang.org/beta/std/task/enum.Poll.html
[clippy-1-36-0]: https://github.com/rust-lang/rust-clippy/blob/master/CHANGELOG.md#rust-136
[cargo-1-36-0]: https://github.com/rust-lang/cargo/blob/master/CHANGELOG.md#cargo-136-2019-07-04
[stdsimd/522]: rust-lang/stdarch#522
[stdsimd/559]: rust-lang/stdarch#559
Erk- added a commit to Erk-/lavalink.rs that referenced this pull request Jul 13, 2019
This change is made because of a breaking change
in rustc: rust-lang/rust#59825

Signed-off-by: Valdemar Erk <v@erk.io>
@Centril Centril added this to the 1.36 milestone Jul 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

Add impl From<&String> for String