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

Remove Anyhow usage to make the crate's error handling no_std compatible #343

Closed
CPerezz opened this issue Jan 12, 2021 · 3 comments · Fixed by #397
Closed

Remove Anyhow usage to make the crate's error handling no_std compatible #343

CPerezz opened this issue Jan 12, 2021 · 3 comments · Fixed by #397
Assignees
Labels
status:minor Low priority improvements team:Core Low Level Core Development Team (Rust)

Comments

@CPerezz
Copy link
Contributor

CPerezz commented Jan 12, 2021

Managing error handling in no_std crates that rely on dusk-plonk it's quite tough since all of the errors use anyhow::Result and anyhow::Error. This is a problem since anyhow always gets inclued and it's no_std version forces to include allocators which might not be desired.

The idea is to create new types for the plonk errors that are no-std compatible and that impl display and other std-related stuff under a std flag.

@CPerezz CPerezz added this to the No-std compatibility milestone Jan 12, 2021
@ZER0 ZER0 added team:Core Low Level Core Development Team (Rust) status:minor Low priority improvements labels Jan 14, 2021
@ZER0 ZER0 removed this from the No-std compatibility milestone Jan 18, 2021
@CPerezz
Copy link
Contributor Author

CPerezz commented Feb 11, 2021

You can implement Display now without the flag and we will add the flags once we introduce alloc and std features.

jules added a commit that referenced this issue Feb 11, 2021
jules added a commit that referenced this issue Feb 11, 2021
jules added a commit that referenced this issue Feb 11, 2021
jules added a commit that referenced this issue Feb 11, 2021
@burdges
Copy link

burdges commented Feb 11, 2021

All true rust types know their size and alignment, which dyn Trait types accomplish using their vtable.

In consequence, one could build a TinyBox<T: Sized> type that allocated when size_of_val(&T) > size_of::<usize>() but otherwise stored T internally, and then TinyBox<dyn Trait> could actually work correctly by using size_of_val(&self), which only accesses the vtable, not the actual self data pointer.

We could then make TinyBox work even without alloc, simply by making its non-alloc form panic when size_of_val(&T) > size_of::<usize>().

At this point, if one has a no_std compatable Error trait then one could rebuild the entire trait infrastructure of std::io on top of this trait. This gives you Result<T,dyn Error> which works even without alloc for small enums, OS ernos, etc., but not large complex error types with backtrace allocations, etc.

I've zero time to implement this, but if anyone gets interested then I'm happy to give some pointers.

cc rust-lang/project-error-handling#20

@CPerezz
Copy link
Contributor Author

CPerezz commented Feb 12, 2021

I really like the idea @burdges . And also seems that there's work on the community towards a similar solution.
I'll add an issue pointing to this message so that we can investigate further once we have more time (or if someone wants to work on it).

Thanks a lot for the suggestion and taking time to explain it, is really interesting! 😃

jules added a commit that referenced this issue Feb 12, 2021
* Remove usage of anyhow and thiserror crates

Fixes #343

* Centralize errors in a single file

This has the added effect of being able to entirely drop
usage of anyhow, even in tests, as well as removing a lot of
map_err calls. Additionally, the Display implementation for Error
is now captured by a feature flag for "std".

* Address review comments

- Remove conditional compilation flag in lib.rs
- Section off errors with comments
- Export the error module directly in the prelude

* Update CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:minor Low priority improvements team:Core Low Level Core Development Team (Rust)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants