This bug is a follow-up to #59: Flatbuffers uses implicit default values for scalars. So all integers, floats, bools, or similar have an implicit default value of 0 or false. This is currently not done by butte. This also breaks compatibility with flatc as we fail to decode messages with default scalars: the field is not present in the buffer and we don't recognize it as a default value.

I've implemented a fix in PR #63 reyk@33c0548

But it depends on PRs #60 and #52 to be fixed before I can create a PR.

difference is unmaintained

The author of the difference crate is unresponsive.

Maintained alternatives:

• dissimilar

• treediff

• diffus

See advisory page for additional details.

Right now we're using anyhow in places that we probably shouldn't be. We should be able to provide much better error messages using thiserror inside the library-ish code and using anyhow only within main.

Unsound impl Follow for bool

The implementation of impl Follow for bool allows to reinterpret arbitrary bytes as a bool.

In Rust bool has stringent requirements for its in-memory representation. Use of this function
allows to violate these requirements and invoke undefined behaviour in safe code.

See advisory page for additional details.

flatc generates Default implementations for the table args. This is especially useful with many fields or when using defaults in the schema.

For example:

table Foo {                                                                                           
  a: int;                                                                                             
  b: int;                                                                                             
  c: int = 3;                                                                                
  d: string;                                                                                          
  e: string (required);                                                                               
}                                                                                                     

flatc:

pub struct FooArgs<'a> {
    pub a: i32,
    pub b: i32,
    pub c: i32,
    pub d: Option<flatbuffers::WIPOffset<&'a  str>>,
    pub e: Option<flatbuffers::WIPOffset<&'a  str>>,
}
impl<'a> Default for FooArgs<'a> {
    #[inline]
    fn default() -> Self {
        FooArgs {
            a: 0,
            b: 0,
            c: 3,
            d: None,
            e: None, // required field
        }
    }
}

butte:

pub struct FooArgs<'a> {
    pub a: i32,
    pub b: i32,
    /// The default value for this field is __3i64__
    pub c: i32,
    pub d: Option<butte::WIPOffset<&'a str>>,
    pub e: butte::WIPOffset<&'a str>,
}

Hi,

I know that butte is still under heavy development but would you mind making a first alpha release for crates.io? Something like 0.0.1–alpha.1 😅

I think it is at a point where it became usable, at least for careful experiments.

Reyk

Hi,

the official Style guide says that namespaces are UpperCamelCase. This doesn't work so well with Rust module names that are in snake_case so flatc converts the namespace names and butte doesn't.

namespace HelloWorld;

butte:

pub mod HelloWorld {

flatc:

pub mod hello_world {

The current parser seems unable to accept field attributes (apparently named metadata in butte-build?). I had a quick try to fix this, but I have no experience with nom 5. @cpcloud maybe you have an idea?

Here's an example of a schema failing to parse:

table HelloReply {
  message: string (required);
}

After tinkering some more with flatc to see what it generates I don't think it makes sense to generate comments for attribute or root_type statements (we still need to accept them in the parser though). file_identifier and file_extension make sense, since they are generated consts.

Originally posted by @cpcloud in #41 (comment)

Hi,

the PR #25 has (exposed) a bug, for example:

table Hello {
    value: int64 = 0;
}

Now uses an i128 for the default values in the Rust code even with the type being i64:

fn add_value(&mut self, value: i64) {
    self.fbb.push_slot::<i64>(Hello::VT_VALUE, value, 0i128);
}

With the following definition:

table MyTable {                                                                                                                                                                                             
    entries: [Entry];                                                                                                                                                                                       
}                                                                                                                                                                                                           

I get different results with flatc and butte. The latter causes a compile-time error when I try to use it:

let entries = builder.create_vector(&[entry]);
let table = MyTable::create(&mut builder, &MyTableArgs { entries });

Error:

expected struct butte::primitives::WIPOffset, found struct butte::primitives::ForwardsUOffset

The generated code seems to be the problem (ForwardsUOffset vs. WIPOffset), flatc:

pub struct MyTableArgs<'a> {                                                                                                                                                                                
    pub entries: Option<flatbuffers::WIPOffset<flatbuffers::Vector<'a , flatbuffers::ForwardsUOffset<Entry<'a >>>>>,                                                                                        
}                                                                                                                                                                                                           

butte:

pub struct MyTableArgs<'a> {                                                                                                                                                                                
    #[allow(clippy::type_complexity)]                                                                                                                                                                       
    pub entries: Option<butte::WIPOffset<butte::Vector<'a, butte::WIPOffset<Entry<&'a [u8]>>>>>,                                                                                                            
}                                                                                                                                                                                                           

DISCLAIMER: I'm using a non-default clippy configuration.

With that, I have to use the following block for the generated Rust code:

#![allow(
    clippy::unreadable_literal,
    clippy::empty_enum,
    clippy::unseparated_literal_suffix
)]
#![allow(single_use_lifetimes)]

This request is a "nice-to-have", not a real issue. But as the flatbuffers crate generates quite outdated and "dirty" rust code, it would be a nice benefit if butte could try to be as rust-2018-clean as possible. Either by adding the allow directives in the generated code or by generating it in the desired style.

• clippy::unreadable_literal: this happens with very large default values, e.g.:

  table Hello {
      value: i64 = 1000000;
  }
  

  Clippy would expect 1_000_000. The FlatBuffers schema doesn't support this and converting the format would probably be an excessive step, but it could be a case for an [allow(clippy::unreadable_literal)] in the generated code.

• clippy::empty_enum: this happens with the offset enums, e.g.:

  pub enum HelloOffset {}

• clippy::unseparated_literal_suffix: this is part of the vtable, e.g.:

  pub const VT_ID: butte::VOffsetT = 4u16;
                                     ^^^^ help: add an underscore: 4_u16

• single_use_lifetimes: This warning is a bit annoying since it generates too many false positives in perfectly valid code. So I should better fix my configuration ;-). Nevertheless, it appears in a number of places:

  impl<'a> HelloOptions<&'a [u8]> {
       ^^ this...        ^^ ...is used only here     
  ``

Hi,

the code coverage test has multiple issues:

• rustfmt is required and missing in coverage.yml.
• Incremental compilation is incompatible with -Zprofile (should add CARGO_INCREMENTAL=0).
• Even with that fixed, the butte-examples build script still fails with errors like:
  butte_build-*.gcda: cannot merge previous GCDA file: corrupt arc tag (0xXXXXXXXX)
  (this only fails in the code coverage test and seems to be related to -Zprofile).

Reyk

The spec supports /// doc comments:

May be written as in most C-based languages. Additionally, a triple comment (///) on a line by itself signals that a comment is documentation for whatever is declared on the line after it (table/struct/field/enum/union/element), and the comment is output in the corresponding C++ code. Multiple such lines per item are allowed.

It would be nice to preserve them in the output.

Additionally, butte fails with doc comments before fields:

Error: Parse was incomplete: not parsed: table MyTable {
    /// foo
   ...

flatc supports fixed-sized arrays of structs. But only for C++ output, not for Rust output 👎

This enhancement is a nice-to-have. But the nice thing about this is that it allows to enforce the size in the schema and not just in the code.

key: [ubyte:32];

or

key: [ubyte] (size: 32); 

The current implementation in butte-build allows only scalars as default field values, matching the grammar at https://google.github.io/flatbuffers/flatbuffers_grammar.html

However, looking at the canonical Flatbuffers example, we see default values can also be enum values. See https://google.github.io/flatbuffers/flatbuffers_guide_writing_schema.html

(I'll implement that PR)

This is a follow up to #41, because it's a fair amount of additional code and will be easier to review in a separate PR.

butte encodes default values correctly but it doesn't handle them on the receiving side. Flatbuffers fields aren't encoded in the buffer if they match the default value. On the receiving side, a default value and the absence of it in the buffer is indicated by an offset of 0 in the vtable. butte erroneously handles the returned 0 as an offset value and returns garbage from the buffer.

The official docs has the following example under Unions:

table PointPosition { x:uint; y:uint; }
table MarkerPosition {}
union Position {
  Start:MarkerPosition,
  Point:PointPosition,
  Finish:MarkerPosition
}

This is useful when having the same type for different actions. The workaround is to wrap MarketPosition in two tables Start and Finish.

Hi,

I'm not quite sure if this is a bug or if I'm doing something wrong. With the following definition:

table Hello {
    value: string;
}

I get a the Option and a Default implementation as expected:

pub struct HelloArgs<'a> {
    pub value: Option<butte::WIPOffset<&'a str>>,
}
impl<'a> Default for HelloArgs<'a> {
  fn default() -> Self {
    Self { value: None }
  }
}

But it now fails on runtime with the missing required field value error. I think it is related to the fact that the create() method only calls the internal add_value() method if the field is Some but the finish() step fails because Hello::VT_VALUE is not set.

        if let Some(x) = args.value {
            builder.add_value(x);
        }

...add_value() sets the Hello::VT_VALUE slot...

        self.fbb.required(o, Hello::VT_VALUE, "value");