[Rust] Support alloc-only no_std platforms #5366

jrobsonchase · 2019-05-22T11:32:08Z

Hi! I'd love to be able to use flatbuffers on embedded systems, and it looks like it wouldn't be hard to support seeing as it's already 90% there.

This adds the #![no_std] attribute, changes all std imports to core, and pulls Vec from the alloc crate rather than std. I'd also love to see a StaticFlatBufferBuilder (or some such) that works only with ungrowable buffers, but that'll be a bigger change.

For platforms with std, this shouldn't change a thing - it just allows it to be used as a dependency for no_std crates.

googlebot · 2019-05-22T11:32:13Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

googlebot · 2019-05-22T11:50:53Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

jrobsonchase · 2019-05-22T12:23:01Z

Shoot, I keep forgetting that things that have been stabilized in nightly aren't necessarily stabilized in stable as well 😛

So I see two paths forward:

Put no_std/alloc support behind a feature gate that'll need to be removed once alloc lands in stable so that the upstream flatbuffers crate can be used or
Just sit on this PR until stable rust has the proper support and use a fork for now

Thoughts?

rw · 2019-05-23T00:04:56Z

@jrobsonchase Wow, thanks for submitting this PR! no_std support would be great.

My first concern is ensuring that we work on stable Rust. After that, I'm happy to figure out how to support no_std. I like your idea of using a Cargo feature flag. Would stable and nightly users both be happy with that?

jrobsonchase · 2019-05-23T14:10:49Z

It should be working on stable now via the default std flag. To make it work on no_std, you just specify default-features = false on the dependency and it'll add the no_std attr and pull in alloc, which is the way that no_std + alloc crates have been going about it these days.

I seem to have broken the generator tests. I think there were instructions in the PR guidelines that I skipped over because I wasn't originally planning on making any generator changes 😛 How do I go about making them happy again?

rw · 2019-05-24T02:18:27Z

@jrobsonchase Cool!

Here's how you can fix it. There's a big error message in the middle of all the test output:

ERROR: ********************************************************
ERROR: The following differences were found after running the
ERROR: tests/generate_code.sh script.  Maybe you forgot to run
ERROR: it after making changes in a generator or schema?
ERROR: ********************************************************

https://travis-ci.org/google/flatbuffers/jobs/535909377

rw · 2019-05-24T19:20:09Z

@jrobsonchase Is it feasible to add a new integration test file to this directory, to test this PR's functionality?

https://github.com/google/flatbuffers/tree/master/tests/docker/languages

jrobsonchase · 2019-05-24T19:42:23Z

Sure, I can look into that :)

tests/RustTest.sh

rw · 2019-06-08T01:13:58Z

@jrobsonchase What do we need to do to get this finished? Thanks for starting it!

jrobsonchase · 2019-06-09T15:32:46Z

It's ready to go as far as I can tell. The CI failure looks like an unrelated download problem.

Maybe travis needs a kick?

jrobsonchase · 2019-06-26T17:13:40Z

@rw ping

rw · 2019-06-28T18:19:21Z

@jrobsonchase Can you push a noop commit to trigger Travis? Then I'll check the logs to see that your new Dockerfile-based test ran successfully.

rw · 2019-10-03T23:50:08Z

@jrobsonchase Looks like we accidentally let this PR languish, sorry about that! Are you still up for finishing it?

bbqsrc · 2020-02-12T08:13:01Z

What's left to push this over the line?

TheVova · 2020-02-16T14:56:43Z

also asking whats left on this? if any help is needed let me know. i'd love to see this on embedded :)

domenukk · 2021-06-19T22:12:38Z

Missing no_std support is what keeps me from using flatbuffers, would be great to have from my PoV

bbqsrc · 2021-06-20T16:46:45Z

@rw this code is still needed for use in embedded environments. We use a fork in production as a workaround, and ultimately we'd prefer not to. 😉

jacobrosenthal · 2021-06-20T17:07:31Z

I've been following this issue for years as yes it would be nice to have for embedded. But really I'm actually waiting for the step further than this. The point of flatbuffers is zero copy, presumably zero allocation so its sad that allocation is still such a big part of even this pr. Most embedded code in rust land doesnt even use alloc partly because some allocator bits are still nightly only but mainly because the ecosystem is mostly real time nerds who are trying to avoid the original sin of allocation as much as they can.

bbqsrc · 2021-06-21T13:53:08Z

While that may be so, the first step to getting to an alloc-less situation (such as by using heapless where necessary) is to have the crate build in #[no_std] at all.

CasperN · 2021-06-21T16:52:24Z

While that may be so, the first step to getting to an alloc-less situation (such as by using heapless where necessary) is to have the crate build in #[no_std] at all.

I think we may want to turn FlatBufferBuilder into a trait and have separate implementations. They could be specialized to different use cases E.g.

string pooling and even vtable deduplication might be unwanted in small environments - then there'd only be one vector in the builder, which could be replaced with a slice.
no-std with a provided custom allocator
no-std no allocator

This may require some minor rethinking of the API, e.g. use result types so we can fail to build a flatbuffer if the builder runs out of memory without just panic/abort.

jrobsonchase · 2021-10-18T21:01:39Z

@rw This is ready for another review

josephlr

This looks good! Thanks for working on this. Mostly some remaining nits.

rust/flatbuffers/src/lib.rs

src/idl_gen_rust.cpp

conan/travis/build.sh

rust/flatbuffers/src/builder.rs

josephlr · 2021-10-19T08:46:26Z

tests/rust_usage_test/tests/no_std_test.rs

+            my_game::example::Test::new(100, 101)
+        ][..]
+    );
+}


Be sure to run rustfmt on this file (should have a trailing newline).

Perhaps there should be a CI check for formating. Generated files aside, almost everything has changes when cargo fmt is run.

rust/flatbuffers/Cargo.toml

josephlr · 2021-10-19T09:00:58Z

@jrobsonchase I think there are some more std usages that got missed.

If you run cargo build --no-default-features in the flatbuffers/rust/flatbuffers directory, the compiler should raise an error on the remaining usages. It looks like it's almost all just repacing std with core.

josephlr · 2021-10-19T09:26:33Z

There's also the issue that thiserror doesn't yet support no_std. So it seems like an on-by-default "std" feature will be necessary (as that crate is used to derive the std::error::Error implementation for InvalidFlatbuffer). I've run into this issue before with getrandom, it's not too bad to fix.

First you want to make the dependency on thiserror optional, triggered by the "std", and on by default (as to not have a breaking change). So in the Cargo.toml do:

[dependencies]
smallvec = "1.6.1"
bitflags = "1.2.1"
thiserror = { version = "1.0.23", optional = true }

[features]
default = ["std"]
# Enable the std::error::Error implementation for InvalidFlatbuffer
std = ["thiserror"]

Then in verifier.rs:

// The thiserror crate requires use of libstd.
#[cfg(feature = "std")]
extern crate std;
#[cfg(feature = "std")]
use thiserror::Error;

/// Describes how a flatuffer is invalid and, for data errors, roughly where. No extra tracing
/// information is given for DoS detecting errors since it will probably be a lot.
#[derive(Clone, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "std", derive(Error))]
pub enum InvalidFlatbuffer {
    #[cfg_attr(feature = "std", error("Missing required field `{required}`.\n{error_trace}"))]
    MissingRequiredField {
        required: &'static str,
        error_trace: ErrorTrace,
    },
    // etc...

This requires that you've made the changes to lib.rs (the the comment above) to make the crate unconditionally no_std.

jrobsonchase · 2021-10-19T11:05:54Z

Thanks for the review - seems like things drifted further than I had thought in the time it was left to languish.

Also re-run generator on sample.

This includes making thiserror optional. Additionally, tests are now run both with and without default features, which will ensure that the runtime continues to build/run without std support.

jrobsonchase · 2021-10-19T12:48:05Z

@josephlr Ready for another look

dbaileychess · 2021-12-13T21:50:26Z

@josephlr @jrobsonchase What's the status of this review?

dbaileychess · 2022-01-29T02:23:12Z

Does #6989 solve this issue for you?

dbaileychess · 2022-02-03T16:45:40Z

Closing seeing #6989 seems to address the same thing. Please reopen if not the case.

jrobsonchase force-pushed the no_std branch from e991490 to bc30239 Compare May 22, 2019 11:50

jrobsonchase force-pushed the no_std branch 4 times, most recently from d55d378 to 4ee7bcb Compare May 22, 2019 17:58

rw suggested changes May 29, 2019

View reviewed changes

tests/RustTest.sh Outdated Show resolved Hide resolved

jrobsonchase force-pushed the no_std branch from 0e5b4ef to 0c6129d Compare May 30, 2019 00:07

aardappel force-pushed the master branch 3 times, most recently from f381fd7 to 31f8799 Compare December 26, 2019 20:36

aardappel force-pushed the master branch from 224428b to 0dba639 Compare March 12, 2020 22:12

aardappel force-pushed the master branch 4 times, most recently from 6290611 to f116b80 Compare May 5, 2020 20:42

github-actions bot added c++ codegen Involving generating code from schema labels Jun 19, 2021

github-actions bot removed the stale label Jun 20, 2021

Support alloc-only no_std platforms

8b5e920

jrobsonchase force-pushed the no_std branch from 4ad0cb7 to 9cfc393 Compare September 24, 2021 19:30

github-actions bot added the grpc label Sep 24, 2021

jrobsonchase requested a review from rw September 27, 2021 17:19

josephlr suggested changes Oct 19, 2021

View reviewed changes

jrobsonchase added 2 commits October 19, 2021 08:41

Update rust generator for no_std

d10ca85

Also re-run generator on sample.

Add no_std tests

6e11369

jrobsonchase force-pushed the no_std branch from 9cfc393 to 577a963 Compare October 19, 2021 12:43

jrobsonchase added 3 commits October 19, 2021 08:45

rust/flatbuffers: Fix lingering references to std

6754d5a

This includes making thiserror optional. Additionally, tests are now run both with and without default features, which will ensure that the runtime continues to build/run without std support.

rust/flatbuffers: Clean up imports

3f83fcc

rust/no_std_test: Stop using deprecated get_ functions

fcb744f

jrobsonchase force-pushed the no_std branch from 577a963 to fcb744f Compare October 19, 2021 12:45

MarWit mentioned this pull request Dec 13, 2021

rust: Allow for usage of flatbuffers in #![no_std] environment #6989

Merged

dbaileychess closed this Feb 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Rust] Support alloc-only no_std platforms #5366

[Rust] Support alloc-only no_std platforms #5366

jrobsonchase commented May 22, 2019

googlebot commented May 22, 2019

googlebot commented May 22, 2019

jrobsonchase commented May 22, 2019

rw commented May 23, 2019 •

edited

Loading

jrobsonchase commented May 23, 2019

rw commented May 24, 2019

rw commented May 24, 2019

jrobsonchase commented May 24, 2019

rw commented Jun 8, 2019

jrobsonchase commented Jun 9, 2019

jrobsonchase commented Jun 26, 2019

rw commented Jun 28, 2019

rw commented Oct 3, 2019

bbqsrc commented Feb 12, 2020

TheVova commented Feb 16, 2020

domenukk commented Jun 19, 2021

bbqsrc commented Jun 20, 2021

jacobrosenthal commented Jun 20, 2021

bbqsrc commented Jun 21, 2021

CasperN commented Jun 21, 2021

jrobsonchase commented Oct 18, 2021

josephlr left a comment

josephlr Oct 19, 2021

jrobsonchase Oct 19, 2021

josephlr commented Oct 19, 2021

josephlr commented Oct 19, 2021

jrobsonchase commented Oct 19, 2021

jrobsonchase commented Oct 19, 2021

dbaileychess commented Dec 13, 2021

dbaileychess commented Jan 29, 2022

dbaileychess commented Feb 3, 2022

[Rust] Support alloc-only no_std platforms #5366

[Rust] Support alloc-only no_std platforms #5366

Conversation

jrobsonchase commented May 22, 2019

googlebot commented May 22, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

googlebot commented May 22, 2019

jrobsonchase commented May 22, 2019

rw commented May 23, 2019 • edited Loading

jrobsonchase commented May 23, 2019

rw commented May 24, 2019

rw commented May 24, 2019

jrobsonchase commented May 24, 2019

rw commented Jun 8, 2019

jrobsonchase commented Jun 9, 2019

jrobsonchase commented Jun 26, 2019

rw commented Jun 28, 2019

rw commented Oct 3, 2019

bbqsrc commented Feb 12, 2020

TheVova commented Feb 16, 2020

domenukk commented Jun 19, 2021

bbqsrc commented Jun 20, 2021

jacobrosenthal commented Jun 20, 2021

bbqsrc commented Jun 21, 2021

CasperN commented Jun 21, 2021

jrobsonchase commented Oct 18, 2021

josephlr left a comment

Choose a reason for hiding this comment

josephlr Oct 19, 2021

Choose a reason for hiding this comment

jrobsonchase Oct 19, 2021

Choose a reason for hiding this comment

josephlr commented Oct 19, 2021

josephlr commented Oct 19, 2021

jrobsonchase commented Oct 19, 2021

jrobsonchase commented Oct 19, 2021

dbaileychess commented Dec 13, 2021

dbaileychess commented Jan 29, 2022

dbaileychess commented Feb 3, 2022

rw commented May 23, 2019 •

edited

Loading