Skip to content
This repository has been archived by the owner on Aug 15, 2021. It is now read-only.

no_std support #79

Closed
chrysn opened this issue Nov 12, 2018 · 26 comments
Closed

no_std support #79

chrysn opened this issue Nov 12, 2018 · 26 comments

Comments

@chrysn
Copy link
Contributor

chrysn commented Nov 12, 2018

Serde supports no_std nicely (judging from the documentation); it would be nice if serde_cbor could too.

I'm currently looking around for what parts of the crate would need to be conditional on std's presence, so basically whether cfg(feature=...) gating; will keep this issue updated.

@chrysn
Copy link
Contributor Author

chrysn commented Nov 13, 2018

The single thing that makes this tricky is the prevalence of IO errors throughout the return codes -- many things possibly return an std::io::Error which is unknown to no_std environments, even though they have no chance of actually producing one. Looking at different variants of how to solve this -- my current string replacement orgy of annotating every error or result with its IO error type (for lack of higher-kinded types) probably won't be the solution, but might should be helpful in finding out what else is needed.

@sfackler
Copy link
Collaborator

I was going to suggest looking at serde_json, but it looks like that doesn't have no_std support yet!

@chrysn
Copy link
Contributor Author

chrysn commented Nov 13, 2018 via email

@dtolnay
Copy link

dtolnay commented Nov 13, 2018

For serde_json we intend to provide a no_std mode that requires alloc. The no alloc case is covered by serde_json_core.

serde_json::ser::Formatter uses io::Error but in no_std mode we can make all those methods private and disallow downstream impls of the trait. Haven't looked at whether it is a more complicated situation in this crate.

@chrysn
Copy link
Contributor Author

chrysn commented Nov 13, 2018 via email

@pyfisch
Copy link
Owner

pyfisch commented Nov 13, 2018

I don't think the Error type is a problem as it does not really expose strings or the io::Error type in the public API. https://docs.rs/serde_cbor/0.9.0/serde_cbor/error/struct.Error.html

@sfackler
Copy link
Collaborator

It does allocate though so it'd need to change in no_std mode if we don't want to require alloc.

The one bit of complexity in the deserialization logic is dealing with indefinite length bytes and strings since we build those up into a buffer.

@chrysn
Copy link
Contributor Author

chrysn commented Nov 13, 2018

I'll have a look at the infinite strings when I get there, but intuitively I don't see much of a challenge in terms of allocation on non-allocating platforms, for those can't have arbitrarily long strings in the first place. (They'd only deal with that in a streaming fashion, but I haven't dived deep enough into serde to see whether that's actually supported.)

Meanwhile, I'm hacking along at an experimental branch, trying to get any no_std variation working before I think about how that's best sold in terms of well-argued commits.

@dtolnay
Copy link

dtolnay commented Nov 13, 2018

The no_std use I'm aiming for is more than just not having std in public types (as using private to prevent impls would achieve)
there is just no std::io defined

This is the same use case as in serde_json. By removing method signatures from the public API we can adjust them internally to use a type that is not io::Result.

@chrysn
Copy link
Contributor Author

chrysn commented Nov 14, 2018

I've to take some rather brutal cuts (disabling ser, and not implementing any string stuff in no_std mode), which I'll try to push into reasonable ones yet -- but can now parse some basic CBOR using this crate on no_std with the experimental branch.

@chrysn
Copy link
Contributor Author

chrysn commented Nov 14, 2018

Status update (let me know if they're too frequent): Serialization works as well, and with fewer limitations (no problems around strings). It'll all probably need some cleanup to make the delta go down, and make it clearer what the Write trait does exactly. (Also, string deserialization is still to be solved.)

I could need a bit of guidance when it comes to error handling. In the first commits, I've established a model of "either it's std, then IO errors are std::io::Error, or it's not, then IO errors are a single enum variant without payload" (with an IoResult to go along with it, also typed depending on std presence). With the Write abstraction, more could be done -- any writer can have its own IO error type, as long as it can Into (which, by convention, would produce IO variants of error because the errors were IO errors before). That generates flexibility within the library that might be impossible to expose on the public side without adding generics everywhere. I think the latter is still the preferable route to go (as it keeps us a bit further out of the ifdef hell I'm actually trying to steer clear), but it'd help to know what you think of that.

@pyfisch
Copy link
Owner

pyfisch commented Nov 14, 2018

Can you show us the branch? I think guidance is easier if we can see the code.

@chrysn
Copy link
Contributor Author

chrysn commented Nov 14, 2018

Oups, thought I had linked it already; here it is: https://github.com/chrysn-pull-requests/cbor/tree/no_std_experiments

@pyfisch
Copy link
Owner

pyfisch commented Nov 15, 2018

I've established a model of "either it's std, then IO errors are std::io::Error, or it's not, then IO errors are a single enum variant without payload"

Looks good.

About the Write abstraction I am not sure. I think it may be good idea to first land no-std-deserialization and figure serialization later out.

@pyfisch
Copy link
Owner

pyfisch commented Nov 15, 2018

@sfackler What do you think?

@chrysn
Copy link
Contributor Author

chrysn commented Nov 16, 2018

Finite strings now work in deserialization too.

Infinite strings still don't work on no_std, as they'd need either some kind of scatter-gather support of serde visitors (sounds unlikely to me), or an additional reader that works on a mutable slice (and shifts data around as it reads over it -- a bit scary and possibly needs unsafe, but nice for embedded environments).

Preparing infinite string deserialization I've found two possibilities for enhancement in the IoRead (which could be flippantly said to be about using standard library features rather than re-implementing them oneself); they're currently at the end of my branch but I'll pick them out to a dedicated PR of their own.

Ad not-sure-about-write: This is a slightly bigger patch mainly because of error handling oddities I haven't managed track down and might be an error in Rust (see rust-lang/rust#55984); lets' wait on the serialization side until that's been cleared up.

[edit: fixed link to error handling issue]

@wildarch
Copy link
Contributor

It would be great to see no_std support for serde_cbor 😄! I'd love to help fix any unresolved issues that prevent @chrysn's branch from being merged. What issues are currently left?

@chrysn
Copy link
Contributor Author

chrysn commented Dec 19, 2018

Currently what's pending is the review of #80. Once that is merged, I'll rebase the no_std branch onto that, but the patch series in #80 has seen enough standalone progress that doing much with the no_std branch is not too practical at the moment.

(With the MutSliceRead of #80, even infinite strings can be deserialized if a reader with write access to its buffer is used.)

@wildarch
Copy link
Contributor

That makes sense, I'll be patient then 😉.

Something else related to no_std support: It might be useful to not seal the internal Read trait when the std feature is disabled. Without std there is no io::Read, which only leaves slices as a source. Given that no_std platforms are often embedded devices with very little memory, having some kind of Read trait is a must for parsing a large amount of data. Maybe making the internal Read trait public would be a solution for now, at least until core::io becomes a thing?

@chrysn
Copy link
Contributor Author

chrysn commented Dec 19, 2018 via email

@wildarch
Copy link
Contributor

It looks like that is where we are heading, but to be fair multiple options are being considered and AFAIK no decision has been made yet, see RFC2262.

API stability could be a concern indeed. I was assuming the no_std API would be considered unstable anyway, but perhaps I am mistaken? I personally like the unproven feature flag idea.

Maybe it could also still be an option to make the Read trait part of the public API: As far as I can tell it has not seen any changes in the past two years, so it is pretty stable. I suspect the reason to make it private was not to hide API details, but to avoid confusion between this Read and io::Read.

@PraneshAnubhav
Copy link

@wildarch @chrysn
I want to use CBOR in rust with no_std. Is this available now? If yes, can you please tell me how to use it?

@wildarch
Copy link
Contributor

wildarch commented Jun 3, 2019

Hi!

Always nice to see more people interested in no-std Rust 😄.

To get this to work you can configure your Cargo.toml to use the master branch of this repo and disable the default features, e.g.

serde_cbor = { git = "https://github.com/pyfisch/cbor", default-features = false }

Please note that I'm on holiday right now so I don't have a laptop to verify the above.

@PraneshAnubhav
Copy link

@wildarch
Thanks for your response! It works. I have referred to no_std test cases for it's usage in no_std Rust.

@pyfisch
Copy link
Owner

pyfisch commented Sep 7, 2019

I think this is done?
Are there any improvements planned to no-std support? @wildarch @chrysn

@wildarch
Copy link
Contributor

wildarch commented Sep 7, 2019

I think basic no_std support is there, so indeed this issue could be resolved. I am not currently working on anything related to this, but maybe @chrysn is working on something?

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

6 participants