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

no_std is stable now #45

Merged
merged 1 commit into from
Apr 20, 2016
Merged

no_std is stable now #45

merged 1 commit into from
Apr 20, 2016

Conversation

Ericson2314
Copy link
Contributor

Not sure if we still want __core

@Amanieu
Copy link
Contributor

Amanieu commented Apr 9, 2016

We still need __core because we don't know whether the crate using bitflags! has core or std, and it could even have overridden these names to mean something else entirely. Therefore the only reliable way of referring to core is through $crate::__core.

@Amanieu
Copy link
Contributor

Amanieu commented Apr 9, 2016

However note that removing the no_std Cargo feature is a breaking change. We could leave it as a no-op or remove it and bump our version number.

@Ericson2314
Copy link
Contributor Author

Hmm I would just bump it. Don't think this crate is ready for 1.0 and less churn until associated types constants are stable anyways.

Also, is pub extern crate a thing? Can we do that with $crate::core and drop the underscores?

@Amanieu
Copy link
Contributor

Amanieu commented Apr 9, 2016

I think pub extern crate currently gives a warning saying it doesn't work properly and that you should use pub use instead.

The reason I added underscores was because __core is not part of the API of this crate, it only needs to be publicly exported so that it can be used by the macro.

@Ericson2314
Copy link
Contributor Author

Alright, makes sense. I'll try pub extern crate core as __core, and bump the version.

@Ericson2314
Copy link
Contributor Author

rust-lang/rust#26775 Ok pub extern crate is currently broken.

@Ericson2314
Copy link
Contributor Author

Nevermind, it does work after all. Used that, and bumped version number, so this should be ready to go.

@Amanieu
Copy link
Contributor

Amanieu commented Apr 10, 2016

Travis doesn't seem to agree (see below)

@Ericson2314
Copy link
Contributor Author

Ah, I only tested on nightly oops. I'll revert that, and fix the Travis error on the other two where it tries to build with the no_std feature.

@phil-opp
Copy link

I think you can just do a:

pub use core as __core;

The extern crate core is not needed since it's imported automatically when using #![no_std].

Edit: Is there away to hide the __core module from the generated documentation? #[doc(hidden)] does not seem to work here…

@Ericson2314
Copy link
Contributor Author

@phil-opp It's needed for tests when #![no_std] is disabled.

@alexcrichton
Copy link
Contributor

Looks like CI is failing? Otherwise seems like a good change to me!

@Ericson2314 Ericson2314 force-pushed the master branch 4 times, most recently from 1851532 to cde2114 Compare April 20, 2016 02:55
@Ericson2314
Copy link
Contributor Author

Ok, fixed problems. Also made it always use #![no_std] and conditionally import std for tests, which seems more idiomatic.

@alexcrichton
Copy link
Contributor

Can you back out the version bump? That's typically done as a separate commit

@Ericson2314
Copy link
Contributor Author

Separate PR I assume you mean? Unless I messed up it is a separate commit ATM.

@alexcrichton
Copy link
Contributor

Yes, separate PR (or I'll just do it when I merge this)

@Ericson2314
Copy link
Contributor Author

Sure. I'll just remove the commit then.

@Ericson2314
Copy link
Contributor Author

Done!

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.

4 participants