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

Support Error in no_std environments #268

Merged
merged 11 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ jobs:
test-features:
name: test features
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
std: [std, '']
steps:
- uses: actions/checkout@v3
- uses: dtolnay/rust-toolchain@v1
Expand All @@ -128,7 +132,7 @@ jobs:
go install github.com/pelletier/go-toml/cmd/tomljson@latest
echo "$HOME/go/bin" >> $GITHUB_PATH

- run: ci/test_all_features.sh
- run: ci/test_all_features.sh ${{ matrix.std }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@JelteF I think here will be enough just to run tests 2 times sequentially, rather than matrix over the input parameter.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think it's quite nice to see if both no_std and std are broken with a quick glance at the job output. Also it would double our CI time, since it would be the longest running job by ~2x the previous one. So I changed this back to use a matrix. I did make the matrix a bit more descriptive though, so that the job output is clearer.

Copy link
Collaborator

@tyranron tyranron Jul 5, 2023

Choose a reason for hiding this comment

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

@JelteF you should edit branch protection rules, because now CI awaits test features job to complete, while we have test features (std) and test features (no_std) now only.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done. If you approve the PR it should merge automatically.




Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
should prevent code style linters from attempting to modify the generated
code.
- Upgrade to `syn` 2.0.
- The `Error` derive now works in nightly `no_std` environments when enabling
`#![feature(error_in_core)]`.

### Fixed

Expand Down
11 changes: 7 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ members = ["impl"]
[dependencies]
derive_more-impl = { version = "=0.99.17", path = "impl" }

[build-dependencies]
rustc_version = { version = "0.4", optional = true }

[dev-dependencies]
rustversion = "1.0"
trybuild = "1.0.56"
Expand All @@ -50,7 +53,7 @@ debug = ["derive_more-impl/debug"]
deref = ["derive_more-impl/deref"]
deref_mut = ["derive_more-impl/deref_mut"]
display = ["derive_more-impl/display"]
error = ["derive_more-impl/error", "std"]
error = ["derive_more-impl/error"]
from = ["derive_more-impl/from"]
from_str = ["derive_more-impl/from_str"]
index = ["derive_more-impl/index"]
Expand All @@ -67,7 +70,7 @@ is_variant = ["derive_more-impl/is_variant"]
unwrap = ["derive_more-impl/unwrap"]
try_unwrap = ["derive_more-impl/try_unwrap"]

std = []
std = ["derive_more-impl/std"]
full = [
"add_assign",
"add",
Expand Down Expand Up @@ -96,7 +99,7 @@ full = [
"try_unwrap",
]

testing-helpers = ["derive_more-impl/testing-helpers"]
testing-helpers = ["derive_more-impl/testing-helpers", "dep:rustc_version"]

[[test]]
name = "add_assign"
Expand Down Expand Up @@ -151,7 +154,7 @@ required-features = ["display"]
[[test]]
name = "error"
path = "tests/error_tests.rs"
required-features = ["error", "std"]
required-features = ["error"]

[[test]]
name = "from"
Expand Down
15 changes: 15 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#[cfg(not(feature = "testing-helpers"))]
fn detect_nightly() {}

#[cfg(feature = "testing-helpers")]
fn detect_nightly() {
use rustc_version::{version_meta, Channel};

if version_meta().unwrap().channel == Channel::Nightly {
println!("cargo:rustc-cfg=nightly");
}
}

fn main() {
detect_nightly();
}
8 changes: 6 additions & 2 deletions ci/test_all_features.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#!/usr/bin/env bash
set -euxo pipefail

for feature in $(tomljson Cargo.toml | jq --raw-output '.features | keys[]' | grep -v 'default\|std\|testing-helpers'); do
cargo test -p derive_more --tests --no-default-features --features "$feature,testing-helpers";
for feature in $(tomljson Cargo.toml | jq --raw-output '.features | keys[]' | grep -v 'default\|std\|full\|testing-helpers'); do
if [ "${1:-}" = 'std' ]; then
cargo test -p derive_more --tests --no-default-features --features "$feature,std,testing-helpers";
else
cargo test -p derive_more --tests --no-default-features --features "$feature,testing-helpers";
fi
done
4 changes: 3 additions & 1 deletion impl/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ unicode-xid = { version = "0.2.2", optional = true }
rustc_version = { version = "0.4", optional = true }

[dev-dependencies]
derive_more = { path = "..", features = ["add", "debug", "error", "from_str", "not", "try_into", "try_unwrap"] }
derive_more = { path = "..", features = ["add", "debug", "error", "from_str", "not", "std", "try_into", "try_unwrap"] }
itertools = "0.11.0"

[badges]
Expand Down Expand Up @@ -71,4 +71,6 @@ is_variant = ["dep:convert_case"]
unwrap = ["dep:convert_case"]
try_unwrap = ["dep:convert_case"]

std = []
JelteF marked this conversation as resolved.
Show resolved Hide resolved

testing-helpers = ["dep:rustc_version"]
12 changes: 11 additions & 1 deletion impl/doc/error.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,23 @@ ignored for one of these methods by using `#[error(not(backtrace))]` or
`#[error(not(source))]`.


### What works in `no_std`?

If you want to use the `Error` derive on `no_std` environments, then you need to
compile with nightly and enable this feature:
```ignore
#![feature(error_in_core)]
```

Backtraces don't work though, because the `Backtrace` type is only available in
`std`.


## Example usage

```rust
# #![cfg_attr(nightly, feature(error_generic_member_access, provide_any))]
// Nightly requires enabling this features:
// Nightly requires enabling these features:
// #![feature(error_generic_member_access, provide_any)]
# #[cfg(not(nightly))] fn main() {}
# #[cfg(nightly)] fn main() {
Expand Down
24 changes: 12 additions & 12 deletions impl/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn expand(
let state = State::with_attr_params(
input,
trait_name,
quote! { ::std::error },
quote!{ ::derive_more::__private::Error },
Copy link
Collaborator

Choose a reason for hiding this comment

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

@JelteF how about just ::derive_more::Error?

We can bring the trait in scope together with the macro. This may be handy for much cases. And we can do this for all the traits (in a separate PR, surely).

Still, if someone needs only a macro, we could have done it like use derive_more::macro::Error.

Copy link
Owner Author

Choose a reason for hiding this comment

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

We can bring the trait in scope together with the macro. This may be handy for much cases. And we can do this for all the traits (in a separate PR, surely).

Yeah, I think that's probably a good idea. Because of this change I'm working on adding the following line to all our tests:

#![cfg_attr(not(feature = "std"), no_std)]

And automatically importing the traits, possibly from alloc would be quite useful.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'll leave it as __private here and we can make it public in the follow up PR where we do this for all traits.

trait_name.to_lowercase(),
allowed_attr_params(),
)?;
Expand All @@ -39,7 +39,7 @@ pub fn expand(

let source = source.map(|source| {
quote! {
fn source(&self) -> Option<&(dyn ::std::error::Error + 'static)> {
fn source(&self) -> Option<&(dyn ::derive_more::__private::Error + 'static)> {
use ::derive_more::__private::AsDynError;
#source
}
Expand All @@ -48,7 +48,7 @@ pub fn expand(

let provide = provide.map(|provide| {
quote! {
fn provide<'_demand>(&'_demand self, demand: &mut ::std::any::Demand<'_demand>) {
fn provide<'_demand>(&'_demand self, demand: &mut ::core::any::Demand<'_demand>) {
#provide
}
}
Expand All @@ -62,7 +62,7 @@ pub fn expand(
&generics,
quote! {
where
#ident #ty_generics: ::std::fmt::Debug + ::std::fmt::Display
#ident #ty_generics: ::core::fmt::Debug + ::core::fmt::Display
},
);
}
Expand All @@ -73,7 +73,7 @@ pub fn expand(
&generics,
quote! {
where
#(#bounds: ::std::fmt::Debug + ::std::fmt::Display + ::std::error::Error + 'static),*
#(#bounds: ::core::fmt::Debug + ::core::fmt::Display + ::derive_more::__private::Error + 'static),*
},
);
}
Expand All @@ -82,7 +82,7 @@ pub fn expand(

let render = quote! {
#[automatically_derived]
impl #impl_generics ::std::error::Error for #ident #ty_generics #where_clause {
impl #impl_generics ::derive_more::__private::Error for #ident #ty_generics #where_clause {
#source
#provide
}
Expand Down Expand Up @@ -207,7 +207,7 @@ impl<'input, 'state> ParsedFields<'input, 'state> {
let source_provider = self.source.map(|source| {
let source_expr = &self.data.members[source];
quote! {
::std::error::Error::provide(&#source_expr, demand);
::derive_more::__private::Error::provide(&#source_expr, demand);
}
});
let backtrace_provider = self
Expand All @@ -217,7 +217,7 @@ impl<'input, 'state> ParsedFields<'input, 'state> {
.then(|| {
let backtrace_expr = &self.data.members[backtrace];
quote! {
demand.provide_ref::<std::backtrace::Backtrace>(&#backtrace_expr);
demand.provide_ref::<::std::backtrace::Backtrace>(&#backtrace_expr);
}
});

Expand All @@ -237,7 +237,7 @@ impl<'input, 'state> ParsedFields<'input, 'state> {
let pattern = self.data.matcher(&[source], &[quote! { source }]);
Some(quote! {
#pattern => {
::std::error::Error::provide(source, demand);
::derive_more::__private::Error::provide(source, demand);
}
})
}
Expand All @@ -248,16 +248,16 @@ impl<'input, 'state> ParsedFields<'input, 'state> {
);
Some(quote! {
#pattern => {
demand.provide_ref::<std::backtrace::Backtrace>(backtrace);
::std::error::Error::provide(source, demand);
demand.provide_ref::<::std::backtrace::Backtrace>(backtrace);
::derive_more::__private::Error::provide(source, demand);
}
})
}
None => {
let pattern = self.data.matcher(&[backtrace], &[quote! { backtrace }]);
Some(quote! {
#pattern => {
demand.provide_ref::<std::backtrace::Backtrace>(backtrace);
demand.provide_ref::<::std::backtrace::Backtrace>(backtrace);
}
})
}
Expand Down
16 changes: 10 additions & 6 deletions impl/src/fmt/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,12 +331,16 @@ impl<'a> Expansion<'a> {
/// Generates trait bounds for a struct or an enum variant.
fn generate_bounds(&self) -> Vec<syn::WherePredicate> {
let Some(fmt) = &self.attrs.fmt else {
return self.fields.iter().next().map(|f| {
let ty = &f.ty;
let trait_ident = &self.trait_ident;
vec![parse_quote! { #ty: ::core::fmt::#trait_ident }]
})
.unwrap_or_default();
return self
.fields
.iter()
.next()
.map(|f| {
let ty = &f.ty;
let trait_ident = &self.trait_ident;
vec![parse_quote! { #ty: ::core::fmt::#trait_ident }]
})
.unwrap_or_default();
};

fmt.bounded_types(self.fields)
Expand Down
7 changes: 7 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(all(not(feature = "std"), feature = "error"), feature(error_in_core))]
// These links overwrite the ones in `README.md`
// to become proper intra-doc links in Rust docs.
//! [`From`]: crate::From
Expand Down Expand Up @@ -73,10 +74,16 @@ pub use self::r#str::FromStrError;
#[cfg(feature = "error")]
mod vendor;


// Not public API.
#[doc(hidden)]
#[cfg(feature = "error")]
pub mod __private {
#[cfg(feature = "std")]
pub use ::std::error::Error;

#[cfg(not(feature = "std"))]
pub use ::core::error::Error;
pub use crate::vendor::thiserror::aserror::AsDynError;
}

Expand Down
7 changes: 6 additions & 1 deletion src/vendor/thiserror/aserror.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
#[cfg(feature = "std")]
use std::error::Error;
use std::panic::UnwindSafe;

#[cfg(not(feature = "std"))]
use core::error::Error;

use core::panic::UnwindSafe;

pub trait AsDynError<'a>: Sealed {
fn as_dyn_error(&self) -> &(dyn Error + 'a);
Expand Down
2 changes: 2 additions & 0 deletions tests/error/derives_for_enums_with_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ enum TestErr {
source: SimpleErr,
field: i32,
},
#[cfg(std)]
NamedImplicitBoxedSource {
JelteF marked this conversation as resolved.
Show resolved Hide resolved
source: Box<dyn Error + Send + 'static>,
field: i32,
Expand Down Expand Up @@ -98,6 +99,7 @@ fn named_implicit_source() {
assert!(err.source().unwrap().is::<SimpleErr>());
}

#[cfg(std)]
#[test]
fn named_implicit_boxed_source() {
let err = TestErr::NamedImplicitBoxedSource {
Expand Down
12 changes: 8 additions & 4 deletions tests/error/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#[cfg(feature = "std")]
use std::error::Error;

#[cfg(not(feature = "std"))]
use core::error::Error;

use derive_more::Error;

/// Derives `std::fmt::Display` for structs/enums.
Expand Down Expand Up @@ -29,17 +33,17 @@ use derive_more::Error;
/// ```
macro_rules! derive_display {
(@fmt) => {
fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
write!(f, "")
}
};
($type:ident) => {
impl ::std::fmt::Display for $type {
impl ::core::fmt::Display for $type {
derive_display!(@fmt);
}
};
($type:ident, $($type_parameters:ident),*) => {
impl<$($type_parameters),*> ::std::fmt::Display for $type<$($type_parameters),*> {
impl<$($type_parameters),*> ::core::fmt::Display for $type<$($type_parameters),*> {
derive_display!(@fmt);
}
};
Expand All @@ -50,7 +54,7 @@ mod derives_for_generic_enums_with_source;
mod derives_for_generic_structs_with_source;
mod derives_for_structs_with_source;

#[cfg(nightly)]
#[cfg(all(feature = "std", nightly))]
mod nightly;

derive_display!(SimpleErr);
Expand Down
2 changes: 2 additions & 0 deletions tests/error_tests.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(nightly, feature(error_generic_member_access, provide_any))]
#![cfg_attr(not(feature = "std"), feature(error_in_core))]

mod error;