Skip to content

Conversation

Ryman
Copy link
Contributor

@Ryman Ryman commented May 14, 2016

This also disabled Once when const_fn is not active as it doesn't seem to be useful without it (and there's no tests exercising it without static usage).

This shouldn't break anything as the default features are opt-in and should give the same behaviour as before.

@Ericson2314
Copy link
Contributor

Nice! How about https://github.com/reem/rust-unreachable for unreachable!()? It works equally well on nightly or stable.

@Ericson2314
Copy link
Contributor

@Ryman
Copy link
Contributor Author

Ryman commented May 14, 2016

I considered the first one but didn't want to add a dependency, but I'm happy to change to that if you want. The second one looks good, but it's not no_std... yet!

@Ericson2314
Copy link
Contributor

Well, the final call is up to @mvdnes, but I never worry about the number of dependencies---traditionally, everyone using C rolled their own spinlock for just that reason!. Plus, with rust-lang/rfcs#1216 on the horizon, I expect that the first or both of those crates will be subsumed by core.

@mvdnes
Copy link
Collaborator

mvdnes commented May 14, 2016

Nice work!

On the unreachable side:
The unreachable! macro seems fine, but I understand some performance reasons.
We could also copy the two relevant functions from the dependencies. Something like this:

#[inline]
unsafe fn unreachable() -> ! {
    enum Void { };
    let x: &Void = std::mem::transmute(1usize);
    match *x {}
}

@mvdnes mvdnes merged commit 886a542 into zesterer:master May 20, 2016
@mvdnes
Copy link
Collaborator

mvdnes commented May 20, 2016

I have merged this without modifications to the unreachable part. If someone has a good idea to update it, it can always be added later.

Anyway, thanks! 👍

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.

3 participants