Skip to content

/bytes

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

"allocator" and "nightly" features (for no_std environments) #153

Closed
wants to merge 2 commits into from

Conversation

Reviewers

@tarcieri tarcieri

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@tonychain @jbowens @carllerche @Ericson2314 @tarcieri
@tonychain
Copy link

tonychain commented Jul 18, 2017 •
edited

This gates all allocator-dependent features on an "allocator" feature.

It also adds a "nightly" feature which enables using the "allocator" feature in no_std environments. This requires using #[feature(alloc)] which requires nightly.

The "allocator" feature is automatically enabled when either the "std" or "nightly" features are enabled.

Travis CI is configured to check that builds succeed with both the "nightly" feature along with "std" and "nightly" in combination. To avoid the problem of nightly changes breaking the build, these combinations are specifically flagged as allowed failures in the Travis CI configuration.

@tonychain tonychain referenced this pull request Jul 18, 2017

Closed

no_std support (using enabled-by-default "std" feature) #135

@tonychain

This comment has been minimized.

Sign in to view
Copy link
Author

tonychain commented Jul 18, 2017 •
edited

Note this PR includes a commit from the no_std PR, which should get merged first:

#135
@tarcieri

tarcieri reviewed Jul 18, 2017
View changes

src/lib.rs
@@ -70,8 +70,16 @@

#![deny(warnings, missing_docs, missing_debug_implementations)]
#![doc(html_root_url = "https://docs.rs/bytes/0.4")]
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(feature = "nightly", feature(alloc))]

This comment has been minimized.

Sign in to view
Copy link
@tarcieri

tarcieri Jul 18, 2017

@alexcrichton so really this is the part that requires nightly, and that's because even if I have an #[allocator] defined Box, Vec, and String do not get added to the prelude.

If they did, then we could simply have an enabled-by-default, non-nightly-dependent alloc cargo feature for gating features that depend on Box, Vec, and String.

I'm not sure how hard that would be, but I imagine it would be immensely useful for no_std users who have a heap available.

@tonychain tonychain force-pushed the chain:alloc branch 2 times, most recently from d0a74b1 to a57e18b Jul 18, 2017

@tonychain tonychain changed the title "nightly" feature (for using allocation in no_std environments) "allocator" and "nightly" features (for no_std environments) Jul 20, 2017

@tonychain tonychain force-pushed the chain:alloc branch from a57e18b to e456afb Jul 20, 2017

@tonychain

tonychain reviewed Jul 20, 2017
View changes

src/lib.rs
@@ -98,3 +110,15 @@ pub use byteorder::{ByteOrder, BigEndian, LittleEndian};
#[cfg(feature = "serde")]
#[doc(hidden)]
pub mod serde;

/// Custom (internal-only) prelude for this module

This comment has been minimized.

Sign in to view
Copy link
@tonychain

tonychain Jul 20, 2017

Author

I made this to avoid having to individually import each of these types everywhere they're used.

Seems like a bit of a hack, but dramatically reduces the scope of what the nightly feature does which I think is nice.

It also makes this easy to change in the event the liballoc/libcollections merge is reverted

@tonychain tonychain force-pushed the chain:alloc branch from e456afb to 041a0be Jul 20, 2017

tonychain added some commits Jun 16, 2017

@tonychain @jbowens
no_std support (using enabled-by-default "std" feature) 
- Changes all references to libcore features from ::std to ::core
- Feature gates anything dependent on std on the "std" feature
2c6b1d8
@tonychain @jbowens
"allocator" and "nightly" features (for no_std environments) 
This commit gates all allocator-dependent features on an "allocator" feature.

It also adds a "nightly" feature which enables using the "allocator" feature in
no_std environments. This requires using #[feature(alloc)] which requires
nightly.

The "allocator" feature is automatically enabled when either the "std" or
"nightly" features are enabled.

Travis CI is configured to check that builds succeed with both the "nightly"
feature along with "std" and "nightly" in combination. To avoid the problem
of nightly changes breaking the build, these combinations are specifically
flagged as allowed failures in the Travis CI configuration.
a90575c

@jbowens jbowens force-pushed the chain:alloc branch from 041a0be to a90575c Nov 14, 2017

@jbowens

This comment has been minimized.

Sign in to view
Copy link

jbowens commented Nov 14, 2017

Hey @carllerche. I'd love to see no_std for bytes. Is this pull request still the right direction?
@carllerche

This comment has been minimized.

Sign in to view
Copy link
Owner

carllerche commented Dec 14, 2017

Sorry to have left this without a response.

I don't think we want to add a dependency (even hidden behind a feature flag) on the allocator crate as it is highly unstable.

#135 is fine as an approach to support no_std but it does not include any types that require allocation.

I'm going to close this for now as #135 is the preferred path forward.

@carllerche carllerche closed this Dec 14, 2017

@tarcieri tarcieri referenced this pull request Feb 9, 2018

Merged

"alloc" and "nightly" features (for no_std environments) #4

@tarcieri tarcieri referenced this pull request Mar 15, 2019

Merged

RFC: Stabilize the alloc crate #2480

@Ericson2314

This comment has been minimized.

Sign in to view
Copy link

Ericson2314 commented Mar 15, 2019

So the argument behind rust-lang/rfcs#2480 is that while alloc is itself unstable, the items do not change much as std basically reexports them. So in practice is the a maintenance burden of adding optional features like this really that bad?
@tarcieri

This comment has been minimized.

Sign in to view
Copy link

tarcieri commented Mar 15, 2019

If rust-lang/rfcs#2480 lands, this will all work on stable. At that time I will happily revive this PR.
@Ericson2314

This comment has been minimized.

Sign in to view
Copy link

Ericson2314 commented Mar 15, 2019 •
edited

@tarcieri I am commenting here and there because I both want this PR and ones like it really badly and don't want to stabilize alloc just yet, and why I see no conflict between those two positions.
@carllerche

This comment has been minimized.

Sign in to view
Copy link
Owner

carllerche commented Mar 15, 2019

I have seen the stable build break due to adding nightly only features guarded with a feature flag.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.