Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

no_std support (via an enabled-by-default "std" feature) #157

Closed
wants to merge 1 commit into from

Conversation

tonychain
Copy link

Adds basic support for using error-chain in no_std contexts by disabling an
enabled-by-default "std" feature.

@tarcieri tarcieri mentioned this pull request Jun 8, 2017
@@ -0,0 +1,433 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
Copy link

Choose a reason for hiding this comment

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

This is a vendored version the stable parts of libstd's error.rs.

I know, "Eww", right? I'm not sure of a better solution though.

//! [`Display`]: ../fmt/trait.Display.html
//! [`cause`]: trait.Error.html#method.cause

// This file contains the stable parts of std::error, vendored and modified
Copy link

Choose a reason for hiding this comment

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

The story here becomes even more complicated when we vendor the originally-in-libcore code which got moved into std into error-chain to make it no_std friendly.

@@ -0,0 +1,37 @@
//! Re-exports of types for glossing over no_std/std distinctions
Copy link

Choose a reason for hiding this comment

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

This whole module is a giant hack so we have a singular name we can use across both no_std and std to refer to these modules.

Copy link

Choose a reason for hiding this comment

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

I now see https://github.com/brson/error-chain/pull/64/files and it looks like it was using a similar hack... /cc @Yamakaky

@tarcieri
Copy link

tarcieri commented Jun 8, 2017

Well, all of the --no-default-features tests break because now it assumes the absence of the "std" feature means no_std, which can only possibly work on nightly.

Even on nightly it breaks, because much of the documentation assumes std so all of the doc tests break when --no-default-features is passed.

@tarcieri
Copy link

tarcieri commented Jun 9, 2017

Okay, after tweaking the build matrix a little bit, everything passes except the no_std smoke tests. This is because there are examples that use std::io, which is of course unavailable with no_std.

Not sure what to do here... possibly just disable the smoke tests with no_std since they assume std?

Adds basic support for using error-chain in no_std contexts by disabling an
enabled-by-default "std" feature.
@tarcieri
Copy link

tarcieri commented Jun 9, 2017

I made the tests for no_std build-only, and the build is green. I'm not sure of a good way to make the smoke tests pass with no_std without a lot of selective gating even then, the documentation comments reference std.

@Nemo157
Copy link

Nemo157 commented Jun 9, 2017

This would need a major version bump since it's changing the minimum Rust version of --no-default-features from 1.10/1.13 to nightly. I've seen multiple library crates in the wild that depend on error-chain without default features to avoid pulling in the backtrace support unless their dependents use it, they would all have to update to use the std feature.

@tarcieri
Copy link

tarcieri commented Jun 9, 2017

Alternatively no_std could be made a feature, rather than an on-by-default std feature. Feels kind of backwards to me, but may be less surprising for typical users.

@le-jzr
Copy link

le-jzr commented Jun 9, 2017

A slightly less backwards variant: have std feature and a nightly feature. --no-default-features would not cause no_std behavior unless nightly is enabled, at least until that ability is available in stable.

@tarcieri
Copy link

tarcieri commented Jun 9, 2017

As this PR stands, it expects to get Box and String from libcollections if std is unavailable, so I don't think there's a meaningful mode without either std or nightly-dependent features.

@tarcieri tarcieri mentioned this pull request Jun 9, 2017
@Yamakaky
Copy link
Contributor

Maybe we should open a PR to move Error to core? Seems like to be possible. Part of std::fmt would have to be moved to, but I think it should be possible.

@tarcieri
Copy link

tarcieri commented Jun 10, 2017

@Yamakaky there was already a PR to do that which was rejected: rust-lang/rust#33149

Might be worth asking on that PR (all right, I just did)

@arielb1
Copy link

arielb1 commented Jun 12, 2017

The libcore PR was rejected because it made Error fundamental, which was a breaking change.

I think that can be bypassed with this hack (a combination of what I'll call the "exotic closure" trait and "predicate" trait):

in libcore:

#[doc(hidden)]
#[unstable(feature="libstd_implementation_detail")]
pub trait ErrorOrStrMapper {
    type Output;
    fn from_error<E: Error>(self, e: E) -> Self::Output;
    fn from_str(self, s: &str) -> Self::Output;
}

#[doc(hidden)]
#[unstable(feature="libstd_implementation_detail")]
trait ErrorOrStr {
    fn convert<F: ErrorOrStrMapper>(self, f: F) -> F::Output;
}

impl<E: Error> ErrorOrStr for E {
    fn to_error<F: ErrorOrStrMapper>(self, f: F) -> F::Output {
        f.from_error(self)
    }
}

// ok because libcore knows that `! &'a str : Error`
impl<'a> ErrorOrStr for &'a str {
    fn to_error<F: ErrorOrStrMapper>(self, f: F) -> F::Output {
        f.from_str(self)
    }
}

in liballoc (From<&str> for Box<Error> must be implemented there, because all the types are available in liballoc, so StringError must use Box<str>):

struct StringError(Box<str>);

struct BoxErrorMapper;
impl ErrorOrStrMapper for BoxErrorMapper {
    type Output = Box<Error + Send + Sync>;
    fn from_error<E: Error>(self, e: E) -> Self::Output {
        Box::new(e)
    }
    fn from_str(self, s: &str) -> Self::Output {
        // I'm not sure on that `From` - we might want to return an
        // OomError here if we fail to allocate enough memory.
        Box::new(StringError(From::from(s)))
    }
}

impl<E: ErrorOrStr> From<E> for Box<Error + Send + Sync> {
    fn from(err: E) -> Self {
        err.to_error(BoxErrorMapper)
    }
}

impl<E: ErrorOrStr> From<E> for Box<Error> {
    fn from(err: E) -> Self {
        err.to_error(BoxErrorMapper)
    }
}

impl From<Box<str>> for Box<Error + Send + Sync> {
    fn from(s: Box<str>) -> Self {
        Box::new(StringError(s))
    }
}

// ok because String is local, so we know `!String: ErrorOrStr`
impl From<Box<str>> for Box<Error> {
    fn from(s: String) -> Self {
        Box::new(StringError(s))
    }
}

in libcollections:

// ok because String is local, so we know `!String: ErrorOrStr`
impl From<String> for Box<Error + Send + Sync> {
    fn from(s: String) -> Self {
        Self::from(s.into_boxed_str())
    }
}

// ok because String is local, so we know `!String: ErrorOrStr`
impl From<String> for Box<Error> {
    fn from(s: String) -> Self {
        Self::from(s.into_boxed_str())
    }
}

The downside is that the docs will have the From impl as E: ErrorOrStr rather than separate E: &str and E: Error.

@tarcieri
Copy link

@arielb1 perhaps that's worth mentioning on rust-lang/rust#33149 ?

#[cfg(not(feature = "std"))]
pub use collections::string;
#[cfg(feature = "std")]
pub use std::string;
Copy link
Contributor

@Yamakaky Yamakaky Oct 11, 2017

Choose a reason for hiding this comment

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

It would be clearer to use something like that:

pub use imp::*;
#[cfg(not(feature = "std"))]
mod imp {
    pub use core::ops;
    ...
}
#[cfg(feature = "std")]
mod imp {
    pub use std::ops;
    ...
}

pub mod types;

#[cfg(not(feature = "std"))]
pub mod error;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be put in types.rs

@tarcieri
Copy link

This entire PR is blocked on getting std::error::Error into liballoc. Anyone aware of movement on that front?

@Yamakaky
Copy link
Contributor

Oh, right, sorry. No, I haven't heard about it.

@tarcieri
Copy link

So, this PR has diverged from master, depends on a blocker I don't think is going to get addressed soon, and at this point I no longer work in no_std environments.

Perhaps I should close it, and we can revisit if std::error::Error ever winds up in liballoc?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants