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] #9

Merged
merged 1 commit into from
Mar 30, 2016
Merged

Add #![no_std] #9

merged 1 commit into from
Mar 30, 2016

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Mar 24, 2016

No description provided.

@DanielKeep
Copy link
Owner

I'm knocking this back for now for two reasons:

  • Although it isn't stated anywhere in this repo, I have crates of my own that depend on custom_derive and support back to Rust 1.2. no_std breaks everything prior to current stable. As such, it needs to be gated behind a feature.
  • This patch doesn't compile at all, because enum_derive uses std::error, which isn't in core.

I stopped looking at that point.

I'm not closing this just yet, because I don't know what you want to do about this. I will say up-front that I'm a little leery of defining a custom Error trait, since it seems like that should be in libcore.

@Zoxc
Copy link
Contributor Author

Zoxc commented Mar 26, 2016

std::error::Error used to be in libcore but was moved out for technical reasons: https://doc.rust-lang.org/nightly/src/std/up/src/libstd/error.rs.html#39

I doesn't seem very useful here so I just removed the impl onno_std.

DanielKeep added a commit that referenced this pull request Mar 27, 2016
DanielKeep added a commit that referenced this pull request Mar 27, 2016
@DanielKeep
Copy link
Owner

Ok, I've made a few amendments to the commit in the issue-9 branch, and just wanted to run them by you before merging.

I went from an optional no_std feature to a default-enabled std feature. This is because I feel Cargo features should generally be additive. In particular, that enabling a feature causes code to disappear seems weird and counter-intuitive. Also, I got this exchange when I asked on IRC about what was more appropriate:

no_std support: should I have an optional "no_std" feature that removes stuff, or a by-default "std" feature?
Quxxy: opt in to std
Quxxy: the former is inline with the no_std attribute in the language
never have a feature that removes things
er, in line?
10 seconds, two opposite answers. :D
because cargo features are a union of what all upstream dependencies ask for
^ I do not understand the aspect of cargo
so if foo wants some stuff, and bar asks your crate to remove that stuff instead
seems easily breakable
then foo won't have that stuff and shit will break
features can be mutually exclusive
cargo features should always only add features
different deps could use mutually exclusive features
scott: Except cargo doesn't work like that
it just unions the features
well, then quxxy should use the by-default "std" feature, right?

As an aside, if do you convince me to go back to no_std, I'll probably rename it to no-std in keeping with the std-unstable feature.

Aside from that, I'm fine with adding this to these crates (and so on to conv).

DanielKeep added a commit that referenced this pull request Mar 30, 2016
Change to using an enabled-by-default `std` feature.
@DanielKeep DanielKeep merged commit c22a0ce into DanielKeep:master Mar 30, 2016
@DanielKeep
Copy link
Owner

Ok, I went through with the amendments and published a new version.

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