aakash-sahai / nanopb

Automatically exported from code.google.com/p/nanopb
zlib License
0 stars 0 forks source link

Long enum names #38

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In protocol buffers, according to protoc.exe (win32 binary), 

"... enum values use C++ scoping rules, meaning that enum values are siblings 
of their type, not children of it."

For the following test.proto:

/////////////////////////////////////////////////////////////////////
enum ENUM_ONE
{
    FLAG_A = 0;
    FLAG_B = 1;
    MAX = 2;
}

enum ENUM_TWO
{
    FLAG_A = 0;
    FLAG_B = 1;
    MAX = 2;
}

message TEST
{
    optional ENUM_ONE one = 1;
    optional ENUM_TWO two = 2;
}
/////////////////////////////////////////////////////////////////////

Compiling with protoc results in this output:

test.proto:12:9: "FLAG_A" is already defined.
test.proto:12:9: Note that enum values use C++ scoping rules, meaning that enum 
values are siblings of their type, not children of it.  Therefore, "FLAG_A" 
must be unique within the global scope, not just within "ENUM_TWO".
test.proto:13:9: "FLAG_B" is already defined.
test.proto:13:9: Note that enum values use C++ scoping rules, meaning that enum 
values are siblings of their type, not children of it.  Therefore, "FLAG_B" 
must be unique within the global scope, not just within "ENUM_TWO".
test.proto:14:9: "MAX" is already defined.
test.proto:14:9: Note that enum values use C++ scoping rules, meaning that enum 
values are siblings of their type, not children of it.  Therefore, "MAX" must 
be unique within the global scope, not just within "ENUM_TWO".

So to resolve this, we use unique names at file scope so all the siblings are 
happy:

/////////////////////////////////////////////////////////////////////
enum ENUM_ONE
{
    ENUM_ONE_FLAG_A     = 0;
    ENUM_ONE_FLAG_B     = 1;
    ENUM_ONE_MAX        = 2;
}

enum ENUM_TWO
{
    ENUM_TWO_FLAG_A     = 0;
    ENUM_TWO_FLAG_B     = 1;
    ENUM_TWO_MAX        = 2;
}

message TEST
{
    optional ENUM_ONE one = 1;
    optional ENUM_TWO two = 2;
}
/////////////////////////////////////////////////////////////////////

Which results in this output in .pb.c file:

/* Enum definitions */
typedef enum {
    ENUM_ONE_ENUM_ONE_FLAG_A = 0,
    ENUM_ONE_ENUM_ONE_FLAG_B = 1,
    ENUM_ONE_ENUM_ONE_MAX = 2
} ENUM_ONE;

typedef enum {
    ENUM_TWO_ENUM_TWO_FLAG_A = 0,
    ENUM_TWO_ENUM_TWO_FLAG_B = 1,
    ENUM_TWO_ENUM_TWO_MAX = 2
} ENUM_TWO;

You can see the enum name prefixes are duplicated.

Suggested patch to nanopb_generator.py:

@@ -69,13 +69,13 @@
     return Names(type_name[1:].split('.'))

 class Enum:
     def __init__(self, names, desc):
         '''desc is EnumDescriptorProto'''
         self.names = names + desc.name
-        self.values = [(x.name, x.number) for x in desc.value]
+        self.values = [(self.names + x.name, x.number) for x in desc.value]

     def __str__(self):
         result = 'typedef enum {\n'
         result += ',\n'.join(["    %s = %d" % x for x in self.values])
         result += '\n} %s;' % self.names
         return result
@@ -125,12 +125,14 @@
         # CTYPE is the name of the c type to use in the struct.
         if datatypes.has_key(desc.type):
             self.ctype, self.ltype = datatypes[desc.type]
         elif desc.type == FieldD.TYPE_ENUM:
             self.ltype = 'PB_LTYPE_VARINT'
             self.ctype = names_from_type_name(desc.type_name)
+            if self.default is not None:
+                self.default = self.ctype + self.default
         elif desc.type == FieldD.TYPE_STRING:
             self.ltype = 'PB_LTYPE_STRING'
             if self.max_size is None:
                 is_callback = True
             else:
                 self.ctype = 'char'

Original issue reported on code.google.com by jim.qua...@gmail.com on 20 Oct 2012 at 5:00

GoogleCodeExporter commented 9 years ago
Hmm, sounds reasonable.

However, to maintain backwards compatibility, this must be a configurable 
option. Probably a file-level option like [(nanopb).short_names = true].

Original comment by Petteri.Aimonen on 20 Oct 2012 at 8:10

GoogleCodeExporter commented 9 years ago
This issue was updated by revision db1eefc24bd9.

Original comment by Petteri.Aimonen on 29 Oct 2012 at 5:17

GoogleCodeExporter commented 9 years ago
Fixed in nanopb-0.1.7

Original comment by Petteri.Aimonen on 12 Nov 2012 at 7:33