-
Notifications
You must be signed in to change notification settings - Fork 23
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
allow implementing core::error::Error
when available
#184
Conversation
00425b9
to
be8fbcd
Compare
I think its good lib.rs use nutype::nutype;
#[nutype(validate(finite))]
struct A(f32); Cargo.toml [dependencies]
nutype = { path = "../nutype/nutype" } Outputs of
|
@vic1707 Hi thanks for your PRs. |
No problem take your time 👍 |
Added the |
Hi @greyblake may I ask for an update ? |
Hi, sorry I am on vacation, will be back in 10 days.
…On Sun, Sep 29, 2024, 18:21 Victor LEFEBVRE ***@***.***> wrote:
Hi @greyblake <https://github.com/greyblake> may I ask for an update ?
—
Reply to this email directly, view it on GitHub
<#184 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA3W2E7SJ4AGSQFV5CHHQTZZALGPAVCNFSM6AAAAABOE6I4NSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBRGM4TMNJUGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi @greyblake may I ask for an update ? |
@vic1707 Sorry, I'll try to merge this on the weekend. |
#[allow(unused_variables)] | ||
pub fn gen_impl_error_trait(error_type_name: &ErrorTypeName) -> TokenStream { | ||
cfg_if! { | ||
if #[cfg(feature = "std")] { | ||
if #[cfg(ERROR_IN_CORE)] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the implementation is the same we should have something like
#[cfg(ERROR_IN_CORE)] || #[cfg(feature = "std")]
instead of another else if
branch that generates the same code.
If #[cfg(ERROR_IN_CORE)] || #[cfg(feature = "std")]
by some reason is not possible, then we should still DRY it up, by extracting the generated code in a var.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its not exactly the same implementation due to std
and core
imports.
I tried to DRY it up but couldn't at the time (partly because I didn't even think of #[cfg(ERROR_IN_CORE)] || #[cfg(feature = "std")]
).
I saw a bunch of similar PRs about std
vs core
in the mean time, I'll give it another go 👍.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I can't find any infos on how to do an OR operation, reading the source code I don't see any option to do it (but I could've missed it, I'm not really good with macros 😂).
The not()
operation doesn't seem to be possible either.
I have another idea, I'll keep you updated !
EDIT: I forgot that the correct syntax is simply #[cfg(any(option1, option2))]
🤦.
@@ -68,27 +68,38 @@ pub fn gen_def_parse_error( | |||
}; | |||
|
|||
cfg_if! { | |||
if #[cfg(feature = "std")] { | |||
if #[cfg(ERROR_IN_CORE)] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here we should have #[cfg(ERROR_IN_CORE)] || #[cfg(feature = "std")]
@vic1707 Thank you for your contribution and again sorry for the delay. |
e5d428a
to
dbd7b69
Compare
dbd7b69
to
d8239e8
Compare
@greyblake, just pushed a dryer version, what do you think ? I thought of doing let error = cfg_if! {
if #[cfg(ERROR_IN_CORE)] {
quote! { ::core::error::Error }
} else {
quote! { ::std::error::Error }
}
}; instead of the proposed version but it's not yet supported by rust itself, see: rust-lang/rust#15701. Also, do you think you could publish a new version after this merge ? |
@vic1707 Sorry by bad, in my thinking I assumed that we could just use the |
No problem, it helped me get a dryer version out, win-win 👍. |
rust 1.81.0 made
std::error::Error
available in core.Users with that version or higher should be getting the
Error
impl no matter ifstd
feature is enabled.fixes #179 without introducing a breaking change