allocate immediately required fields (#1)

Reviewed-on: BigfootDev/flatbuffers-bigfoot#1
Co-authored-by: Romain BOULLARD <romain.boullard@protonmail.com>

CMakeLists.txt

@@ -245,6 +245,8 @@ set(FlatBuffers_Tests_SRCS
   tests/native_type_test_impl.cpp
   tests/bigfoot_ref_test_impl.h
   tests/bigfoot_ref_test_impl.cpp
+  tests/required_table_default_test.h
+  tests/required_table_default_test.cpp
   tests/alignment_test.h
   tests/alignment_test.cpp
   tests/64bit/offset64_test.h
@@ -561,6 +563,7 @@ if(FLATBUFFERS_BUILD_TESTS)
   compile_schema_for_test(tests/native_inline_table_test.fbs "${FLATC_OPT_COMP}")
   compile_schema_for_test(tests/native_type_test.fbs "${FLATC_OPT_COMP}")
   compile_schema_for_test(tests/bigfoot_ref_test.fbs "${FLATC_OPT_COMP}")
+  compile_schema_for_test(tests/required_table_default_test.fbs "${FLATC_OPT_COMP}")
   compile_schema_for_test(tests/key_field/key_field_sample.fbs "${FLATC_OPT_COMP}")
   compile_schema_for_test(tests/64bit/test_64bit.fbs "${FLATC_OPT_COMP};--bfbs-gen-embed")
   compile_schema_for_test(tests/64bit/evolution/v1.fbs "${FLATC_OPT_COMP}")

src/idl_gen_cpp.cpp

@@ -2049,8 +2049,19 @@ class CppGenerator : public BaseGenerator {
               " = " + (native_default ? std::string(native_default->constant)
                                       : GetDefaultScalarValue(field, true));
         } else if (field.value.type.base_type == BASE_TYPE_STRUCT) {
-          if (IsStruct(field.value.type) && native_default) {
+          if (IsStruct(field.value.type)) {
-            field_di = " = " + native_default->constant;
+            if (native_default) {
+              field_di = " = " + native_default->constant;
+            }
+          } else if (!cpp_type && !field.native_inline && field.IsRequired() &&
+                     PtrType(&field) != "naked") {
+            // A required table field can never legally be absent once
+            // Verify()/Pack() succeed, so default-construct it here instead
+            // of leaving every hand-written native constructor to remember
+            // to allocate it before the first Pack().
+            const auto nn = WrapNativeNameInNameSpace(
+                *field.value.type.struct_def, opts_);
+            field_di = "{new " + nn + "()}";
           }
         }
       }

tests/required_table_default_test.cpp

@@ -0,0 +1,39 @@
+#include "required_table_default_test.h"
+
+#include "required_table_default_test_generated.h"
+#include "test_assert.h"
+
+namespace flatbuffers {
+namespace tests {
+
+// Exercises the (required) table-field default member initializer added to
+// GenMember() in idl_gen_cpp.cpp: a `(required)` table field can never
+// legally be missing once Pack()/Verify() succeed, so a plain native default
+// constructor must not leave it null - callers should never have to
+// remember to allocate it themselves before the first Pack(). The sibling
+// `optional_inner` field (same type, no `required`) is checked alongside it
+// to confirm the change is scoped to required fields only.
+void RequiredTableDefaultTest() {
+  using RequiredTableDefaultTestNS::Holder;
+  using RequiredTableDefaultTestNS::HolderT;
+
+  HolderT src;
+  TEST_NOTNULL(src.required_inner.get());
+  TEST_NULL(src.optional_inner.get());
+
+  // Never manually allocated required_inner - only relying on the default
+  // constructor above - yet Pack() must still succeed and round-trip it.
+  src.required_inner->value = 42;
+
+  flatbuffers::FlatBufferBuilder fbb;
+  fbb.Finish(Holder::Pack(fbb, &src));
+
+  auto dst = RequiredTableDefaultTestNS::UnPackHolder(fbb.GetBufferPointer());
+
+  TEST_NOTNULL(dst->required_inner.get());
+  TEST_EQ(dst->required_inner->value, 42u);
+  TEST_NULL(dst->optional_inner.get());
+}
+
+}  // namespace tests
+}  // namespace flatbuffers

tests/required_table_default_test.fbs

@@ -0,0 +1,17 @@
+namespace RequiredTableDefaultTestNS;
+
+table Inner {
+  value: uint32;
+}
+
+// `required_inner` has no default in the wire format (tables can't have one),
+// but Pack()/Verify() both reject a buffer where it is missing - so the
+// native default constructor must never leave it null. `optional_inner` is
+// the same field type without `required`, kept alongside it to confirm the
+// fix only changes required table fields, not every table pointer field.
+table Holder {
+  required_inner: Inner (required);
+  optional_inner: Inner;
+}
+
+root_type Holder;

tests/required_table_default_test.h

@@ -0,0 +1,12 @@
+#ifndef REQUIRED_TABLE_DEFAULT_TEST_H
+#define REQUIRED_TABLE_DEFAULT_TEST_H
+
+namespace flatbuffers {
+namespace tests {
+
+void RequiredTableDefaultTest();
+
+}  // namespace tests
+}  // namespace flatbuffers
+
+#endif  // REQUIRED_TABLE_DEFAULT_TEST_H

tests/test.cpp

@@ -68,6 +68,7 @@
 #include "util_test.h"
 #include "cpp_vector_type_test.h"
 #include "bigfoot_ref_test_impl.h"
+#include "required_table_default_test.h"
 #include "vector_table_naked_ptr_test.h"
 
 void FlatBufferBuilderTest();
@@ -1817,6 +1818,7 @@ int FlatBufferTests(const std::string& tests_data_path) {
   FixedLengthArrayTest();
   NativeTypeTest();
   flatbuffers::tests::BigfootRefTest();
+  flatbuffers::tests::RequiredTableDefaultTest();
   OptionalScalarsTest();
   ParseFlexbuffersFromJsonWithNullTest();
   FlatbuffersSpanTest();