Fix compiler errors with complex oneof hierarchy and sizeof() (#610)
Previously the generator would refer to internal symbols generated
by OneOf.encoded_size(). However the include file might not have
those symbols if they were resolved at generator time.
This commit changes it so that the generator only refers to symbols
that are part of public API for the other include files.
diff --git a/generator/nanopb_generator.py b/generator/nanopb_generator.py
index c3e64d7..aabdddf 100755
--- a/generator/nanopb_generator.py
+++ b/generator/nanopb_generator.py
@@ -749,16 +749,24 @@
submsg = dependencies[str(self.submsgname)]
other_dependencies = dict(x for x in dependencies.items() if x[0] != str(self.struct_name))
encsize = submsg.encoded_size(other_dependencies)
+
+ my_msg = dependencies.get(str(self.struct_name))
+ external = (not my_msg or submsg.protofile != my_msg.protofile)
+
+ if encsize and encsize.symbols and external:
+ # Couldn't fully resolve the size of a dependency from
+ # another file. Instead of including the symbols directly,
+ # just use the #define SubMessage_size from the header.
+ encsize = None
+
if encsize is not None:
# Include submessage length prefix
encsize += varint_max_size(encsize.upperlimit())
- else:
- my_msg = dependencies.get(str(self.struct_name))
- if my_msg and submsg.protofile == my_msg.protofile:
- # The dependency is from the same file and size cannot be
- # determined for it, thus we know it will not be possible
- # in runtime either.
- return None
+ elif not external:
+ # The dependency is from the same file and size cannot be
+ # determined for it, thus we know it will not be possible
+ # in runtime either.
+ return None
if encsize is None:
# Submessage or its size cannot be found.
@@ -1000,7 +1008,7 @@
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()))
required_defs = list(itertools.chain.from_iterable(s.required_defines for k,s in dynamic_sizes.items()))
- return EncodedSize(0, ['sizeof(%s)' % union_name], [union_def], required_defs)
+ return EncodedSize(0, ['sizeof(union %s)' % union_name], [union_def], required_defs)
def has_callbacks(self):
return bool([f for f in self.fields if f.has_callbacks()])
diff --git a/tests/regression/issue_610/SConscript b/tests/regression/issue_610/SConscript
index 8fe80d2..1c6e276 100644
--- a/tests/regression/issue_610/SConscript
+++ b/tests/regression/issue_610/SConscript
@@ -3,8 +3,28 @@
Import("env")
+# First the simple case with two files
env.NanopbProto("LogMessage.proto")
env.NanopbProto(["DataPacket.proto", "LogMessage.proto"])
env.Object("LogMessage.pb.c")
env.Object("DataPacket.pb.c")
+# Then the complex hierarchy case
+all_files = [
+'nanopb_generator_bug/first/a/aa.proto',
+'nanopb_generator_bug/first/a/ab.proto',
+'nanopb_generator_bug/first/a.proto',
+'nanopb_generator_bug/first/b/ba.proto',
+'nanopb_generator_bug/first/b.proto',
+'nanopb_generator_bug/first.proto'
+]
+
+env2 = env.Clone()
+env2.Append(CPPPATH = "$BUILD/regression/issue_610")
+
+for f in all_files:
+ env2.NanopbProto([f] + all_files)
+
+for f in all_files:
+ env2.Object(f.replace('.proto', '.pb.c'))
+
diff --git a/tests/regression/issue_610/nanopb_generator_bug/first.proto b/tests/regression/issue_610/nanopb_generator_bug/first.proto
new file mode 100644
index 0000000..613c6db
--- /dev/null
+++ b/tests/regression/issue_610/nanopb_generator_bug/first.proto
@@ -0,0 +1,11 @@
+syntax="proto3";
+
+import "nanopb_generator_bug/first/a.proto";
+import "nanopb_generator_bug/first/b.proto";
+
+message First {
+ oneof oneof_first {
+ first.A obj_a = 1;
+ first.B obj_b = 2;
+ }
+}
diff --git a/tests/regression/issue_610/nanopb_generator_bug/first/a.proto b/tests/regression/issue_610/nanopb_generator_bug/first/a.proto
new file mode 100644
index 0000000..a11b304
--- /dev/null
+++ b/tests/regression/issue_610/nanopb_generator_bug/first/a.proto
@@ -0,0 +1,13 @@
+syntax="proto3";
+
+package first;
+
+import "nanopb_generator_bug/first/a/aa.proto";
+import "nanopb_generator_bug/first/a/ab.proto";
+
+message A {
+ oneof oneof_a {
+ a.A obj_a = 1;
+ a.B obj_b = 2;
+ }
+}
diff --git a/tests/regression/issue_610/nanopb_generator_bug/first/a/aa.proto b/tests/regression/issue_610/nanopb_generator_bug/first/a/aa.proto
new file mode 100644
index 0000000..d522abb
--- /dev/null
+++ b/tests/regression/issue_610/nanopb_generator_bug/first/a/aa.proto
@@ -0,0 +1,5 @@
+syntax="proto3";
+
+package first.a;
+
+message A {}
diff --git a/tests/regression/issue_610/nanopb_generator_bug/first/a/ab.proto b/tests/regression/issue_610/nanopb_generator_bug/first/a/ab.proto
new file mode 100644
index 0000000..8f2cda6
--- /dev/null
+++ b/tests/regression/issue_610/nanopb_generator_bug/first/a/ab.proto
@@ -0,0 +1,5 @@
+syntax="proto3";
+
+package first.a;
+
+message B {}
diff --git a/tests/regression/issue_610/nanopb_generator_bug/first/b.proto b/tests/regression/issue_610/nanopb_generator_bug/first/b.proto
new file mode 100644
index 0000000..530bd94
--- /dev/null
+++ b/tests/regression/issue_610/nanopb_generator_bug/first/b.proto
@@ -0,0 +1,11 @@
+syntax="proto3";
+
+package first;
+
+import "nanopb_generator_bug/first/b/ba.proto";
+
+message B {
+ oneof oneof_b {
+ b.A obj_a = 1;
+ }
+}
diff --git a/tests/regression/issue_610/nanopb_generator_bug/first/b/ba.proto b/tests/regression/issue_610/nanopb_generator_bug/first/b/ba.proto
new file mode 100644
index 0000000..6b301f5
--- /dev/null
+++ b/tests/regression/issue_610/nanopb_generator_bug/first/b/ba.proto
@@ -0,0 +1,5 @@
+syntax="proto3";
+
+package first.b;
+
+message A {}
