Arnavion / derive-error-chain

A Macros 1.1 implementation of https://crates.io/crates/error-chain
19 stars 1 forks source link

Use upstream traits - Backtrace, ChainedError, ErrorChainIter, ResultExt, State #5

Closed Yamakaky closed 7 years ago

Yamakaky commented 7 years ago

I recently added the ChainedError trait to error-chain. It would be cool if you could implement in with your crate, so that the two could be used together. Here is how it is implemented in the macro: https://github.com/brson/error-chain/blob/master/src/error_chain.rs#L71-L84. I'll add your crate in my README ;)

Yamakaky commented 7 years ago

In fact, would you be OK to merge our two repos?

Arnavion commented 7 years ago

so that the two could be used together

I don't think this will work. If I implemented it here it would be a completely new ChainedError trait for each errorkind struct (similar to how ChainIter / make_backtrace are emitted once for each errorkind), so it won't interop with error_chain::ChainedError.

In fact, would you be OK to merge our two repos?

Sure, but how would it work given that the codegen is completely separate? macro_rules vs proc_macro_derive function?

Yamakaky commented 7 years ago

There is only one ChainedError trait, unlike the old ChainErr trait.

Arnavion commented 7 years ago

Yes, but since there is no $crate for proc macros, this crate will need to emit a separate ChainedError trait for each errorkind, just like it emits separate ChainIter and make_backtrace instead of using a single $crate:: one.

Yamakaky commented 7 years ago

I think we can do like with serde, where they have serde which defines the traits and serde_derive for the macro 1.1. In this case, you have to extern crate the two of them. It would work the same for our use-case.

Arnavion commented 7 years ago

That makes sense. I can change this one's output to use the error_chain:: exported traits and expect users to use both.

Yamakaky commented 7 years ago

And serde and serde_derive are in the same crate ;) https://github.com/serde-rs/serde

dtolnay commented 7 years ago

In this case, you have to extern crate the two of them.

In Serde we use the following hack to work around this. We expand:

#[derive(Serialize)]
struct Point {
    /* ... */
}

... into:

const _IMPL_SERIALIZE_FOR_Point: () = {
    extern crate serde as _serde;
    impl _serde::Serialize for Point {
        /* ... */
    }
};

This way it works whether or not the user's code has extern crate serde. For instance the example in the readme does not have it.

Yamakaky commented 7 years ago

Hum, didn't know this trick, thanks!

Yamakaky commented 7 years ago

Adding @brson to the conversation.

Yamakaky commented 7 years ago

@dtolnay the user still have to put serde in Cargo.toml?

dtolnay commented 7 years ago

the user still have to put serde in Cargo.toml?

Yes they do.

Yamakaky commented 7 years ago

BTW, macros 1.1 should be in stable in a few weeks now, so this crate is bound to replace the macro.

Arnavion commented 7 years ago

so this crate will replace the macro.

Well... I wouldn't go that far just yet. There are a few inconveniences compared to error_chain! due to limitations of proc macros:

Arnavion commented 7 years ago

@Yamakaky I'm done with the port to use error_chain traits, with one snag.

Because the latest release 0.6.1 doesn't have this commit the codegen of derive-error-chain can't use State::backtrace() either. Do you plan to make a new release with that commit soon, or should I switch to the 0.6.1 version (direct access to State::backtrace field depending on backtrace feature) ?

Yamakaky commented 7 years ago

Can you use master for now? I'd rather not do a release every few days.

Arnavion commented 7 years ago

Okay, I'll keep it in a separate branch then.

Yamakaky commented 7 years ago

Thank you. I already have a breaking syntax change on master, and I'm trying to fix the last one I'm aware of. Please ping me if I haven't made a new release in a week or two ;)