Fix sizeof(union ...) fallback not compiling with C++ (#415, #494)
When dependent messages inside oneof cannot be found by generator,
it generates fallback using sizeof(union ..) construct. Previously
the union was anonymous, which is valid C but not allowed by C++.
diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py
index be77bc0..6afea96 100755
--- a/generator/nanopb_generator.py
+++ b/generator/nanopb_generator.py
@@ -205,24 +205,27 @@
class EncodedSize:
'''Class used to represent the encoded size of a field or a message.
Consists of a combination of symbolic sizes and integer sizes.'''
- def __init__(self, value = 0, symbols = []):
+ def __init__(self, value = 0, symbols = [], declarations = []):
if isinstance(value, EncodedSize):
self.value = value.value
self.symbols = value.symbols
+ self.declarations = value.declarations
elif isinstance(value, strtypes + (Names,)):
self.symbols = [str(value)]
self.value = 0
+ self.declarations = []
else:
self.value = value
self.symbols = symbols
+ self.declarations = declarations
def __add__(self, other):
if isinstance(other, int):
- return EncodedSize(self.value + other, self.symbols)
+ return EncodedSize(self.value + other, self.symbols, self.declarations)
elif isinstance(other, strtypes + (Names,)):
- return EncodedSize(self.value, self.symbols + [str(other)])
+ return EncodedSize(self.value, self.symbols + [str(other)], self.declarations)
elif isinstance(other, EncodedSize):
- return EncodedSize(self.value + other.value, self.symbols + other.symbols)
+ return EncodedSize(self.value + other.value, self.symbols + other.symbols, self.declarations + other.declarations)
else:
raise ValueError("Cannot add size: " + repr(other))
@@ -238,6 +241,9 @@
else:
return '(' + str(self.value) + ' + ' + ' + '.join(self.symbols) + ')'
+ def get_declarations(self):
+ return '\n'.join(self.declarations)
+
def upperlimit(self):
if not self.symbols:
return self.value
@@ -930,32 +936,35 @@
def encoded_size(self, dependencies):
'''Returns the size of the largest oneof field.'''
largest = 0
- symbols = []
+ dynamic_sizes = {}
for f in self.fields:
size = EncodedSize(f.encoded_size(dependencies))
if size is None or size.value is None:
return None
elif size.symbols:
- symbols.append((f.tag, size.symbols[0]))
+ dynamic_sizes[f.tag] = size
elif size.value > largest:
largest = size.value
- if not symbols:
+ if not dynamic_sizes:
# Simple case, all sizes were known at generator time
- return largest
+ return EncodedSize(largest)
if largest > 0:
# Some sizes were known, some were not
- symbols.insert(0, (0, largest))
+ dynamic_sizes[0] = EncodedSize(largest)
- if len(symbols) == 1:
+ # Couldn't find size for submessage at generation time,
+ # have to rely on macro resolution at compile time.
+ if len(dynamic_sizes) == 1:
# Only one symbol was needed
- return EncodedSize(5, [symbols[0][1]])
+ return dynamic_sizes.values()[0]
else:
# Use sizeof(union{}) construct to find the maximum size of
# submessages.
- union_def = ' '.join('char f%d[%s];' % s for s in symbols)
- return EncodedSize(5, ['sizeof(union{%s})' % union_def])
+ union_name = "%s_%s_size_union" % (self.struct_name, self.name)
+ union_def = 'union %s {%s};\n' % (union_name, ' '.join('char f%d[%s];' % (k, s) for k,s in dynamic_sizes.items()))
+ return EncodedSize(0, ['sizeof(%s)' % union_name], [union_def])
def has_callbacks(self):
return bool([f for f in self.fields if f.has_callbacks()])
@@ -1599,6 +1608,7 @@
msize = msg.encoded_size(self.dependencies)
identifier = '%s_size' % msg.name
if msize is not None:
+ yield msize.get_declarations()
yield '#define %-40s %s\n' % (identifier, msize)
else:
yield '/* %s depends on runtime parameters */\n' % identifier
diff --git a/tests/regression/issue_494/SConscript b/tests/regression/issue_494/SConscript
new file mode 100644
index 0000000..3ad59a3
--- /dev/null
+++ b/tests/regression/issue_494/SConscript
@@ -0,0 +1,22 @@
+# Regression test for #494:
+# Using sizeof on anonymous union not allowed in C++, in message_size oneof sizing fallback
+
+Import('env')
+import os, sys
+
+# The build rules here are a bit tricky to make the normal dependency
+# resolution intentionally fail. This causes the generator to use the fallback
+# define which had the problem with C++.
+
+generator_cmd = os.path.join(env['NANOPB'], 'generator-bin', 'nanopb_generator' + env['PROGSUFFIX'])
+if not os.path.exists(generator_cmd):
+ generator_cmd = sys.executable + " " + os.path.join(env['NANOPB'], 'generator', 'nanopb_generator.py')
+
+env.Command("oneof.pb", "oneof.proto", "$PROTOC $PROTOCFLAGS -Ibuild/regression/issue_494 -o$TARGETS $SOURCES")
+env.Command(["oneof.pb.c", "oneof.pb.h"], "oneof.pb", generator_cmd + " -Dbuild/regression/issue_494 $SOURCES")
+env.NanopbProto("submessage.proto")
+env.Depends("oneof.pb", "submessage.proto")
+
+test = env.Program(["oneof_size.cc"])
+env.Depends(test, "oneof.pb.h")
+env.RunTest(test)
diff --git a/tests/regression/issue_494/oneof.proto b/tests/regression/issue_494/oneof.proto
new file mode 100644
index 0000000..92d8bc2
--- /dev/null
+++ b/tests/regression/issue_494/oneof.proto
@@ -0,0 +1,13 @@
+syntax = "proto3";
+
+import "submessage.proto";
+
+message MyMessage
+{
+ oneof foo
+ {
+ SubMessage1 msg1 = 1;
+ SubMessage2 msg2 = 2;
+ SubMessage3 msg3 = 3;
+ }
+}
diff --git a/tests/regression/issue_494/oneof_size.cc b/tests/regression/issue_494/oneof_size.cc
new file mode 100644
index 0000000..493166c
--- /dev/null
+++ b/tests/regression/issue_494/oneof_size.cc
@@ -0,0 +1,19 @@
+#include "oneof.pb.h"
+#include "unittests.h"
+
+int main()
+{
+ int status = 0;
+
+ // Expected maximum encoded size:
+ // 1 byte for MyMessage.foo tag
+ // 1-5 bytes for MyMessage.foo submsg length
+ // 1 byte for SubMessage3.foo tag
+ // 5 bytes for SubMessage3.foo value
+ // 1 byte for SubMessage3.bar tag
+ // 5 bytes for SubMessage3.bar value
+ printf("Size: %d\n", (int)MyMessage_size);
+ TEST(MyMessage_size == 18);
+
+ return status;
+}
diff --git a/tests/regression/issue_494/submessage.proto b/tests/regression/issue_494/submessage.proto
new file mode 100644
index 0000000..585842f
--- /dev/null
+++ b/tests/regression/issue_494/submessage.proto
@@ -0,0 +1,17 @@
+syntax = "proto3";
+
+message SubMessage1
+{
+ uint32 foo = 1;
+}
+
+message SubMessage2
+{
+ uint32 foo = 1;
+}
+
+message SubMessage3
+{
+ uint32 foo = 1;
+ uint32 bar = 2;
+}