AVRO-3992 [C++] Fix compiler warnings in code generated by schema wit…

…h empty record (#2927) * [C++] Fix compiler warnings in code generated by schema with empty record * [C++] Generate union names for record array-unions Added to make writing a test for empty unions easier. * [C++] Fix validatingEncoder for records
apache · Jun 25, 2024 · 4958755 · 4958755
1 parent 6863074
commit 4958755
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 5 deletions.
diff --git a/lang/c++/CMakeLists.txt b/lang/c++/CMakeLists.txt
@@ -171,6 +171,7 @@ gen (tweet testgen3)
 gen (union_array_union uau)
 gen (union_map_union umu)
 gen (union_conflict uc)
+gen (union_empty_record uer)
 gen (recursive rec)
 gen (reuse ru)
 gen (circulardep cd)
@@ -213,7 +214,8 @@ add_dependencies (AvrogencppTests bigrecord_hh bigrecord_r_hh bigrecord2_hh
     tweet_hh
     union_array_union_hh union_map_union_hh union_conflict_hh
     recursive_hh reuse_hh circulardep_hh tree1_hh tree2_hh crossref_hh
-    primitivetypes_hh empty_record_hh cpp_reserved_words_union_typedef_hh)
+    primitivetypes_hh empty_record_hh cpp_reserved_words_union_typedef_hh
+    union_empty_record_hh)
 
 include (InstallRequiredSystemLibraries)
 

diff --git a/lang/c++/impl/avrogencpp.cc b/lang/c++/impl/avrogencpp.cc
@@ -247,6 +247,10 @@ string CodeGen::generateRecordType(const NodePtr &n) {
                     << ' ' << n->nameAt(i) << "_t;\n";
                 types[i] = n->nameAt(i) + "_t";
             }
+            if (n->leafAt(i)->type() == avro::AVRO_ARRAY && n->leafAt(i)->leafAt(0)->type() == avro::AVRO_UNION) {
+                os_ << "    typedef " << types[i] << "::value_type"
+                    << ' ' << n->nameAt(i) << "_item_t;\n";
+            }
         }
     }
     for (size_t i = 0; i < c; ++i) {
@@ -537,8 +541,22 @@ void CodeGen::generateRecordTraits(const NodePtr &n) {
     }
 
     string fn = fullname(decorate(n->name()));
-    os_ << "template<> struct codec_traits<" << fn << "> {\n"
-        << "    static void encode(Encoder& e, const " << fn << "& v) {\n";
+    os_ << "template<> struct codec_traits<" << fn << "> {\n";
+
+    if (c == 0) {
+        os_ << "    static void encode(Encoder&, const " << fn << "&) {}\n";
+        // ResolvingDecoder::fieldOrder mutates the state of the decoder, so if that decoder is
+        // passed in, we need to call the method even though it will return an empty vector.
+        os_ << "    static void decode(Decoder& d, " << fn << "&) {\n";
+        os_ << "        if (avro::ResolvingDecoder *rd = dynamic_cast<avro::ResolvingDecoder *>(&d)) {\n";
+        os_ << "            rd->fieldOrder();\n";
+        os_ << "        }\n";
+        os_ << "    }\n";
+        os_ << "};\n";
+        return;
+    }
+
+    os_ << "    static void encode(Encoder& e, const " << fn << "& v) {\n";
 
     for (size_t i = 0; i < c; ++i) {
         // the nameAt(i) does not take c++ reserved words into account

diff --git a/lang/c++/impl/parsing/ValidatingCodec.cc b/lang/c++/impl/parsing/ValidatingCodec.cc
@@ -502,6 +502,7 @@ void ValidatingEncoder<P>::setItemCount(size_t count) {
 
 template<typename P>
 void ValidatingEncoder<P>::startItem() {
+    parser_.processImplicitActions();
     if (parser_.top() != Symbol::Kind::Repeater) {
         throw Exception("startItem at not an item boundary");
     }

diff --git a/lang/c++/jsonschemas/union_empty_record b/lang/c++/jsonschemas/union_empty_record
@@ -0,0 +1,25 @@
+{
+  "type": "record",
+  "name": "StackCalculator",
+  "fields": [
+    {
+      "name": "stack",
+      "type": {
+        "type": "array",
+        "items": [
+          "int",
+          {
+            "type": "record",
+            "name": "Dup",
+            "fields": []
+          },
+          {
+            "type": "record",
+            "name": "Add",
+            "fields": []
+          }
+        ]
+      }
+    }
+  ]
+}
diff --git a/lang/c++/test/AvrogencppTests.cc b/lang/c++/test/AvrogencppTests.cc
@@ -21,6 +21,7 @@
 #include "bigrecord_r.hh"
 #include "tweet.hh"
 #include "union_array_union.hh"
+#include "union_empty_record.hh"
 #include "union_map_union.hh"
 
 #include <boost/test/included/unit_test.hpp>
@@ -267,13 +268,45 @@ void testEncoding2() {
     check(t2, t1);
 }
 
-boost::unit_test::test_suite *
-init_unit_test_suite(int /*argc*/, char * /*argv*/[]) {
+void testEmptyRecord() {
+    uer::StackCalculator calc;
+    uer::StackCalculator::stack_item_t item;
+    item.set_int(3);
+    calc.stack.push_back(item);
+    item.set_Dup(uer::Dup());
+    calc.stack.push_back(item);
+    item.set_Add(uer::Add());
+    calc.stack.push_back(item);
+
+    ValidSchema s;
+    ifstream ifs("jsonschemas/union_empty_record");
+    compileJsonSchema(ifs, s);
+
+    unique_ptr<OutputStream> os = memoryOutputStream();
+    EncoderPtr e = validatingEncoder(s, binaryEncoder());
+    e->init(*os);
+    avro::encode(*e, calc);
+    e->flush();
+
+    DecoderPtr d = validatingDecoder(s, binaryDecoder());
+    unique_ptr<InputStream> is = memoryInputStream(*os);
+    d->init(*is);
+    uer::StackCalculator calc2;
+    avro::decode(*d, calc2);
+
+    BOOST_CHECK_EQUAL(calc.stack.size(), calc2.stack.size());
+    BOOST_CHECK_EQUAL(calc2.stack[0].idx(), 0);
+    BOOST_CHECK_EQUAL(calc2.stack[1].idx(), 1);
+    BOOST_CHECK_EQUAL(calc2.stack[2].idx(), 2);
+}
+
+boost::unit_test::test_suite *init_unit_test_suite(int /*argc*/, char * /*argv*/[]) {
     auto *ts = BOOST_TEST_SUITE("Code generator tests");
     ts->add(BOOST_TEST_CASE(testEncoding));
     ts->add(BOOST_TEST_CASE(testResolution));
     ts->add(BOOST_TEST_CASE(testEncoding2<uau::r1>));
     ts->add(BOOST_TEST_CASE(testEncoding2<umu::r1>));
     ts->add(BOOST_TEST_CASE(testNamespace));
+    ts->add(BOOST_TEST_CASE(testEmptyRecord));
     return ts;
 }