Embedded-AMS / EmbeddedProto

Embedded Proto is a C++ Protocol Buffers implementation specifically suitable for microcontrollers. It is small, reliable and easy to use.
https://embeddedproto.com
210 stars 38 forks source link

Cannot have a member named `id` #23

Closed sgerbino closed 3 years ago

sgerbino commented 3 years ago

Hello, great project thanks for sharing!

I have a proto file that contains a member id:

message block_topology {
   bytes id = 1;
   uint64 height = 2;
   bytes previous = 3;
}

The generated code uses an enum id and causing a conflict while compiling.

Related error:

In file included from /Users/sgerbino/Projects/koinos-system-contracts/contracts/echo/echo.cpp:1:
/Users/sgerbino/Projects/koinos-system-contracts/contracts/echo/common.h:142:7: error: must use 'enum' tag to refer to type 'id' in this scope
      id id_tag = id::NOT_SET;
      ^
      enum
/Users/sgerbino/Projects/koinos-system-contracts/contracts/echo/common.h:99:27: note: enum 'id' is hidden by a non-type declaration of 'id' here
    inline const uint8_t* id() const { return id_.get_const(); }
                          ^
/Users/sgerbino/Projects/koinos-system-contracts/contracts/echo/common.h:147:30: error: must use 'enum' tag to refer to type 'id' in this scope
        id_tag = static_cast<id>(id_number);
                             ^
                             enum
/Users/sgerbino/Projects/koinos-system-contracts/contracts/echo/common.h:99:27: note: enum 'id' is hidden by a non-type declaration of 'id' here
    inline const uint8_t* id() const { return id_.get_const(); }
BartHertog commented 3 years ago

Hello sgerbio,

Thank you for taking the time to report this issue. It was bound to happen some time, lucky the fix is simple.

The enum class which holds all the field numbers is called id. The definition of this enum conflicts with the definition of the getter function for your field called id. A preliminary fix is made by renaming the enum from id to ID. The chance of another name conflict is however ever present.

Because this is also a breaking change for anybody using oneof's, some more consideration will be taken on a proper way to prevent name collisions in the future. A fix will however be available soon.

Best regards,

Bart

BartHertog commented 3 years ago

Hello sgerbio,

After deliberation two decisions where made:

  1. The enumeration was renamed to FieldNumber to be consisted with the Google protobuf naming conversion.
  2. The formal release of this breaking change for oneof field users has been postponed to version 3.0.0. An release candidate branch has been made and push for you to use if desired.

Best regards,

Bart