Make signature of Path::strip_prefix accept non-references #48989
Conversation
BREAKING CHANGE: This has the potential to cause regressions in type inference.
|
r? @dtolnay (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
src/libstd/path.rs
Outdated
| /// ``` | ||
| #[stable(since = "1.7.0", feature = "path_strip_prefix")] | ||
| pub fn strip_prefix<'a, P: ?Sized>(&'a self, base: &'a P) | ||
| -> Result<&'a Path, StripPrefixError> | ||
| pub fn strip_prefix<'a, P>(&'a self, base: P) |
There was a problem hiding this comment.
I believe the 'a lifetime can now be elided.
There was a problem hiding this comment.
Oops, thanks for catching this!
src/libstd/path.rs
Outdated
| where P: AsRef<Path> | ||
| { | ||
| self._strip_prefix(base.as_ref()) | ||
| } | ||
|
|
||
| fn _strip_prefix<'a>(&'a self, base: &'a Path) | ||
| fn _strip_prefix<'a>(&'a self, base: &Path) |
There was a problem hiding this comment.
Same here, I believe 'a can be elided.
dtolnay
added
the
T-libs-api
|
I would be on board with attempting to land this. The original signature is indeed strange, and the way impl Path {
- pub fn strip_prefix<'a, P: ?Sized>(&'a self, base: &'a P) -> Result<&'a Path, StripPrefixError>
+ pub fn strip_prefix<P>(&self, base: P) -> Result<&Path, StripPrefixError>
where P: AsRef<Path>;
}@rfcbot fcp merge |
|
Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged teams: Concerns: 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. |
rfcbot
added
the
proposed-final-comment-period
This sounds like “let’s do a crater run”. What’s the process for that? |
|
@rfcbot concern crater |
shepmaster
added
S-waiting-on-team
pietroalbini
added
S-waiting-on-crater
|
Added this to the crater queue. |
|
@bors try |
|
⌛ Trying commit 7cbc93b with merge df2fbb30d918e67865a57dbf24fd35a01fdc5d39... |
|
☀️ Test successful - status-travis |
|
Crater run started, ETA is ~3 days. |
|
Crater results all look like spurious failures. http://cargobomb-reports.s3.amazonaws.com/pr-48989/index.html |
|
@bors r+ |
|
📌 Commit 7cbc93b has been approved by |
bors
added
S-waiting-on-bors
Make signature of Path::strip_prefix accept non-references I did this a while back but didn't submit a PR. Might as well see what happens. Fixes #48390. **Note: This has the potential to cause regressions in type inference.** However, in order for code to break, it would need to be relying on the signature to determine that a type is `&_`, while still being able to figure out what the `_` is. I'm having a hard time imagining such a scenario in real code.
|
☀️ Test successful - status-appveyor, status-travis |
rfcbot
removed
the
proposed-final-comment-period
I did this a while back but didn't submit a PR. Might as well see what happens.
Fixes #48390.
Note: This has the potential to cause regressions in type inference. However, in order for code to break, it would need to be relying on the signature to determine that a type is
&_, while still being able to figure out what the_is. I'm having a hard time imagining such a scenario in real code.