Skip to content

Allow API functions to take AsRef<str> instead of &str #175

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gavincrawford
Copy link

@gavincrawford gavincrawford commented Aug 14, 2025

Generalizes the outward facing API and allows String to be passed to functions that were previously &str exclusive.

We could also go ahead and apply this to the Path arguments, but I wasn't sure if that's in scope for a single PR, or if we'd prefer to split that up.

@BurntSushi
Copy link
Member

I have resisted making a similar change to regex for Regex::new, and similarly for its matching routines. I would recommend avoiding the generic parameter here primarily so that type inference always has a concrete target when these routines are used. Otherwise, type inference can fail. (In fact, for that reason, I wouldn't be surprised if this were too big of a breaking change to be put into a semver compatible release.)

@gavincrawford
Copy link
Author

Fair point. I feel that the type inference issues caused here might mostly be trivial, but were there specific cases with regex that caused large problems elsewhere? Nevertheless, you're right, this could result in breaking changes somewhere.

@BurntSushi
Copy link
Member

A type inference issue caused by this change is nearly definitionally trivial. It's almost certainly always easy to work around by turbo fishing or adding a type annotation somewhere or fully qualifying something. The point is that it's a paper cut and that you don't get a ton of benefit from it. Conversely, with the status quo, you'll never have a type inference issue (because there are no generics). The status quo does mean you may sometimes need to insert a & or maybe an .as_str() or something, but even with AsRef<str> you might need this. e.g., You typically don't want to pass ownership of a String into a routine like this, even though this sort of generic bound allows it.

It really doesn't take a lot to get cut by type inference, e.g., rust-lang/regex#811 (comment)

Obviously in many cases a generic bound pays dividends. But I don't think it gives you much here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants