Tracing V2: Proto fields reflection.

components/tracing/tools/proto_zero_plugin/proto_zero_generator.cc

Index: components/tracing/tools/proto_zero_plugin/proto_zero_generator.cc
diff --git a/components/tracing/tools/proto_zero_plugin/proto_zero_generator.cc b/components/tracing/tools/proto_zero_plugin/proto_zero_generator.cc
index 713d3cca42fea6c28cb102b900dff6842c26bad2..744cc97678f625c17f78e90032c39ffabb13cce1 100644
--- a/components/tracing/tools/proto_zero_plugin/proto_zero_generator.cc
+++ b/components/tracing/tools/proto_zero_plugin/proto_zero_generator.cc
@@ -45,7 +45,8 @@ class GeneratorJob {
                Printer* stub_cc_printer)
     : source_(file),
       stub_h_(stub_h_printer),
-      stub_cc_(stub_cc_printer) {}
+      stub_cc_(stub_cc_printer),
+      generate_reflection_(false) {}
 
   bool GenerateStubs() {
     Preprocess();
@@ -61,6 +62,12 @@ class GeneratorJob {
   void SetOption(const std::string& name, const std::string& value) {
     if (name == "wrapper_namespace") {
       wrapper_namespace_ = value;
+    } else if (name == "reflection") {

[Primiano Tucci (use gerrit), 2016/09/01 16:18:06]

ditto, less code to maintain plz :)

+      if (value == "true" || value == "1") {
+        generate_reflection_ = true;

[picksi1, 2016/09/01 16:38:39]

There is an implication that generate_reflection defaults to false here, which
may change at some future point? Should this if/else (or switch if it changes)
set generate_reflection to false if value == 'false' || value = '1' and move
this Abort into the else branch.

+      } else if (value != "false" && value != "0") {
+        Abort(std::string() + "Invalid 'reflection' value '" + value + "'.");
+      }
     } else {
       Abort(std::string() + "Unknown plugin option '" + name + "'.");
     }
@@ -101,6 +108,23 @@ class GeneratorJob {
     return name;
   }
 
+  inline std::string Capitalize(const std::string& str) {

[picksi1, 2016/09/01 16:38:39]

Could you use Capitalize() in the SetOption function and compare with TRUE and
FALSE, as currently passing 'True' or 'False' (i.e. capitalized) would fail.

+    std::string copy(str);
+    copy.at(0) = toupper(copy.at(0));
+    return copy;
+  }
+
+  inline std::string GetFieldNumberConstant(const FieldDescriptor* field) {
+    std::string name = field->camelcase_name();
+    if (!name.empty()) {
+      name = std::string("k") + Capitalize(name) + "FieldNumber";
+    } else {
+      // Protoc allows fields like 'bool _ = 1'.
+      Abort("Empty field name in camel case notation.");
+    }
+    return name;
+  }
+
   // Small enums can be written faster without involving VarInt encoder.
   inline bool IsTinyEnumField(const FieldDescriptor* field) {
     if (field->type() != FieldDescriptor::TYPE_ENUM)
@@ -263,6 +287,15 @@ class GeneratorJob {
     }
     stub_cc_->Print("\n");
 
+    if (generate_reflection_ && messages_.size() > 0)
+      stub_cc_->Print(
+          "namespace {\n"
+          "  static const ::tracing::v2::proto::ProtoFieldDescriptor "
+          "__invalid_field = {\"\", "

[Primiano Tucci (use gerrit), 2016/09/01 16:18:06]

just calling kInvalidField should be enough and fits our naming conventions.

+          "::tracing::v2::proto::ProtoFieldDescriptor::Type::TYPE_INVALID, "
+          "0, false};\n"
+          "}\n\n");
+
     // Print namespaces.
     for (const std::string& ns : namespaces_) {
       stub_h_->Print("namespace $ns$ {\n", "ns", ns);
@@ -435,6 +468,81 @@ class GeneratorJob {
         "outer_class", outer_class);
   }
 
+  void GenerateReflectionHelpers(const Descriptor* message) {
+    // Field numbers.
+    if (message->field_count() > 0) {

[picksi1, 2016/09/01 16:38:39]

There are a number of places where the code has this if >0 statement. If we own
the Descriptor class (not sure?) should we add a new function called
has_fields() to wrap this repeated functionality?

+      stub_h_->Print("enum : int32_t {\n");
+      stub_h_->Indent();
+
+      for (int i = 0; i < message->field_count(); ++i) {
+        const FieldDescriptor* field = message->field(i);
+        stub_h_->Print(
+            "$name$ = $id$,\n",
+            "name", GetFieldNumberConstant(field),
+            "id", std::to_string(field->number()));
+      }
+      stub_h_->Outdent();
+      stub_h_->Print("};\n");
+    }
+
+    // Fields reflection.
+    stub_h_->Print(
+        "static const ::tracing::v2::proto::ProtoFieldDescriptor* "
+        "GetFieldDescriptor(uint32_t field_id);\n");
+
+    std::string class_name = GetCppClassName(message);
+    if (message->field_count() > 0) {
+      stub_cc_->Print(
+          "static const ::tracing::v2::proto::ProtoFieldDescriptor "
+          "__fields_$class$[] = {\n",

[Primiano Tucci (use gerrit), 2016/09/01 16:18:06]

ditto, just kFields...
as this is a non exported symbol is extremely unlikely to clash, no need to __
magic I think :)

+          "class", class_name);
+      stub_cc_->Indent();
+      for (int i = 0; i < message->field_count(); ++i) {
+        const FieldDescriptor* field = message->field(i);
+        // Caveats:
+        // There is assumption that field type enums (protoc and ProtoZero) have
+        // same numbers for corresponding types. Hence, there is a test.

[Primiano Tucci (use gerrit), 2016/09/01 16:18:06]

s/Hence, there is a test./This is covered by TEST_NAME in 
...blabla_unittest.cc/

+        stub_cc_->Print(
+            "{\"$name$\", static_cast"
+            "<::tracing::v2::proto::ProtoFieldDescriptor::Type>($type$), "
+            "$number$, $is_repeated$},\n",
+            "name", field->name(),
+            "type", std::to_string(static_cast<int>(field->type())),
+            "number", std::to_string(field->number()),
+            "is_repeated", std::to_string(field->is_repeated()));
+      }
+      stub_cc_->Outdent();
+      stub_cc_->Print("};\n\n");
+    }
+
+    stub_cc_->Print(
+        "const ::tracing::v2::proto::ProtoFieldDescriptor* "
+        "$class$::GetFieldDescriptor(uint32_t field_id) {\n",
+        "class", class_name);
+    stub_cc_->Indent();
+    if (message->field_count() > 0) {
+      stub_cc_->Print("switch (field_id) {\n");
+      stub_cc_->Indent();
+      for (int i = 0; i < message->field_count(); ++i) {
+        stub_cc_->Print(
+            "case $field$:\n"
+            "  return &__fields_$class$[$id$];\n",
+            "class", class_name,
+            "field", GetFieldNumberConstant(message->field(i)),
+            "id", std::to_string(i));
+      }
+      stub_cc_->Print(
+          "default:\n"
+          "  return &__invalid_field;\n");
+      stub_cc_->Outdent();
+      stub_cc_->Print("}\n");
+    } else {
+      stub_cc_->Print("return &__invalid_field;\n");
+    }
+    stub_cc_->Outdent();
+    stub_cc_->Print("}\n\n");
+  }
+
   void GenerateMessageDescriptor(const Descriptor* message) {
     stub_h_->Print(
         "class $name$ : public ::tracing::v2::ProtoZeroMessage {\n"
@@ -442,6 +550,9 @@ class GeneratorJob {
         "name", GetCppClassName(message));
     stub_h_->Indent();
 
+    if (generate_reflection_)
+      GenerateReflectionHelpers(message);

[picksi1, 2016/09/01 16:38:39]

Is this function correctly named GenerateReflectionHelpers()? The code reads
slightly oddly as the condition is called generater_reflection, so should this
function just be called GenerateReflection()?

+
     // Using statements for nested messages.
     for (int i = 0; i < message->nested_type_count(); ++i) {
       const Descriptor* nested_message = message->nested_type(i);
@@ -475,30 +586,6 @@ class GeneratorJob {
       }
     }
 
-    // Field numbers.
-    if (message->field_count() > 0) {
-      stub_h_->Print("enum : int32_t {\n");
-      stub_h_->Indent();
-
-      for (int i = 0; i < message->field_count(); ++i) {
-        const FieldDescriptor* field = message->field(i);
-        std::string name = field->camelcase_name();
-        if (!name.empty()) {
-          name.at(0) = toupper(name.at(0));
-        } else {
-          // Protoc allows fields like 'bool _ = 1'.
-          Abort("Empty field name in camel case notation.");
-        }
-
-        stub_h_->Print(
-            "k$name$FieldNumber = $id$,\n",
-            "name", name,
-            "id", std::to_string(field->number()));
-      }
-      stub_h_->Outdent();
-      stub_h_->Print("};\n");
-    }
-
     // Field descriptors.
     for (int i = 0; i < message->field_count(); ++i) {
       const FieldDescriptor* field = message->field(i);
@@ -530,6 +617,7 @@ class GeneratorJob {
   Printer* const stub_cc_;
   std::string error_;
 
+  bool generate_reflection_;
   std::string package_;
   std::string wrapper_namespace_;
   std::vector<std::string> namespaces_;