Flatbuffers: Rust: forward and backward compatibility of generated code

[vglavnyy]

How to use a Rust generated code with an evolution growing schema?
Main question is "what to do with panic!?".
I'm novice with Rust and following examples may be wrong.

For example, consider the monster_test.fbs. Let the Monster schema from the master branch is the scheme with version v-0. This scheme has the Race enum:

enum Race:byte {
  None = -1,
  Human = 0,
  Dwarf,
  Elf,
}
table Monster{
  signed_enum:Race = None (id:48);
}

The generated code in the master:
https://github.com/google/flatbuffers/blob/26f238c24888bf02a872f304fe9f03433fd0dd00/tests/monster_test_generated.rs#L1327-L1330

Good use-case 1 (compatible update)
Let's add a new enumerator to the Race and update the schema to v-1:

enum Race:byte {
  None = -1,
  Human = 0,
  Dwarf, // = 1
  Elf,   // = 2
  Halfling = 3,
}

• Let Tower-0 is a monster factory built at age-0 by schema v-0 rules.
• Let Tower-1 is a monster factory built at age-1 by schema v-1 rules.
• Let a Commander-1 is a broadcast server which has newest implementation v-1.

What will the Tower-0 do if it receives a new request with Halfling from the commander?
It will panic due to unwrap() call.

Bad use-case 2 (broken update)
Typically, enum values should only ever be added, never removed.
Let this happen, remove the Elf and update the scheme to v-2.

enum Race:byte {
  None = -1,
  Human = 0,
  Dwarf, // = 1
         // = 2 (removed Elf race)
  Halfling = 3,
}

What will happen?
The Tower-0 will panic with the Halfling request from the Commander-1.
The Tower-1 will panic with the Elf request from a Commander-0.

What should an user-code do for compatibility?
Probably, signed_enum(&self) should return Option<Race> instead of Race.
Or should a program use std::panic::catch_unwind?

Instead of the towers it can be something real (non ideal), AI-robots for example.

[aardappel]

From the pov how FlatBuffers was designed (not necessarily from the pov of Rust), the code should certainly not panic on unknown values. It should just return the unknown value. The user should ignore unknown values.

I believe it was stated in another issue/pr that it is undefined behavior to have an undeclared enum value. FlatBuffer enums have C semantics (they're just integer constants), so if a language's idea of an "enum" is too strongly typed to accommodate that, then typically that language feature should not be used. It should use integer constants instead. See for example Java, where the object based enum feature is not used.

Of course, that is not ergonomic for users of the language, but is is important that FlatBuffers semantics are being preserved.

In Rust, another option would be to return a Maybe type from the accessor.

Or maybe just turn a blind eye to this "undefined behavior" :P

@rw @jean-airoldie

[jean-airoldie]

AFAIK, the rust code generator is currently wrong because it cast a integer as a c enum without doing any bound checks, which would cause undefined behavior in the case of an undeclared enum value. There is no way around this UB until the code generator is fixed.

To fix this code generator issue we would make accessing an enum faillible (because of bounds check) and it would return either a Option<Race> or a Result<Race, i8> (assuming we want to keep the unknown enum value). So this would essentially fix both UB and your enum evolution concern.

[aardappel]

But it wouldn't fix the ergonomic concern. Users having to deal with Option before they know unknown values even exists is odd.

Option is better than Results, since this old code has no business speculating about future values.

[jean-airoldie]

I'm not sure I'm following why Option is not ergonomic for that use case.

[jean-airoldie]

Depending on the context it might make sense to call Option::unwrap and panic, but most of the time you would run a match statement:

match signed_enum() {
  Some(race) => (), // do stuff,
  None => (), // do some other stuff
}

If you know that enum variants will be added in the future and a unknown enum is a recoverable error, than you would do the later. But that would depend on the context.

[vglavnyy]

The Result<Race, i8> with try!() macro is a more generic solution than simple Option<Race>.

Is it possible to generate both getters Option<Race> and Result<Race, i8> using
--gen-object-api as a switch?

[jean-airoldie]

You can use the try operator with Option on nightly with NoneError.

[jean-airoldie]

You could also do Option::ok_or(())? with would map the Option to a Result<()>.

[github-actions[bot]]

This issue is stale because it has been open 6 months with no activity. Please comment or this will be closed in 14 days.

[CasperN]

Can I offer a different design for enums in Rust? With considerations from my draft PR for the object API.

I think we should make

enum Race:byte {
  None = -1,
  Human = 0,
  Dwarf, // = 1
         // = 2 (removed Elf race)
  Halfling = 3,
}

turn into

#[derive(Copy, Clone, Debug)]
struct Race(i8);
impl Race {
  const NONE: i8 = -1;
  const HUMAN: i8 = 0;
  const DWARF: i8 = 1;
  const HALFLING: i8 = 3;
  // --gen-object-api
  fn unpack(self) -> RaceT {
    match self { ... }
  }
}
// --gen-object-api
enum RaceT {
  None,
  Human,
  Dwarf,
  UnknownVariant(i8),
}

Pros:

• Schema writers can still do match race.0 { Race::HUMAN => ... }.
• The c-enum semantics are a lot more obvious.
• struct(i8) has the same binary representation as C while Option<i8> is two bytes, so you don't need to "unpack" when working with arrays of enums (except in the object API).
• This is more consistent with the bit_flags crate, which we should use for flatbuffers-bit-flags. Their representation is struct $Flags { bits: $ty }. Using that does not require optional or result types.

Cons:

• its less native to Rust.