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

Add experimental feature flagged no_std support #28

Closed
wants to merge 4 commits into from

Conversation

ZackPierce
Copy link

What

The addition of feature flags, optional dependencies, and conditional compilation blocks to make aho-corasick work in either std or no_std + alloc environments.

The default-on "use_std" feature flag controls whether or not to compile in #![no_std] mode.

The alloc feature flag must be specified and the toolchain fixed to nightly-2018-03-07 for the no_std experimental mode to operate, due to the rather unpleasant pinning of core_io to particular versions.

How

To build for std, nothing changes. cargo build

To build for no_std:

cargo build --no-default-features --features "alloc"

The test suites pass as normal in either mode, with the exception of one which relies on HashSet, which is not readily available in alloc, and one doc-test that makes use of the filesystem.

Why

This PR is part of a thought experiment regarding what it will take to get regex and its dependencies to operate in no_std + alloc mode. In particular, it is aimed to prompt discussion about whether the general approach is acceptable and feel out options for the "io in no_std" situation. The option used in this experiment thus far, https://github.com/jethrogb/rust-core_io , is thoroughly unofficial and, as mentioned, is both unfortunately version-pinned and somewhat annoying to interoperate.

The default use_std feature flag controls whether or
not to compile in #![no_std] mode.

The alloc feature flag must be specified and the
toolchain fixed to nightly-2018-03-07 for the no_std
experimental mode to operate, due to the core::io
handling.
@BurntSushi
Copy link
Owner

Ah this is pretty cool! Another possibility here is to just not provide the streaming search routines for no_std. At least, I think that would be necessary to merge this in the short term, since I don't think I have the bandwidth to take on a maintenance burden that requires nightly pinning at the moment. :-(

Also, we'll want to keep rust-lang/regex#474 in mind when coming up with the name of the feature here. I wish I just used std in regex. Right now, I'm leaning towards just deprecating use_std and adding std.

@ZackPierce
Copy link
Author

Sure, if restricting the availability of std::io related methods/types in no_std mode is an acceptable option, I'm cool with taking that route. I guess the open question there is whether regex will be able to effectively operate without leveraging said capabilities.

At very least it might provide a low-maintenance route forward until the portability working group (see rust-lang-nursery/portability-wg#12 ) or some other interested party following rust-lang/rfcs#2262 makes progress on refactoring std/core io for better modularity.

@BurntSushi
Copy link
Owner

Yeah regex certainly does not need the io::Read APIs. Not yet, anyway. :)

@ZackPierce ZackPierce force-pushed the no_std_feature_flag branch from 327324e to 312a124 Compare May 3, 2018 02:31
@ZackPierce
Copy link
Author

@BurntSushi

Updated to change the name of the feature flag to "std" and to exclude the std::io-dependent streaming portions of the API when building without the "std" flag.

This allows the removal of the strange nightly pinning dependency, and lets us add the previously-incompatible hashmap_core crate as a dev dependency to enable better quickchecking in no_std mode.

@ZackPierce
Copy link
Author

... and I've taken hashmap_core back out again, because I had forgotten that it is nightly-only, and dev-dependencies currently can't be optional, which caused the non-nightly builds to fail previously.

The library documentation has also been updated to include a basic no_std usage explanation.

@BurntSushi
Copy link
Owner

@ZackPierce I'm going to close this out. While in principle I support the idea of a no_std mode for this crate, I'm not comfortable at this point in time depending on nightly only APIs. I'd rather wait until the alloc story is more solid.

Additionally, I should be working on #34 soon, which will likely involve a rebuild of much of this crate from the ground up. While doing that, I'll try to think more carefully about the no_std use case and see whether I can accommodate it better.

@BurntSushi BurntSushi closed this Oct 7, 2018
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