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

Remove "perf" and "unicode" from default-features #721

Closed
link2xt opened this issue Oct 18, 2020 · 1 comment
Closed

Remove "perf" and "unicode" from default-features #721

link2xt opened this issue Oct 18, 2020 · 1 comment
Labels

Comments

@link2xt
Copy link

link2xt commented Oct 18, 2020

README.md explains that one can disable default features to reduce binary size, but it is practically impossible with large dependency tree. As long as one of the crates includes regex crate without specifying default-features = false, perf feature is enabled. To disable it, you have to go to each dependency maintainer and convince them to disable default features.

For such a widely used library it makes sense to disable these features by default and let application maintainers enable it if they want. Some libraries may also enable "unicode", but only if they really depend on it.

@BurntSushi
Copy link
Member

BurntSushi commented Oct 18, 2020

The problem you point out is certainly real and I knew it would be a problem when I added the features about a year ago. While you've provided a solution to the problem, you have not provided any argument for why the downsides of your solution are better than the downsides of the status quo. In particular, if I did what you're suggesting, then:

  1. It would be a breaking change, and thus require a regex 2 release.
  2. In this hypothetical regex 2 release, regexes like \w would fail to compile in the default configuration. Specifically, from the docs: "Stated differently, enabling or disabling any of the features below can only add or subtract from the total set of valid regular expressions. Enabling or disabling a feature will never modify the match semantics of a regular expression." One can only imagine how confused users would be, and I can empathized with their frustration as they seek to figure out how to fix it. (By re-enabling the features.) Suffice to say, having Regex::new(r"\w").unwrap() fail in the default configuration of this crate seems bonkers to me. Thus, we either need to keep these features enabled, or make the decision that Unicode should be disabled by default in regex 2, which would be a major departure from the existing design.
  3. Disabling these features is primarily geared towards reducing binary size and compilation time, at the expense of performance and correctness. IMO, and as the maintainer of this crate, I generally prioritize both performance and correctness over binary size and compilation time. With that said, where possible, being able to tweak that trade off is a nice convenience to offer. Hence why these crate features exist. That they can be difficult to disable in a large dependency tree is an unfortunate circumstance, but I don't really see another way.

To disable it, you have to go to each dependency maintainer and convince them to disable default features.

Yup, exactly. If a library crate you're using depends on regex and doesn't need or want these extra features, then it should be treated as a bug (or possibly feature enhancement), just like anything else. Sorry, but that's just the way the cookie crumbles in the current Cargo feature system.

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

No branches or pull requests

2 participants