flatbuffers Flatc converts enums within proto messages to wrong namespace in fbs [all languages, FlatBuffers 1.12, 2.0]

We are trying out Flatbuffers instead of the currently used Protobuf in the ASAM Open Simulation Interface.

We are converting interface definitions from proto files to fbs using flatc. In the proto files we have nested messages with enums of the same name. See the following example from the osi_object.proto:

message MovingObject
{
  ...
  optional Type type = 3;
  ...
  optional VehicleClassification vehicle_classification = 6;
  enum Type
  {
    TYPE_UNKNOWN = 0;
    TYPE_OTHER = 1;
    TYPE_VEHICLE = 2;
    TYPE_PEDESTRIAN = 3;
    TYPE_ANIMAL = 4;
  }
  message VehicleClassification
  {
    enum Type
    {
    TYPE_UNKNOWN = 0;
    TYPE_OTHER = 1;
    TYPE_SMALL_CAR = 2;
    TYPE_COMPACT_CAR = 3;
    …
    }
  }
}

The namespaces of the enums are correctly set by flatc in the fbs files:

namespace osi3.MovingObject_;
enum Type : int {
  TYPE_UNKNOWN = 0,
  TYPE_OTHER = 1,
  TYPE_VEHICLE = 2,
  TYPE_PEDESTRIAN = 3,
  TYPE_ANIMAL = 4,
}
namespace osi3.MovingObject_.VehicleClassification_;
enum Type : int {
  TYPE_UNKNOWN = 0,
  TYPE_OTHER = 1,
  TYPE_SMALL_CAR = 2,
  TYPE_COMPACT_CAR = 3,
  …
}

The usage of the enums however is wrong:

table MovingObject {
  ...
  type:osi3.StationaryObject_.Classification_.Type;  //should be type:osi3.MovingObject_.Type;
  ...
  vehicle_classification:osi3.MovingObject_.VehicleClassification;
}

namespace osi3.MovingObject_;
table VehicleClassification {
  type:osi3.MovingObject_.Type;  //should be type:osi3.MovingObject_.VehicleClassification_.Type;
  ...
}

The namespaces of the enums "Type" are mixed up.

Feb 18 '22 15:02 ClemensLinnhoff

Thanks for the report. Could you stripe down the proto to a bare minimum example and also post the command line arguments you are using? I can take a look.

Feb 22 '22 04:02 dbaileychess

Can you look at #7121, I tried to recreate the issue you are seeing and it seems to be generating it correctly. Maybe there is some other tidbit that is different.

Feb 22 '22 06:02 dbaileychess

Thank you for your reply. I did a little more testing and it seems that the order of defining the enums and defining the messages using the enums plays a role. I attached two minimal examples.

osi_object_minimal.proto contains the relevant enums and messages from the original proto file of the open simulation interface. MovingObject.Type and the MovingObject.VehicleClassification.Type are wrong.
In osi_object_minimal_different_order.proto I reversed the order of defining the enums and the messages. Here the types are correctly assigned.

As a command to convert the proto file to fbs I simply use the following command: ./flatc --proto osi_object_minimal.proto

osi_object_minimal.zip

Feb 25 '22 08:02 ClemensLinnhoff

Thanks, I'll take a look.

Feb 28 '22 16:02 dbaileychess

Hi @dbaileychess! Any progress on our issue here? Did you have a look? We would be very happy , if this could be fixed!

Nov 03 '22 09:11 PhRosenberger

Thanks, I'll take a look.

Hey @dbaileychess do you need any more information on this or support? Is there already a plan to integrate a fix in the next release? Could you outline a timeframe for this? Thank you very much!

Jan 19 '23 14:01 jdsika

At an initial glance this bug seems to be the result of how we parse proto files. We can be reproduces this bug if the following criteria are met:

The enum declaration is defined after the field that references it.
There exists an enum of the same name that is defined above the field.

When we perform an enum look up we traverse up the namespace until we find a matching qualified name in the symbol table for defined enums. https://github.com/google/flatbuffers/blob/master/src/idl_parser.cpp#L141-L147

If our desired enum is defined after the field it won't exists in the symbols table so our look up function will travel up the namespace to find an enum with a similiar name.

To fix this issue, off the top of my head I think we can do a two pass on the proto file. On the initial pass we can populate our symbol tables and on the second pass we'll process the proto file like we currently do.

Here's another minimal example to reproduce this bug.

Example

syntax = "proto2";

package bug;

enum Error {
  UNKNOWN = 0;
  INTERNAL = 1;
}

message Outer {
  optional Error outer_error = 1;
  enum Error {
    UNKNOWN = 0;
    INTERNAL = 1;
  }

  message Inner {
    optional Error inner_error = 1;
    enum Error {
      UNKNOWN = 0;
      INTERNAL = 1;
    }
  }
}

// Generated from minimal.proto

namespace bug;

enum Error : int {
  UNKNOWN = 0,
  INTERNAL = 1,
}

namespace bug.Outer_;

enum Error : int {
  UNKNOWN = 0,
  INTERNAL = 1,
}

namespace bug.Outer_.Inner_;

enum Error : int {
  UNKNOWN = 0,
  INTERNAL = 1,
}

namespace bug;

table Outer {
  outer_error:bug.Error;
}

namespace bug.Outer_;

table Inner {
  inner_error:bug.Outer_.Error;
}

Jan 26 '23 08:01 le-michael

Blocked by #7827

Feb 17 '23 01:02 le-michael

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

Aug 18 '23 20:08 github-actions[bot]

Seems that this is not fixed yet?

Aug 20 '23 09:08 jdsika

Not fixed yet. I'll take a look at this again.

Aug 22 '23 17:08 le-michael

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

Feb 20 '24 20:02 github-actions[bot]

This issue still needs to be fixed.

Feb 21 '24 06:02 ClemensLinnhoff