ProtoZero plugin implementation.

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 28fc99771588f5ff1b8ef90af9da552db8f86662..2d57ce02b6cb83396df7ec1bcd7455e20834cbab 100644
--- a/components/tracing/tools/proto_zero_plugin/proto_zero_generator.cc
+++ b/components/tracing/tools/proto_zero_plugin/proto_zero_generator.cc
@@ -4,6 +4,7 @@
 
 #include "proto_zero_generator.h"
 
+#include <map>
 #include <memory>
 #include <string>
 
@@ -15,12 +16,24 @@
 namespace tracing {
 namespace proto {
 
+using google::protobuf::Descriptor;  // Message descriptor.
+using google::protobuf::EnumDescriptor;
+using google::protobuf::EnumValueDescriptor;
+using google::protobuf::FieldDescriptor;
 using google::protobuf::FileDescriptor;
-using google::protobuf::StripSuffixString;
 using google::protobuf::compiler::GeneratorContext;
 using google::protobuf::io::Printer;
 using google::protobuf::io::ZeroCopyOutputStream;
 
+// These protobuf-specific string utils are used since not all required ones
+// are presented in //base and it's better than reinventing a wheel or mixing up

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

NO need for this comment. In general it would be ideal to use base, as that
reduces the dependency on protobuf API surface, which might change over time and
make it hard to roll new versions of protobuf

[kraynov, 2016/07/15 11:01:52]

Discussed offline. Not going to depend on //base at all.

+// different string util libraries.
+using google::protobuf::Split;

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

don't you have this in base?

+using google::protobuf::StripPrefixString;
+using google::protobuf::StripString;
+using google::protobuf::StripSuffixString;
+using google::protobuf::UpperString;

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

You have base::ToUpperASCII

[kraynov, 2016/07/15 11:01:52]

Discussed offline. Not going to change.

+
 namespace {
 
 class GeneratorJob {
@@ -28,22 +41,21 @@ class GeneratorJob {
   GeneratorJob(const FileDescriptor *file,
                Printer* stub_h_printer,
                Printer* stub_cc_printer)
-    : file_(file),
+    : source_(file),
       stub_h_(stub_h_printer),
       stub_cc_(stub_cc_printer) {}
 
   bool GenerateStubs() {
-    stub_h_->Print(
-        "// Autogenerated. DO NOT EDIT.\n"
-        "// Generated by: //components/tracing/proto_zero_plugin.\n\n"
-        "// Package: $package$\n",
-        "package", file_->package());
-    stub_cc_->Print(
-        "// Autogenerated. DO NOT EDIT.\n"
-        "// Generated by: //components/tracing/proto_zero_plugin.\n\n"
-        "// This file intentionally left blank.\n");
-    // TODO(kraynov) Implement in the next CL (crbug.com/608721).
-    return true;
+    Preprocess();
+    PrintPrologue();

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

S/Print/Generate/ here and below

[kraynov, 2016/07/15 11:01:52]

Acknowledged.

+    for (const EnumDescriptor* enumeration: enums_) {
+      PrintEnumDescriptor(enumeration);
+    }
+    for (const Descriptor* message : messages_) {
+      PrintMessageDescriptor(message);
+    }
+    PrintEpilogue();
+    return error_.empty();
   }
 
   // If generator fails to produce stubs for a particular proto definitions
@@ -60,10 +72,312 @@ class GeneratorJob {
     }
   }
 
-  const FileDescriptor* const file_;
+  // Get full name (including outer descriptors) of proto descriptor.
+  template <class T>
+  inline std::string GetDescriptorName(const T* descriptor) {
+    if (!package_.empty()) {
+      return StripPrefixString(descriptor->full_name(), package_ + ".");
+    } else {
+      return descriptor->full_name();
+    }
+  }
+
+  // Get C++ class name corresponding to proto descriptor.
+  // Nested names are splitted by underscores. Underscores in type names aren't
+  // prohibited but not recommended in order to avoid name collisions.
+  template <class T>
+  inline std::string GetCppClassName(const T* descriptor, bool full = false) {
+    std::string name = GetDescriptorName(descriptor);
+    StripString(&name, ".", '_');

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

use base

[kraynov, 2016/07/15 11:01:52]

Discussed offline. Not going to change.

+    if (full)
+      name = full_namespace_prefix_ + name;
+    return name;
+  }
+
+  // Small enums can be written faster without involving VarInt encoder.
+  inline bool IsEnumOptimal(const EnumDescriptor* enumeration) {

[Primiano Tucci (use gerrit), 2016/07/15 10:33:55]

s/IsEnumOptimal/IsTinyEnum/

[kraynov, 2016/07/15 11:01:52]

Acknowledged.

+    for (int i = 0; i < enumeration->value_count(); ++i) {
+      int32_t value = enumeration->value(i)->number();
+      if (value < 0 || value > 0x7F)
+        return false;
+    }
+    return true;
+  }
+
+  void Preprocess() {
+    // Package name maps to a series of namespaces.
+    package_ = source_->package();
+    namespaces_ = Split(package_, ".");
+    full_namespace_prefix_ = "::";
+    for (const std::string& ns : namespaces_) {

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

don't need the braces for single line loops

[kraynov, 2016/07/15 11:01:52]

Acknowledged.

+      full_namespace_prefix_ += ns + "::";
+    }
+    // Collect message descriptors in DFS order.
+    std::vector<const Descriptor*> stack;
+    for (int i = 0; i < source_->message_type_count(); ++i) {
+      stack.push_back(source_->message_type(i));
+    }
+    while (!stack.empty()) {
+      const Descriptor* message = stack.back();
+      stack.pop_back();
+      messages_.push_back(message);
+      for (int i = 0; i < message->nested_type_count(); ++i) {
+        stack.push_back(message->nested_type(i));
+      }
+    }
+    // Collect enums.
+    for (int i = 0; i < source_->enum_type_count(); ++i) {
+      enums_.push_back(source_->enum_type(i));
+    }
+    for (const Descriptor* message : messages_) {

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

are youy sure this loop should not go out? Feels like you are re-iterating over
the same messages over and over, while you populate the messages_ at the same
time.

[kraynov, 2016/07/15 11:01:53]

I iterate over messages_ and populate enums_.

[Primiano Tucci (use gerrit), 2016/07/15 11:53:12]

Oh ignore my comment, I didn't realize that the while(!stack.empty()) brace was
closed above.
I'add some blank lines between the loops to make the code a bit more redable.

+      for (int i = 0; i < message->enum_type_count(); ++i) {
+        enums_.push_back(message->enum_type(i));
+      }
+    }
+  }
+
+  // Print top header, namespaces and forward declarations.
+  void PrintPrologue() {
+    std::string greeting =
+        "// Autogenerated. DO NOT EDIT.\n"
+        "// Protobuf compiler (protoc) has generated these stubs with\n"
+        "// //components/tracing/tools/proto_zero_plugin.\n";
+    std::string guard = package_ + "_" + source_->name() + "_H_";
+    UpperString(&guard);

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

use base for these

[kraynov, 2016/07/15 11:01:52]

Discussed offline. Not going to change.

+    StripString(&guard, ".-", '_');
+
+    stub_h_->Print(
+        "$greeting$\n"
+        "#ifndef $guard$\n"
+        "#define $guard$\n\n"
+        "#include <stddef.h>\n"
+        "#include <stdint.h>\n\n"
+        "// Runtime is optional for testing purposes.\n"
+        "#ifndef PROTO_ZERO_NO_RUNTIME\n"

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

what is this PROTO_ZERO_NO_RUNTIME? 
I think you can just uncomment the include below  now

[kraynov, 2016/07/15 11:01:52]

#ifndef here. PROTO_ZERO_NO_RUNTIME is going to be defined in the unittest in
order to substitute with a fake one. See:
https://codereview.chromium.org/2083373002/patch/60001/70012 fake_runtime.h here
defines PROTO_ZERO_NO_RUNTIME.

[Primiano Tucci (use gerrit), 2016/07/15 11:53:12]

Hmm feels very weird that all emitted messages have this ifdef just because in
one case you want to mock it.
ALso this means that if I want to make a change to the ProtoZeroMessage, now I
have to match the API change in the fake runtime.

I wonder if the best testing strategy here is just using the official runtime.
What's the benefit of the fake one?

[kraynov, 2016/07/15 13:06:58]

Ok, will remove for this time.

+        "// #include \"components/tracing/core/proto_zero_message.h\"\n"
+        "#endif\n\n",
+        "greeting", greeting,
+        "guard", guard);
+    stub_cc_->Print(
+        "$greeting$\n"
+        "// This file intentionally left blank.\n",
+        "greeting", greeting);
+
+    // Print namespaces.
+    for (const std::string& ns : namespaces_)
+      stub_h_->Print("namespace $ns$ {\n", "ns", ns);
+    stub_h_->Print("\n");
+    // Print forward declarations.
+    for (const Descriptor* message : messages_) {
+      stub_h_->Print(
+          "class $class$;\n",
+          "class", GetCppClassName(message));
+    }
+    stub_h_->Print("\n");
+  }
+
+  void PrintEnumDescriptor(const EnumDescriptor* enumeration) {
+    stub_h_->Print(
+        "enum $class$ : int32_t {\n",

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

It seems that the standard proto just generates an plain enum, not enum class.
Any reason why we are diverging here?

+        "class", GetCppClassName(enumeration));
+    stub_h_->Indent();
+    for (int i = 0; i < enumeration->value_count(); ++i) {
+      const EnumValueDescriptor* value = enumeration->value(i);
+      stub_h_->Print(
+          "$name$ = $number$,\n",
+          "name", value->name(),
+          "number", std::to_string(value->number()));

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

is this going to generate enums according to the same pattern of protoc?
E.g.,

enum OmniboxEventProto_Suggestion_ResultType {
  OmniboxEventProto_Suggestion_ResultType_UNKNOWN_RESULT_TYPE = 0,
  OmniboxEventProto_Suggestion_ResultType_URL_WHAT_YOU_TYPED = 1,
  OmniboxEventProto_Suggestion_ResultType_HISTORY_URL = 2,
  OmniboxEventProto_Suggestion_ResultType_HISTORY_TITLE = 3,
  OmniboxEventProto_Suggestion_ResultType_HISTORY_BODY = 4,
...
}

doesn't seem so at first glance

[kraynov, 2016/07/15 11:01:53]

Ok, thanks for pointing on that!

+    }
+    stub_h_->Outdent();
+    stub_h_->Print("};\n\n");
+  }
+
+  void PrintSimpleFieldDescriptor(const FieldDescriptor* field) {
+    std::map<std::string, std::string> setter;
+    setter["id"] = std::to_string(field->number());
+    setter["name"] = field->name();
+    setter["action"] = field->is_repeated() ? "add" : "set";
+
+    std::string appender;
+    std::string cpp_type;
+
+    switch (field->type()) {
+      case FieldDescriptor::TYPE_BOOL: {
+        appender = "AppendBool";
+        cpp_type = "bool";
+        break;
+      }
+      case FieldDescriptor::TYPE_INT32: {
+        appender = "AppendInt32";
+        cpp_type = "int32_t";
+        break;
+      }
+      case FieldDescriptor::TYPE_INT64: {
+        appender = "AppendInt64";
+        cpp_type = "int64_t";
+        break;
+      }
+      case FieldDescriptor::TYPE_UINT32: {
+        appender = "AppendUint32";
+        cpp_type = "uint32_t";
+        break;
+      }
+      case FieldDescriptor::TYPE_UINT64: {
+        appender = "AppendUint64";
+        cpp_type = "uint64_t";
+        break;
+      }
+      case FieldDescriptor::TYPE_SINT32: {
+        appender = "AppendSint32";
+        cpp_type = "int32_t";
+        break;
+      }
+      case FieldDescriptor::TYPE_SINT64: {
+        appender = "AppendSint64";
+        cpp_type = "int64_t";
+        break;
+      }
+      case FieldDescriptor::TYPE_FIXED32: {
+        appender = "AppendFixed32";
+        cpp_type = "uint32_t";
+        break;
+      }
+      case FieldDescriptor::TYPE_FIXED64: {
+        appender = "AppendFixed64";
+        cpp_type = "uint64_t";
+        break;
+      }
+      case FieldDescriptor::TYPE_SFIXED32: {
+        appender = "AppendSfixed32";
+        cpp_type = "int32_t";
+        break;
+      }
+      case FieldDescriptor::TYPE_SFIXED64: {
+        appender = "AppendSfixed64";
+        cpp_type = "int64_t";
+        break;
+      }
+      case FieldDescriptor::TYPE_FLOAT: {
+        appender = "AppendFloat";
+        cpp_type = "float";
+        break;
+      }
+      case FieldDescriptor::TYPE_DOUBLE: {
+        appender = "AppendDouble";
+        cpp_type = "double";
+        break;
+      }
+      case FieldDescriptor::TYPE_ENUM: {
+        if (IsEnumOptimal(field->enum_type())) {

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

appender = IsTinyEnum(field.enum_type()) ? "AppendTinyInt" : "AppendInt32"

[kraynov, 2016/07/15 11:01:53]

Acknowledged.

+          appender = "AppendTinyNumber";
+        } else {
+          appender = "AppendInt32";
+        }
+        cpp_type = GetCppClassName(field->enum_type(), true);
+        break;
+      }
+      case FieldDescriptor::TYPE_STRING: {
+        appender = "AppendString";
+        cpp_type = "const char*";
+        break;
+      }
+      case FieldDescriptor::TYPE_BYTES: {
+        stub_h_->Print(
+            setter,
+            "void $action$_$name$(const uint8_t* data, size_t size) {\n"
+            "  // AppendBytes($id$, data, size);\n"

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

Why AppendBytes is commented out?

[kraynov, 2016/07/15 11:01:53]

It's to be uncommentend in the next CL when API will be finally negotiated.

+            "}\n");
+        return;
+      }
+      default: {
+        Abort("Unsupported field type.");
+        return;
+      }
+    }
+    setter["appender"] = appender;
+    setter["cpp_type"] = cpp_type;
+    stub_h_->Print(
+        setter,
+        "void $action$_$name$($cpp_type$ value) {\n"
+        "  // $appender$($id$, value);\n"

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

why the appender is commented?

[kraynov, 2016/07/15 11:01:53]

It's to be uncommentend in the next CL when API will be finally negotiated.

+        "}\n");
+  }
+
+  void PrintNestedMessageFieldDescriptor(const FieldDescriptor* field) {
+    stub_h_->Print(
+        "$class$* $action$_$name$() {\n"
+        "  // $class$* nested;\n"
+        "  // OpenNestedMessage($id$, &nested);\n"

[Primiano Tucci (use gerrit), 2016/07/15 11:53:12]

This is BeginNestedMessage. See
https://cs.chromium.org/chromium/src/components/tracing/core/proto_zero_messa...

+        "  return nullptr;\n"
+        "}\n",
+        "id", std::to_string(field->number()),
+        "name", field->name(),
+        "action", field->is_repeated() ? "add" : "set",
+        "class", GetCppClassName(field->message_type()));
+  }
+
+  void PrintMessageDescriptor(const Descriptor* message) {
+    stub_h_->Print(
+        "class $name$ /* : public ::tracing::proto::ProtoZeroMessage */ {\n"

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

why this is commented?

+        " public:\n",
+        "name", GetCppClassName(message));
+    stub_h_->Indent();
+
+    // Using statements for nested messages.
+    for (int i = 0; i < message->nested_type_count(); ++i) {
+      const Descriptor* nested_message = message->nested_type(i);
+      stub_h_->Print(
+          "using $local_name$ = $global_name$;\n",
+          "local_name", nested_message->name(),
+          "global_name", GetCppClassName(nested_message, true));
+    }
+    // Using statements for nested enums.
+    for (int i = 0; i < message->enum_type_count(); ++i) {
+      const EnumDescriptor* nested_enum = message->enum_type(i);
+      stub_h_->Print(
+          "using $local_name$ = $global_name$;\n",
+          "local_name", nested_enum->name(),
+          "global_name", GetCppClassName(nested_enum, true));
+    }
+    // Fields descriptors.
+    for (int i = 0; i < message->field_count(); ++i) {
+      const FieldDescriptor* field = message->field(i);
+      if (field->is_required()) {
+        Abort("Required fields are not supported.");

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

why this?

[kraynov, 2016/07/15 11:01:53]

required considered harmful and we aren't going to enforce this constraint on
generation stage. Hence parser could get invalid binary.

[Primiano Tucci (use gerrit), 2016/07/15 11:53:12]

I know but doesn't seem an argument for not emitting code for that. I'd postpone
this decision to codereviews of the actual protos.
Isn't going to make a huge difference here.

[kraynov, 2016/07/15 13:06:58]

Acknowledged.

+        return;
+      }
+      if (field->is_packed()) {
+        Abort("Packed repeated fields are not supported.");
+        return;
+      }
+      if (field->type() != FieldDescriptor::TYPE_MESSAGE) {

[Primiano Tucci (use gerrit), 2016/07/15 10:33:56]

Is the case of nested messages not handled yet?

[kraynov, 2016/07/15 11:01:52]

It's handled in PrintNestedMessageFieldDescriptor.

[Primiano Tucci (use gerrit), 2016/07/15 11:53:12]

Acknowledged.

+        PrintSimpleFieldDescriptor(field);
+      } else {
+        PrintNestedMessageFieldDescriptor(field);
+      }
+    }
+
+    stub_h_->Outdent();
+    stub_h_->Print("};\n\n");
+  }
+
+  void PrintEpilogue() {
+    for (unsigned i = 0; i < namespaces_.size(); ++i) {
+      stub_h_->Print("} // Namespace.\n");
+    }
+    stub_h_->Print("#endif  // Include guard.\n");
+  }
+
+  const FileDescriptor* const source_;
   Printer* const stub_h_;
   Printer* const stub_cc_;
   std::string error_;
+
+  std::string package_;
+  std::vector<std::string> namespaces_;
+  std::string full_namespace_prefix_;
+  std::vector<const Descriptor*> messages_;
+  std::vector<const EnumDescriptor*> enums_;
 };
 
 }  // namespace