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

Convert errors in Accumulator::handle #252

Open
DCNick3 opened this issue Aug 3, 2023 · 4 comments
Open

Convert errors in Accumulator::handle #252

DCNick3 opened this issue Aug 3, 2023 · 4 comments

Comments

@DCNick3
Copy link
Contributor

DCNick3 commented Aug 3, 2023

Currently, Accumulator::handle accepts a darling::Result<T>. This doesn't play nice when using it with non-darling errors (for example, to handle syn error one has to write:

let parsed = accumulator.handle(syn::parse2(tokens).map_err(Into::into));

If Accumulator::handle wound accept Result<T, E> where E: Into<darling::Error>, it would allow to handle other errors without explicit conversion, the same way the ? operator works

@TedDriggs
Copy link
Owner

@ijackson do you recall if there was a reason we didn't do this before? Was it causing problems with type inference?

@ijackson
Copy link
Contributor

ijackson commented Aug 7, 2023

I don't think so. I think handle_in would have such problems with inference, since typically the closure would want to use ?, leading to double-into. I suspect we just didn't consider the two cases separately.

@ijackson
Copy link
Contributor

ijackson commented Aug 7, 2023

If someone makes an MR branch, I can do experimental builds of derive-builder (and derive-builder-fork-arti) to see what the impact seems to be in practice.

@TedDriggs
Copy link
Owner

@DCNick3 can you make a PR with that change for @ijackson to test against?

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

No branches or pull requests

3 participants