Skip to content
This repository has been archived by the owner on Feb 6, 2022. It is now read-only.

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

Closed
Yamakaky opened this issue Nov 21, 2016 · 18 comments
Milestone

Comments

@Yamakaky
Copy link

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
Copy link
Author

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

@Arnavion
Copy link
Owner

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
Copy link
Author

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

@Arnavion
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

@Yamakaky
Copy link
Author

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

@dtolnay
Copy link

dtolnay commented Nov 21, 2016

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
Copy link
Author

Hum, didn't know this trick, thanks!

@Yamakaky
Copy link
Author

Adding @brson to the conversation.

@Yamakaky
Copy link
Author

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

@dtolnay
Copy link

dtolnay commented Nov 21, 2016

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

Yes they do.

@Yamakaky
Copy link
Author

Yamakaky commented Nov 21, 2016

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

@Arnavion
Copy link
Owner

Arnavion commented Nov 21, 2016

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 Arnavion changed the title ChainedError support Use upstream traits - Backtrace, ChainedError, ErrorChainIter, ResultExt, State Nov 22, 2016
@Arnavion
Copy link
Owner

Arnavion commented Nov 23, 2016

@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
Copy link
Author

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

@Arnavion
Copy link
Owner

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

@Yamakaky
Copy link
Author

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 ;)

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

No branches or pull requests

3 participants