[C++] Fix for printing of enum in case output_enum_identifiers=1. (#5706)

* Add test-case for testing of the future Color in json (output_enum_identifiers = true)

* Refactoring of idl_gen_text.cpp. Fix for printing of bit-enum with active output_enum_identifiers=1.

* Move  GenerateText implementation into class

* Remove unnecessary code from flatbuffers.h

include/flatbuffers/flatbuffers.h

@@ -443,6 +443,13 @@ template<typename T, uint16_t length> class Array {
 
   return_type operator[](uoffset_t i) const { return Get(i); }
 
+  // If this is a Vector of enums, T will be its storage type, not the enum
+  // type. This function makes it convenient to retrieve value with enum
+  // type E.
+  template<typename E> E GetEnum(uoffset_t i) const {
+    return static_cast<E>(Get(i));
+  }
+
   const_iterator begin() const { return const_iterator(Data(), 0); }
   const_iterator end() const { return const_iterator(Data(), size()); }
 
@@ -518,10 +525,12 @@ template<typename T, uint16_t length> class Array<Offset<T>, length> {
   static_assert(flatbuffers::is_same<T, void>::value, "unexpected type T");
 
  public:
+  typedef const void* return_type;
+
   const uint8_t *Data() const { return data_; }
 
   // Make idl_gen_text.cpp::PrintContainer happy.
-  const void *operator[](uoffset_t) const {
+  return_type operator[](uoffset_t) const {
     FLATBUFFERS_ASSERT(false);
     return nullptr;
   }

src/idl_gen_text.cpp

@@ -23,118 +23,150 @@
 
 namespace flatbuffers {
 
-static bool GenStruct(const StructDef &struct_def, const Table *table,
-                      int indent, const IDLOptions &opts, std::string *_text);
+struct PrintScalarTag {};
+struct PrintPointerTag {};
+template<typename T> struct PrintTag { typedef PrintScalarTag type; };
+template<> struct PrintTag<const void *> { typedef PrintPointerTag type; };
 
-// If indentation is less than 0, that indicates we don't want any newlines
-// either.
-const char *NewLine(const IDLOptions &opts) {
-  return opts.indent_step >= 0 ? "\n" : "";
-}
+struct JsonPrinter {
+  // If indentation is less than 0, that indicates we don't want any newlines
+  // either.
+  void AddNewLine() {
+    if (opts.indent_step >= 0) text += '\n';
+  }
 
-int Indent(const IDLOptions &opts) { return std::max(opts.indent_step, 0); }
+  void AddIndent(int ident) { text.append(ident, ' '); }
 
-// Output an identifier with or without quotes depending on strictness.
-void OutputIdentifier(const std::string &name, const IDLOptions &opts,
-                      std::string *_text) {
-  std::string &text = *_text;
-  if (opts.strict_json) text += "\"";
+  int Indent() const { return std::max(opts.indent_step, 0); }
+
+  // Output an identifier with or without quotes depending on strictness.
+  void OutputIdentifier(const std::string &name) {
+    if (opts.strict_json) text += '\"';
     text += name;
-  if (opts.strict_json) text += "\"";
-}
+    if (opts.strict_json) text += '\"';
+  }
 
-// Print (and its template specialization below for pointers) generate text
-// for a single FlatBuffer value into JSON format.
-// The general case for scalars:
-template<typename T>
-bool Print(T val, Type type, int /*indent*/, const uint8_t * /*prev_val*/,
-           soffset_t /*vector_index*/, const IDLOptions &opts,
-           std::string *_text) {
-  std::string &text = *_text;
-  if (type.enum_def && opts.output_enum_identifiers) {
-    std::vector<EnumVal const *> enum_values;
-    if (auto ev = type.enum_def->ReverseLookup(static_cast<int64_t>(val))) {
-      enum_values.push_back(ev);
-    } else if (val && type.enum_def->attributes.Lookup("bit_flags")) {
-      for (auto it = type.enum_def->Vals().begin(),
-                e = type.enum_def->Vals().end();
+  // Print (and its template specialization below for pointers) generate text
+  // for a single FlatBuffer value into JSON format.
+  // The general case for scalars:
+  template<typename T>
+  bool PrintScalar(T val, const Type &type, int /*indent*/) {
+    if (IsBool(type.base_type)) {
+      text += val != 0 ? "true" : "false";
+      return true;  // done
+    }
+
+    if (opts.output_enum_identifiers && type.enum_def) {
+      const auto &enum_def = *type.enum_def;
+      if (auto ev = enum_def.ReverseLookup(static_cast<int64_t>(val))) {
+        text += '\"';
+        text += ev->name;
+        text += '\"';
+        return true;  // done
+      } else if (val && enum_def.attributes.Lookup("bit_flags")) {
+        const auto entry_len = text.length();
+        const auto u64 = static_cast<uint64_t>(val);
+        uint64_t mask = 0;
+        text += '\"';
+        for (auto it = enum_def.Vals().begin(), e = enum_def.Vals().end();
              it != e; ++it) {
-        if ((*it)->GetAsUInt64() & static_cast<uint64_t>(val))
-          enum_values.push_back(*it);
+          auto f = (*it)->GetAsUInt64();
+          if (f & u64) {
+            mask |= f;
+            text += (*it)->name;
+            text += ' ';
           }
         }
-    if (!enum_values.empty()) {
-      text += '\"';
-      for (auto it = enum_values.begin(), e = enum_values.end(); it != e; ++it)
-        text += (*it)->name + ' ';
+        // Don't slice if (u64 != mask)
+        if (mask && (u64 == mask)) {
           text[text.length() - 1] = '\"';
-      return true;
+          return true;  // done
         }
+        text.resize(entry_len);  // restore
+      }
+      // print as numeric value
     }
 
-  if (type.base_type == BASE_TYPE_BOOL) {
-    text += val != 0 ? "true" : "false";
-  } else {
     text += NumToString(val);
+    return true;
+  }
+
+  void AddComma() {
+    if (!opts.protobuf_ascii_alike) text += ',';
   }
 
+  // Print a vector or an array of JSON values, comma seperated, wrapped in
+  // "[]".
+  template<typename Container>
+  bool PrintContainer(PrintScalarTag, const Container &c, size_t size,
+                      const Type &type, int indent, const uint8_t *) {
+    const auto elem_indent = indent + Indent();
+    text += '[';
+    AddNewLine();
+    for (uoffset_t i = 0; i < size; i++) {
+      if (i) {
+        AddComma();
+        AddNewLine();
+      }
+      AddIndent(elem_indent);
+      if (!PrintScalar(c[i], type, elem_indent)) { return false; }
+    }
+    AddNewLine();
+    AddIndent(indent);
+    text += ']';
     return true;
-}
+  }
 
-// Print a vector or an array of JSON values, comma seperated, wrapped in "[]".
-template<typename T, typename Container>
-bool PrintContainer(const Container &c, size_t size, Type type, int indent,
-                    const uint8_t *prev_val, const IDLOptions &opts,
-                    std::string *_text) {
-  std::string &text = *_text;
-  text += "[";
-  text += NewLine(opts);
+  // Print a vector or an array of JSON values, comma seperated, wrapped in
+  // "[]".
+  template<typename Container>
+  bool PrintContainer(PrintPointerTag, const Container &c, size_t size,
+                      const Type &type, int indent, const uint8_t *prev_val) {
+    const auto is_struct = IsStruct(type);
+    const auto elem_indent = indent + Indent();
+    text += '[';
+    AddNewLine();
     for (uoffset_t i = 0; i < size; i++) {
       if (i) {
-      if (!opts.protobuf_ascii_alike) text += ",";
-      text += NewLine(opts);
-    }
-    text.append(indent + Indent(opts), ' ');
-    if (IsStruct(type)) {
-      if (!Print(reinterpret_cast<const void *>(c.Data() +
-                                                i * type.struct_def->bytesize),
-                 type, indent + Indent(opts), nullptr, -1, opts, _text)) {
-        return false;
+        AddComma();
+        AddNewLine();
       }
-    } else {
-      if (!Print(c[i], type, indent + Indent(opts), prev_val,
-                 static_cast<soffset_t>(i), opts, _text)) {
+      AddIndent(elem_indent);
+      auto ptr = is_struct ? reinterpret_cast<const void *>(
+                                 c.Data() + type.struct_def->bytesize * i)
+                           : c[i];
+      if (!PrintOffset(ptr, type, elem_indent, prev_val,
+                       static_cast<soffset_t>(i))) {
         return false;
       }
     }
-  }
-  text += NewLine(opts);
-  text.append(indent, ' ');
-  text += "]";
+    AddNewLine();
+    AddIndent(indent);
+    text += ']';
     return true;
-}
+  }
 
-template<typename T>
-bool PrintVector(const Vector<T> &v, Type type, int indent,
-                 const uint8_t *prev_val, const IDLOptions &opts,
-                 std::string *_text) {
-  return PrintContainer<T, Vector<T>>(v, v.size(), type, indent, prev_val, opts,
-                                      _text);
-}
+  template<typename T>
+  bool PrintVector(const void *val, const Type &type, int indent,
+                   const uint8_t *prev_val) {
+    typedef Vector<T> Container;
+    typedef typename PrintTag<typename Container::return_type>::type tag;
+    auto &vec = *reinterpret_cast<const Container *>(val);
+    return PrintContainer<Container>(tag(), vec, vec.size(), type, indent,
+                                     prev_val);
+  }
 
-// Print an array a sequence of JSON values, comma separated, wrapped in "[]".
-template<typename T>
-bool PrintArray(const Array<T, 0xFFFF> &a, size_t size, Type type, int indent,
-                const IDLOptions &opts, std::string *_text) {
-  return PrintContainer<T, Array<T, 0xFFFF>>(a, size, type, indent, nullptr,
-                                             opts, _text);
-}
+  // Print an array a sequence of JSON values, comma separated, wrapped in "[]".
+  template<typename T>
+  bool PrintArray(const void *val, size_t size, const Type &type, int indent) {
+    typedef Array<T, 0xFFFF> Container;
+    typedef typename PrintTag<typename Container::return_type>::type tag;
+    auto &arr = *reinterpret_cast<const Container *>(val);
+    return PrintContainer<Container>(tag(), arr, size, type, indent, nullptr);
+  }
 
-// Specialization of Print above for pointer types.
-template<>
-bool Print<const void *>(const void *val, Type type, int indent,
-                         const uint8_t *prev_val, soffset_t vector_index,
-                         const IDLOptions &opts, std::string *_text) {
+  bool PrintOffset(const void *val, const Type &type, int indent,
+                   const uint8_t *prev_val, soffset_t vector_index) {
     switch (type.base_type) {
       case BASE_TYPE_UNION: {
         // If this assert hits, you have an corrupt buffer, a union type field
@@ -148,18 +180,17 @@ bool Print<const void *>(const void *val, Type type, int indent,
         }
         auto enum_val = type.enum_def->ReverseLookup(union_type_byte, true);
         if (enum_val) {
-        return Print<const void *>(val, enum_val->union_type, indent, nullptr,
-                                   -1, opts, _text);
+          return PrintOffset(val, enum_val->union_type, indent, nullptr, -1);
         } else {
           return false;
         }
       }
       case BASE_TYPE_STRUCT:
         return GenStruct(*type.struct_def, reinterpret_cast<const Table *>(val),
-                       indent, opts, _text);
+                         indent);
       case BASE_TYPE_STRING: {
         auto s = reinterpret_cast<const String *>(val);
-      return EscapeString(s->c_str(), s->size(), _text, opts.allow_non_utf8,
+        return EscapeString(s->c_str(), s->size(), &text, opts.allow_non_utf8,
                             opts.natural_utf8);
       }
       case BASE_TYPE_VECTOR: {
@@ -170,8 +201,7 @@ bool Print<const void *>(const void *val, Type type, int indent,
         #define FLATBUFFERS_TD(ENUM, IDLTYPE, CTYPE, ...) \
           case BASE_TYPE_ ## ENUM: \
             if (!PrintVector<CTYPE>( \
-                  *reinterpret_cast<const Vector<CTYPE> *>(val), \
-                  vec_type, indent, prev_val, opts, _text)) { \
+                  val, vec_type, indent, prev_val)) { \
               return false; \
             } \
             break;
@@ -189,9 +219,7 @@ bool Print<const void *>(const void *val, Type type, int indent,
         #define FLATBUFFERS_TD(ENUM, IDLTYPE, CTYPE, ...) \
           case BASE_TYPE_ ## ENUM: \
             if (!PrintArray<CTYPE>( \
-              *reinterpret_cast<const Array<CTYPE, 0xFFFF> *>(val), \
-              type.fixed_length, \
-              vec_type, indent, opts, _text)) { \
+                val, type.fixed_length, vec_type, indent)) { \
             return false; \
             } \
             break;
@@ -206,34 +234,30 @@ bool Print<const void *>(const void *val, Type type, int indent,
       }
       default: FLATBUFFERS_ASSERT(0); return false;
     }
-}
+  }
 
-template<typename T> static T GetFieldDefault(const FieldDef &fd) {
+  template<typename T> static T GetFieldDefault(const FieldDef &fd) {
     T val;
     auto check = StringToNumber(fd.value.constant.c_str(), &val);
     (void)check;
     FLATBUFFERS_ASSERT(check);
     return val;
-}
+  }
 
-// Generate text for a scalar field.
-template<typename T>
-static bool GenField(const FieldDef &fd, const Table *table, bool fixed,
-                     const IDLOptions &opts, int indent, std::string *_text) {
-  return Print(
+  // Generate text for a scalar field.
+  template<typename T>
+  bool GenField(const FieldDef &fd, const Table *table, bool fixed,
+                int indent) {
+    return PrintScalar(
         fixed ? reinterpret_cast<const Struct *>(table)->GetField<T>(
                     fd.value.offset)
               : table->GetField<T>(fd.value.offset, GetFieldDefault<T>(fd)),
-      fd.value.type, indent, nullptr, -1, opts, _text);
-}
-
-static bool GenStruct(const StructDef &struct_def, const Table *table,
-                      int indent, const IDLOptions &opts, std::string *_text);
+        fd.value.type, indent);
+  }
 
-// Generate text for non-scalar field.
-static bool GenFieldOffset(const FieldDef &fd, const Table *table, bool fixed,
-                           int indent, const uint8_t *prev_val,
-                           const IDLOptions &opts, std::string *_text) {
+  // Generate text for non-scalar field.
+  bool GenFieldOffset(const FieldDef &fd, const Table *table, bool fixed,
+                      int indent, const uint8_t *prev_val) {
     const void *val = nullptr;
     if (fixed) {
       // The only non-scalar fields in structs are structs or arrays.
@@ -243,28 +267,27 @@ static bool GenFieldOffset(const FieldDef &fd, const Table *table, bool fixed,
     } else if (fd.flexbuffer) {
       auto vec = table->GetPointer<const Vector<uint8_t> *>(fd.value.offset);
       auto root = flexbuffers::GetRoot(vec->data(), vec->size());
-    root.ToString(true, opts.strict_json, *_text);
+      root.ToString(true, opts.strict_json, text);
       return true;
     } else if (fd.nested_flatbuffer) {
       auto vec = table->GetPointer<const Vector<uint8_t> *>(fd.value.offset);
       auto root = GetRoot<Table>(vec->data());
-    return GenStruct(*fd.nested_flatbuffer, root, indent, opts, _text);
+      return GenStruct(*fd.nested_flatbuffer, root, indent);
     } else {
       val = IsStruct(fd.value.type)
                 ? table->GetStruct<const void *>(fd.value.offset)
                 : table->GetPointer<const void *>(fd.value.offset);
     }
-  return Print(val, fd.value.type, indent, prev_val, -1, opts, _text);
-}
+    return PrintOffset(val, fd.value.type, indent, prev_val, -1);
+  }
 
-// Generate text for a struct or table, values separated by commas, indented,
-// and bracketed by "{}"
-static bool GenStruct(const StructDef &struct_def, const Table *table,
-                      int indent, const IDLOptions &opts, std::string *_text) {
-  std::string &text = *_text;
-  text += "{";
+  // Generate text for a struct or table, values separated by commas, indented,
+  // and bracketed by "{}"
+  bool GenStruct(const StructDef &struct_def, const Table *table, int indent) {
+    text += '{';
     int fieldout = 0;
     const uint8_t *prev_val = nullptr;
+    const auto elem_indent = indent + Indent();
     for (auto it = struct_def.fields.vec.begin();
          it != struct_def.fields.vec.end(); ++it) {
       FieldDef &fd = **it;
@@ -272,23 +295,20 @@ static bool GenStruct(const StructDef &struct_def, const Table *table,
       auto output_anyway = opts.output_default_scalars_in_json &&
                            IsScalar(fd.value.type.base_type) && !fd.deprecated;
       if (is_present || output_anyway) {
-      if (fieldout++) {
-        if (!opts.protobuf_ascii_alike) text += ",";
-      }
-      text += NewLine(opts);
-      text.append(indent + Indent(opts), ' ');
-      OutputIdentifier(fd.name, opts, _text);
+        if (fieldout++) { AddComma(); }
+        AddNewLine();
+        AddIndent(elem_indent);
+        OutputIdentifier(fd.name);
         if (!opts.protobuf_ascii_alike ||
             (fd.value.type.base_type != BASE_TYPE_STRUCT &&
              fd.value.type.base_type != BASE_TYPE_VECTOR))
-        text += ":";
-      text += " ";
-      switch (fd.value.type.base_type) {
+          text += ':';
+        text += ' ';
         // clang-format off
+        switch (fd.value.type.base_type) {
         #define FLATBUFFERS_TD(ENUM, IDLTYPE, CTYPE, ...) \
           case BASE_TYPE_ ## ENUM: \
-            if (!GenField<CTYPE>(fd, table, struct_def.fixed, \
-                                  opts, indent + Indent(opts), _text)) { \
+            if (!GenField<CTYPE>(fd, table, struct_def.fixed, elem_indent)) { \
               return false; \
             } \
             break;
@@ -300,13 +320,12 @@ static bool GenStruct(const StructDef &struct_def, const Table *table,
               FLATBUFFERS_GEN_TYPES_POINTER(FLATBUFFERS_TD)
               FLATBUFFERS_GEN_TYPE_ARRAY(FLATBUFFERS_TD)
         #undef FLATBUFFERS_TD
-            if (!GenFieldOffset(fd, table, struct_def.fixed, indent + Indent(opts),
-                                prev_val, opts, _text)) {
+              if (!GenFieldOffset(fd, table, struct_def.fixed, elem_indent, prev_val)) {
                 return false;
               }
             break;
-        // clang-format on
         }
+        // clang-format on
         // Track prev val for use with union types.
         if (struct_def.fixed) {
           prev_val = reinterpret_cast<const uint8_t *>(table) + fd.value.offset;
@@ -315,9 +334,26 @@ static bool GenStruct(const StructDef &struct_def, const Table *table,
         }
       }
     }
-  text += NewLine(opts);
-  text.append(indent, ' ');
-  text += "}";
+    AddNewLine();
+    AddIndent(indent);
+    text += '}';
+    return true;
+  }
+
+  JsonPrinter(const Parser &parser, std::string &dest)
+      : opts(parser.opts), text(dest) {
+    text.reserve(1024);  // Reduce amount of inevitable reallocs.
+  }
+
+  const IDLOptions &opts;
+  std::string &text;
+};
+
+static bool GenerateTextImpl(const Parser &parser, const Table *table,
+                             const StructDef &struct_def, std::string *_text) {
+  JsonPrinter printer(parser, *_text);
+  if (!printer.GenStruct(struct_def, table, 0)) { return false; }
+  printer.AddNewLine();
   return true;
 }
 
@@ -326,30 +362,20 @@ bool GenerateTextFromTable(const Parser &parser, const void *table,
                            const std::string &table_name, std::string *_text) {
   auto struct_def = parser.LookupStruct(table_name);
   if (struct_def == nullptr) { return false; }
-  auto &text = *_text;
-  text.reserve(1024);  // Reduce amount of inevitable reallocs.
   auto root = static_cast<const Table *>(table);
-  if (!GenStruct(*struct_def, root, 0, parser.opts, &text)) { return false; }
-  text += NewLine(parser.opts);
-  return true;
+  return GenerateTextImpl(parser, root, *struct_def, _text);
 }
 
 // Generate a text representation of a flatbuffer in JSON format.
 bool GenerateText(const Parser &parser, const void *flatbuffer,
                   std::string *_text) {
-  std::string &text = *_text;
   FLATBUFFERS_ASSERT(parser.root_struct_def_);  // call SetRootType()
-  text.reserve(1024);  // Reduce amount of inevitable reallocs.
   auto root = parser.opts.size_prefixed ? GetSizePrefixedRoot<Table>(flatbuffer)
                                         : GetRoot<Table>(flatbuffer);
-  if (!GenStruct(*parser.root_struct_def_, root, 0, parser.opts, _text)) {
-    return false;
-  }
-  text += NewLine(parser.opts);
-  return true;
+  return GenerateTextImpl(parser, root, *parser.root_struct_def_, _text);
 }
 
-std::string TextFileName(const std::string &path,
+static std::string TextFileName(const std::string &path,
                                 const std::string &file_name) {
   return path + file_name + ".json";
 }

tests/test.cpp

@@ -650,6 +650,19 @@ void JsonEnumsTest() {
   auto result = GenerateText(parser, builder.GetBufferPointer(), &jsongen);
   TEST_EQ(result, true);
   TEST_EQ(std::string::npos != jsongen.find("color: \"Red Blue\""), true);
+  // Test forward compatibility with 'output_enum_identifiers = true'.
+  // Current Color doesn't have '(1u << 2)' field, let's add it.
+  builder.Clear();
+  std::string future_json;
+  auto future_name = builder.CreateString("future bitflag_enum");
+  MonsterBuilder future_color(builder);
+  future_color.add_name(future_name);
+  future_color.add_color(
+      static_cast<Color>((1u << 2) | Color_Blue | Color_Red));
+  FinishMonsterBuffer(builder, future_color.Finish());
+  result = GenerateText(parser, builder.GetBufferPointer(), &future_json);
+  TEST_EQ(result, true);
+  TEST_EQ(std::string::npos != future_json.find("color: 13"), true);
 }
 
 #if defined(FLATBUFFERS_HAS_NEW_STRTOD) && (FLATBUFFERS_HAS_NEW_STRTOD > 0)