Skip to content

The signature of Path::strip_prefix is bizarrely overconstrained #48390

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

Closed
ExpHP opened this issue Feb 21, 2018 · 4 comments
Closed

The signature of Path::strip_prefix is bizarrely overconstrained #48390

ExpHP opened this issue Feb 21, 2018 · 4 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@ExpHP
Copy link
Contributor

ExpHP commented Feb 21, 2018

pub fn strip_prefix<'a, P: ?Sized>(
    &'a self, 
    base: &'a P  // <===  ???  a reference?
) -> Result<&'a Path, StripPrefixError> 
where P: AsRef<Path>, 

This can clearly be generalized to

pub fn strip_prefix<'a, P>(
    &'a self, 
    base: P
) -> Result<&'a Path, StripPrefixError> 
where P: AsRef<Path>, 

Implementation.

(Note: Like many similar attempts to "generalize" function signatures, this is a potentially breaking change due to potential regressions in type inference)

@vitalyd
Copy link

vitalyd commented Feb 21, 2018

This would be a breaking change since you can’t pass ?Sized types anymore (and that’s likely the reason for having it a reference to begin with). It’s debatable whether such impls exist for this case, but that would need to be answered first before considering this change IMO.

@ExpHP
Copy link
Contributor Author

ExpHP commented Feb 21, 2018

@vitalyd: You've said a similar thing before on users, but I still don't see the argument.

Looking back at the thread now I see you responded to my request for clarification:

vitalyd wrote:
The contrived example is if some code you don’t control gives you an R: Read + ?Sized based &mut R - say that’s all you have when they call into your code. Whether they should arrange things differently is a different question. But as I mentioned, that’s pretty contrived. Your assumption (fairly valid but not required) is that you’re starting with a place where you can form a &mut R yourself and pass that to a fn<R: Read>(r: R).

But even with this clarification I cannot see the issue. It seems to me I can call an fn(R) where R: Read just fine with a &mut R where R: Read + ?Sized.

fn foo<R: Read>(r: R) { }

fn bar<R: Read + ?Sized>(r: &mut R) { foo(r) }  // ok

Notice I did not form the &mut R myself.

@ExpHP
Copy link
Contributor Author

ExpHP commented Feb 21, 2018

Okay. Wait. I see something. It's an insanely subtle distinction, but it's there:

Only the &mut R form will implicitly reborrow.

fn foo1<R: Read + ?Sized>(r: &mut R) { }
fn foo2<R: Read>(r: R) { }

fn bar<R: Read + ?Sized>(r: &mut R) { foo1(r); foo1(r); } // okay
fn bar<R: Read + ?Sized>(r: &mut R) { foo2(r); foo2(r); } // Error: `r` was moved

...but this doesn't apply to strip_prefix, which takes an immutable borrow.

@vitalyd
Copy link

vitalyd commented Feb 21, 2018

Right - for the Read conversation in that thread, reborrowing would be one concern (this nicety doesn't apply in a fully generic context).

Thinking about the strip_prefix case a bit more, I'm failing to come up with anything that will break. It seems like removing ?Sized could break some weird concoction, but I can't actually think of anything after all. The thing that would spook me though, as we've touched on in the forum, is type inference gone bad. Not sure about that one though.

@pietroalbini pietroalbini added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Feb 27, 2018
bors added a commit that referenced this issue Apr 24, 2018
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. 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