Fix issue with unknown message sizes being referred inside oneof encoded size calculation (#569)
Nanopb generator attempts to calculate the maximum encoded size of messages
when possible. If the message size is unbounded due to e.g. callbacks, the
define for it will be skipped. However, for oneofs we need to calculate the
maximum of all fields inside it, which is not easily done in C in compile time.
The current method uses sizeof(union { .. }) to calculate the largest value.
A recent change in 3ff6743173 (#415, #494) moved the union to a separate line.
This in turn caused compilation errors when the union was defined but one of
the symbols it depends on (from another .proto file) was not defined.
This commit fixes it by adding #if defined() guard around the definition.
diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py
index e778593..b04135e 100755
--- a/generator/nanopb_generator.py
+++ b/generator/nanopb_generator.py
@@ -211,33 +211,38 @@
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 = [], declarations = []):
+ def __init__(self, value = 0, symbols = [], declarations = [], required_defines = []):
if isinstance(value, EncodedSize):
self.value = value.value
self.symbols = value.symbols
self.declarations = value.declarations
+ self.required_defines = value.required_defines
elif isinstance(value, strtypes + (Names,)):
self.symbols = [str(value)]
self.value = 0
self.declarations = []
+ self.required_defines = [str(value)]
else:
self.value = value
self.symbols = symbols
self.declarations = declarations
+ self.required_defines = required_defines
def __add__(self, other):
if isinstance(other, int):
- return EncodedSize(self.value + other, self.symbols, self.declarations)
+ return EncodedSize(self.value + other, self.symbols, self.declarations, self.required_defines)
elif isinstance(other, strtypes + (Names,)):
- return EncodedSize(self.value, self.symbols + [str(other)], self.declarations)
+ return EncodedSize(self.value, self.symbols + [str(other)], self.declarations, self.required_defines + [str(other)])
elif isinstance(other, EncodedSize):
- return EncodedSize(self.value + other.value, self.symbols + other.symbols, self.declarations + other.declarations)
+ return EncodedSize(self.value + other.value, self.symbols + other.symbols,
+ self.declarations + other.declarations, self.required_defines + other.required_defines)
else:
raise ValueError("Cannot add size: " + repr(other))
def __mul__(self, other):
if isinstance(other, int):
- return EncodedSize(self.value * other, [str(other) + '*' + s for s in self.symbols])
+ return EncodedSize(self.value * other, [str(other) + '*' + s for s in self.symbols],
+ self.declarations, self.required_defines)
else:
raise ValueError("Cannot multiply size: " + repr(other))
@@ -248,8 +253,18 @@
return '(' + str(self.value) + ' + ' + ' + '.join(self.symbols) + ')'
def get_declarations(self):
+ '''Get any declarations that must appear alongside this encoded size definition,
+ such as helper union {} types.'''
return '\n'.join(self.declarations)
+ def get_cpp_guard(self, local_defines):
+ '''Get an #if preprocessor statement listing all defines that are required for this definition.'''
+ needed = [x for x in self.required_defines if x not in local_defines]
+ if needed:
+ return '#if ' + ' && '.join(['defined(%s)' % x for x in needed]) + "\n"
+ else:
+ return ''
+
def upperlimit(self):
if not self.symbols:
return self.value
@@ -977,7 +992,8 @@
# submessages.
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])
+ required_defs = sum([s.required_defines for k,s in dynamic_sizes.items()], start = [])
+ return EncodedSize(0, ['sizeof(%s)' % union_name], [union_def], required_defs)
def has_callbacks(self):
return bool([f for f in self.fields if f.has_callbacks()])
@@ -1622,12 +1638,21 @@
yield '\n'
yield '/* Maximum encoded size of messages (where known) */\n'
+ messagesizes = []
for msg in self.messages:
- msize = msg.encoded_size(self.dependencies)
identifier = '%s_size' % msg.name
+ messagesizes.append((identifier, msg.encoded_size(self.dependencies)))
+
+ # If we require a symbol from another file, put a preprocessor if statement
+ # around it to prevent compilation errors if the symbol is not actually available.
+ local_defines = [identifier for identifier, msize in messagesizes if msize is not None]
+ for identifier, msize in messagesizes:
if msize is not None:
+ cpp_guard = msize.get_cpp_guard(local_defines)
+ yield cpp_guard
yield msize.get_declarations()
yield '#define %-40s %s\n' % (identifier, msize)
+ if cpp_guard: yield "#endif\n"
else:
yield '/* %s depends on runtime parameters */\n' % identifier
yield '\n'
diff --git a/tests/regression/issue_569/SConscript b/tests/regression/issue_569/SConscript
new file mode 100644
index 0000000..a3ec773
--- /dev/null
+++ b/tests/regression/issue_569/SConscript
@@ -0,0 +1,16 @@
+# Regression test for #569:
+# Compilation error when dependent message size is not defined.
+
+Import("env")
+
+# For C compiler
+env.NanopbProto("a")
+env.NanopbProto("b")
+env.Object("a.pb.c")
+env.Object("b.pb.c")
+
+# For C++ compiler
+env.NanopbProtoCpp("a")
+env.NanopbProtoCpp("b")
+env.Object("a_cpp.o", "a.pb.cpp")
+env.Object("b_cpp.o", "b.pb.cpp")
diff --git a/tests/regression/issue_569/a.proto b/tests/regression/issue_569/a.proto
new file mode 100644
index 0000000..94cbaad
--- /dev/null
+++ b/tests/regression/issue_569/a.proto
@@ -0,0 +1,14 @@
+syntax = "proto3";
+
+package a;
+
+message AUnknown {
+ string data = 1;
+}
+
+message A {
+ oneof data {
+ AUnknown unknown = 1;
+ int32 x = 2;
+ }
+}
diff --git a/tests/regression/issue_569/b.proto b/tests/regression/issue_569/b.proto
new file mode 100644
index 0000000..a8bf22c
--- /dev/null
+++ b/tests/regression/issue_569/b.proto
@@ -0,0 +1,19 @@
+syntax = "proto3";
+
+import "a.proto";
+
+package b;
+
+message B {
+ oneof data {
+ a.AUnknown unknown = 1;
+ int32 x = 2;
+ }
+}
+
+message B2 {
+ oneof data {
+ a.AUnknown unknown = 1;
+ a.A unknown2 = 2;
+ }
+}
\ No newline at end of file
