Impl AsRef<Path> for Cow<str>

1

I noticed that we have the following impls:

impl AsRef<Path> for OsStr
impl AsRef<Path> for str
impl AsRef<Path> for Cow<OsStr>

but not

impl AsRef<Path> for Cow<str>

Do you folks think this impl would be useful? Admittedly it's a bit edge-casey, and users can always deref the Cow (to get a str) in cases where they want to pass a Cow<str> to functions which take AsRef<Path>.

However from a symmetry argument, we already implemented for Cow<OsStr>, and arguably Cow<str> is a more common type from what I've seen, so the implementation may save users a few characters here and there.

1 Like
2

Yeah, send a PR with the impl. Looks like an oversight.

1 Like
3

Hmm, actually looks like it's not that simple. Adding this impl can break code, as .as_ref() on Cow<str> suddenly takes on a second meaning. It's probably not worth trying to figure this out!

4

That kind of breakage is relatively allowed, though. There have been plenty of AsRef additions that have caused things like that.

5

I have a feeling that this one in particular is pretty common though, since getting a &str from a Cow<str> is a common operation (obviously), and this is one of the easier/more common ways to do it.

That's just to say, it might be worth doing a crater run as a smoke test.

6

:+1: on a potential crater run. I don't think method resolution otherwise has a way to distinguish between which should be called.

7

Yeah, I mean this really seems like it'll break a lot. If I clone the Rust repository, add this impl to the stdlib, and then try ./x.py test --stage 1

error[E0282]: type annotations needed
    --> /home/david/.cargo/registry/src/github.com-1ecc6299db9ec823/regex-1.4.6/src/re_unicode.rs:1247:14
     |
1247 |         self.as_ref().replace_append(caps, dst)
     |         -----^^^^^^--
     |         |    |
     |         |    cannot infer type for type parameter `T` declared on the trait `AsRef`
     |         this method call resolves to `&T`
     |
     = note: type must be known at this point

error[E0282]: type annotations needed
    --> /home/david/.cargo/registry/src/github.com-1ecc6299db9ec823/regex-1.4.6/src/re_unicode.rs:1257:14
     |
1257 |         self.as_ref().replace_append(caps, dst)
     |         -----^^^^^^--
     |         |    |
     |         |    cannot infer type for type parameter `T` declared on the trait `AsRef`
     |         this method call resolves to `&T`
     |
     = note: type must be known at this point

... I'm only a very occasional contributor to the compiler, so I don't even know how to continue with the compiler testing framework without patching the regex crate :joy:

8

You could probably add an inherent as_ref to Cow as a way to make it non-breaking.

Dunno if libs would be willing to go along with that, though.

9

^^ that's probably the only way to do it if we want this impl, short of special-casing the compiler.

1 Like
10

Agreed that inherent as_ref would be the way to go. Though I didn't notice prior art of an inherent as_ref anywhere else in the stdlib when I looked briefly earlier. I would argue that special-casing the compiler is undesirable - the array .into_iter() special case was much more worth the effort than this nice-to-have.

Not sure I'm invested enough in having this impl at this point! I'm going to leave this unimplemented for now, though I guess anyone who wants this impl in the future always has this thread as a good starting block.

11

I think that an inherent as_ref here would be a weird hack. Especially since the use-cases of as_ref for &Cow<'_, T> -> &T are covered by the Deref implementation of Cow. The two occurrences in re_unicode probably will work with the .as_ref() calls simply removed entirely (using the Deref for the method call then).

I think that aiming for a more general solution would be worth the effort of implementing such a special-casing mechanism. Ideally, if we could somehow make certain trait implementations take precedence over others for type inference (?) or method resolution (?) or whatever the most relevant mechanism at play here is, that’s a neat feature to improve the current situation of “allowed breakage” from adding trait impls in the standard library. Even for things less common than Cow<str>, we could avoid any potential for breakage entirely by somehow marking such new trait implementation as “newer” and hence “less important” an letting the compiler pretend it isn’t there in some contexts (i.e. in type inference or method resolution or where ever necessary). Similar to .into_iter(), we’d always eventually remove those special precedence rules over then next edition.

12

If we're getting an inherent method, wouldn't naming it as_path be better? That would at least match with the (presumably) &Path return type.

13

As far as I understand, the intention of the suggestion of an inherent method “as_ref” (particularly on Cow<str> taking &self, returning &str) was to avoid breakage in existing code. A different name isn’t an option, if that’s the goal. Notably, the inherent method was not meant for the conversion to &Path. The AsRef trait is commonly used as a trait bound on functions to make them less picky w.r.t. their argument type. An fn foo(x: impl AsRef<Path>) can take a &PathBuf or &str or String or an OsString or a &&&Cow<'_, OsStr>, etc. The change suggested here would allow such a method foo to not only accept Cow<OsStr> and Cow<Path> but also Cow<str>. One example of an existing such method is File::open.

2 Likes
14

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.