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 no_std + alloc environment support take 2 #1479

Merged
merged 3 commits into from
Mar 20, 2019

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Mar 18, 2019

This replaces #1438.

This allows to use Box<dyn Future>, many *Ext traits's methods etc... in the no_std + alloc environment.

  • This adds the alloc features to futures, futures-core, futures-sink, and futures-util.
  • When using the alloc feature, the nightly feature must be used at the same time.

cc @Nemo157

@taiki-e taiki-e force-pushed the alloc2 branch 2 times, most recently from cb135f7 to 88ce34b Compare March 19, 2019 07:14
#[cfg(all(feature = "alloc", not(feature = "std")))]
extern crate alloc;
#[cfg(feature = "std")]
extern crate std as alloc;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarifying: because the portions of std that appear in alloc use an identical API, this declaration makes it possible for everything to compile on stable when the alloc feature is disabled but the std feature is enabled, or when both features are enabled (in which case the alloc crate won't be used, only the std crate).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

@@ -1,2 +1,12 @@
#[macro_use]
mod poll;

macro_rules! cfg_target_has_atomic {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a bunch of spots in e.g. futures-util/src/stream/mod.rs that seem like they could benefit greatly from this-- or does that not work out because they're trait items?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-- or does that not work out because they're trait items?

Yeah, it doesn't work well with trait items... playground

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we should be able to work around this problem by using procedural macros, but in that case, I would like to compare with the approach using proc_macro_attribute.

  1. proc_macro
// definition
#[proc_macro]
pub fn cfg_target_has_atomic(input: TokenStream) -> TokenStream {
    let mut file: File = parse_macro_input!(input);
    let attr = parse_quote! {
       #[cfg_attr(
            feature = "cfg-target-has-atomic",
            cfg(all(target_has_atomic = "cas", target_has_atomic = "ptr"))
        )]
    };
    file.items.iter_mut().for_each(|item| {
        match item {
            Item::Fn(ItemFn { attrs, .. }) 
            | Item::Use(ItemUse { attrs, .. }) 
            | Item::Mod(ItemMod { attrs, .. }) => attrs.push(attr.clone()),
            _ => {}        
        }
    });
    TokenStream::from(file.into_token_stream())
}

// usage
cfg_target_has_atomic! {
    #[cfg(feature = "alloc")]
    mod buffer_unordered;
    #[cfg(feature = "alloc")]
    pub use self::buffer_unordered::BufferUnordered;
}
  1. proc_macro_attribute
// definition
#[proc_macro_attribute]
pub fn cfg_target_has_atomic(_args: TokenStream, input: TokenStream) -> TokenStream {
    let mut tts = TokenStream::from(quote! {
       #[cfg_attr(
            feature = "cfg-target-has-atomic",
            cfg(all(target_has_atomic = "cas", target_has_atomic = "ptr"))
        )]
    });
    tts.extend(input);
    tts
}

// usage
#[cfg_target_has_atomic]
#[cfg(feature = "alloc")]
mod buffer_unordered;
#[cfg_target_has_atomic]
#[cfg(feature = "alloc")]
pub use self::buffer_unordered::BufferUnordered;

1 is especially useful when many items are in one place, but the indent is deeper.
(I have no strong opinion on which one to use.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that'd work-- I don't have particularly strong thoughts either way. We can leave that for a future PR, though, since it doesn't seem like the current situation is too bad (just some extra boilerplate in a few spots).

@cramertj
Copy link
Member

This looks great! left a couple of small comments, but otherwise this seems like a great approach towards supporting alloc-only environments.

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