Merge "Extension overhaul."

9 files changed, 258 insertions, 131 deletions

diff --git a/src/google/protobuf/compiler/javanano/javanano_extension.cc b/src/google/protobuf/compiler/javanano/javanano_extension.cc
index 0bc9c9d..754ed55 100644
--- a/src/google/protobuf/compiler/javanano/javanano_extension.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_extension.cc
@@ -42,28 +42,84 @@ namespace compiler {
 namespace javanano {
 
 using internal::WireFormat;
+using internal::WireFormatLite;
+
+namespace {
+
+const char* GetTypeConstantName(const FieldDescriptor::Type type) {
+  switch (type) {
+    case FieldDescriptor::TYPE_INT32   : return "TYPE_INT32"   ;
+    case FieldDescriptor::TYPE_UINT32  : return "TYPE_UINT32"  ;
+    case FieldDescriptor::TYPE_SINT32  : return "TYPE_SINT32"  ;
+    case FieldDescriptor::TYPE_FIXED32 : return "TYPE_FIXED32" ;
+    case FieldDescriptor::TYPE_SFIXED32: return "TYPE_SFIXED32";
+    case FieldDescriptor::TYPE_INT64   : return "TYPE_INT64"   ;
+    case FieldDescriptor::TYPE_UINT64  : return "TYPE_UINT64"  ;
+    case FieldDescriptor::TYPE_SINT64  : return "TYPE_SINT64"  ;
+    case FieldDescriptor::TYPE_FIXED64 : return "TYPE_FIXED64" ;
+    case FieldDescriptor::TYPE_SFIXED64: return "TYPE_SFIXED64";
+    case FieldDescriptor::TYPE_FLOAT   : return "TYPE_FLOAT"   ;
+    case FieldDescriptor::TYPE_DOUBLE  : return "TYPE_DOUBLE"  ;
+    case FieldDescriptor::TYPE_BOOL    : return "TYPE_BOOL"    ;
+    case FieldDescriptor::TYPE_STRING  : return "TYPE_STRING"  ;
+    case FieldDescriptor::TYPE_BYTES   : return "TYPE_BYTES"   ;
+    case FieldDescriptor::TYPE_ENUM    : return "TYPE_ENUM"    ;
+    case FieldDescriptor::TYPE_GROUP   : return "TYPE_GROUP"   ;
+    case FieldDescriptor::TYPE_MESSAGE : return "TYPE_MESSAGE" ;
+
+    // No default because we want the compiler to complain if any new
+    // types are added.
+  }
+
+  GOOGLE_LOG(FATAL) << "Can't get here.";
+  return NULL;
+}
+
+}  // namespace
 
 void SetVariables(const FieldDescriptor* descriptor, const Params params,
                   map<string, string>* variables) {
-  (*variables)["name"] = 
-    RenameJavaKeywords(UnderscoresToCamelCase(descriptor));
-  (*variables)["number"] = SimpleItoa(descriptor->number());
   (*variables)["extends"] = ClassName(params, descriptor->containing_type());
-
-  string type;
+  (*variables)["name"] = RenameJavaKeywords(UnderscoresToCamelCase(descriptor));
+  bool repeated = descriptor->is_repeated();
+  (*variables)["repeated"] = repeated ? "Repeated" : "";
+  (*variables)["type"] = GetTypeConstantName(descriptor->type());
   JavaType java_type = GetJavaType(descriptor->type());
-  switch (java_type) {
-    case JAVATYPE_ENUM:
-      type = "java.lang.Integer";
-      break;
-    case JAVATYPE_MESSAGE:
-      type = ClassName(params, descriptor->message_type());
-      break;
-    default:
-      type = BoxedPrimitiveTypeName(java_type);
-      break;
+  string tag = SimpleItoa(WireFormat::MakeTag(descriptor));
+  if (java_type == JAVATYPE_MESSAGE) {
+    (*variables)["ext_type"] = "MessageTyped";
+    string message_type = ClassName(params, descriptor->message_type());
+    if (repeated) {
+      message_type += "[]";
+    }
+    (*variables)["class"] = message_type;
+    // For message typed extensions, tags_params contains a single tag
+    // for both singular and repeated cases.
+    (*variables)["tag_params"] = tag;
+  } else {
+    (*variables)["ext_type"] = "PrimitiveTyped";
+    if (!repeated) {
+      (*variables)["class"] = BoxedPrimitiveTypeName(java_type);
+      (*variables)["tag_params"] = tag;
+    } else {
+      (*variables)["class"] = PrimitiveTypeName(java_type) + "[]";
+      if (!descriptor->is_packable()) {
+        // Non-packable: nonPackedTag == tag, packedTag == 0
+        (*variables)["tag_params"] = tag + ", " + tag + ", 0";
+      } else if (descriptor->options().packed()) {
+        // Packable and packed: tag == packedTag
+        string non_packed_tag = SimpleItoa(WireFormatLite::MakeTag(
+            descriptor->number(),
+            WireFormat::WireTypeForFieldType(descriptor->type())));
+        (*variables)["tag_params"] = tag + ", " + non_packed_tag + ", " + tag;
+      } else {
+        // Packable and not packed: tag == nonPackedTag
+        string packed_tag = SimpleItoa(WireFormatLite::MakeTag(
+            descriptor->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED));
+        (*variables)["tag_params"] = tag + ", " + tag + ", " + packed_tag;
+      }
+    }
   }
-  (*variables)["type"] = type;
 }
 
 ExtensionGenerator::
@@ -75,21 +131,16 @@ ExtensionGenerator(const FieldDescriptor* descriptor, const Params& params)
 ExtensionGenerator::~ExtensionGenerator() {}
 
 void ExtensionGenerator::Generate(io::Printer* printer) const {
-  if (descriptor_->is_repeated()) {
-    printer->Print(variables_,
-      "\n"
-      "// extends $extends$\n"
-      "public static final com.google.protobuf.nano.Extension<java.util.List<$type$>> $name$ = \n"
-      "    com.google.protobuf.nano.Extension.createRepeated($number$,\n"
-      "        new com.google.protobuf.nano.Extension.TypeLiteral<java.util.List<$type$>>(){});\n");
-  } else {
-    printer->Print(variables_,
-      "\n"
-      "// extends $extends$\n"
-      "public static final com.google.protobuf.nano.Extension<$type$> $name$ =\n"
-      "    com.google.protobuf.nano.Extension.create($number$,\n"
-      "        new com.google.protobuf.nano.Extension.TypeLiteral<$type$>(){});\n");
-  }
+  printer->Print("\n");
+  PrintFieldComment(printer, descriptor_);
+  printer->Print(variables_,
+    "public static final com.google.protobuf.nano.Extension<\n"
+    "    $extends$,\n"
+    "    $class$> $name$ =\n"
+    "        com.google.protobuf.nano.Extension.create$repeated$$ext_type$(\n"
+    "            com.google.protobuf.nano.Extension.$type$,\n"
+    "            $class$.class,\n"
+    "            $tag_params$);\n");
 }
 
 }  // namespace javanano
diff --git a/src/google/protobuf/compiler/javanano/javanano_helpers.cc b/src/google/protobuf/compiler/javanano/javanano_helpers.cc
index ede4c6f..e8326a4 100644
--- a/src/google/protobuf/compiler/javanano/javanano_helpers.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_helpers.cc
@@ -264,6 +264,22 @@ string FieldDefaultConstantName(const FieldDescriptor *field) {
   return "_" + RenameJavaKeywords(UnderscoresToCamelCase(field)) + "Default";
 }
 
+void PrintFieldComment(io::Printer* printer, const FieldDescriptor* field) {
+  // We don't want to print group bodies so we cut off after the first line
+  // (the second line for extensions).
+  string def = field->DebugString();
+  string::size_type first_line_end = def.find_first_of('\n');
+  printer->Print("// $def$\n",
+    "def", def.substr(0, first_line_end));
+  if (field->is_extension()) {
+    string::size_type second_line_start = first_line_end + 1;
+    string::size_type second_line_length =
+        def.find('\n', second_line_start) - second_line_start;
+    printer->Print("// $def$\n",
+      "def", def.substr(second_line_start, second_line_length));
+  }
+}
+
 JavaType GetJavaType(FieldDescriptor::Type field_type) {
   switch (field_type) {
     case FieldDescriptor::TYPE_INT32:
@@ -310,7 +326,27 @@ JavaType GetJavaType(FieldDescriptor::Type field_type) {
   return JAVATYPE_INT;
 }
 
-const char* BoxedPrimitiveTypeName(JavaType type) {
+string PrimitiveTypeName(JavaType type) {
+  switch (type) {
+    case JAVATYPE_INT    : return "int";
+    case JAVATYPE_LONG   : return "long";
+    case JAVATYPE_FLOAT  : return "float";
+    case JAVATYPE_DOUBLE : return "double";
+    case JAVATYPE_BOOLEAN: return "boolean";
+    case JAVATYPE_STRING : return "java.lang.String";
+    case JAVATYPE_BYTES  : return "byte[]";
+    case JAVATYPE_ENUM   : return "int";
+    case JAVATYPE_MESSAGE: return NULL;
+
+    // No default because we want the compiler to complain if any new
+    // JavaTypes are added.
+  }
+
+  GOOGLE_LOG(FATAL) << "Can't get here.";
+  return NULL;
+}
+
+string BoxedPrimitiveTypeName(JavaType type) {
   switch (type) {
     case JAVATYPE_INT    : return "java.lang.Integer";
     case JAVATYPE_LONG   : return "java.lang.Long";
diff --git a/src/google/protobuf/compiler/javanano/javanano_helpers.h b/src/google/protobuf/compiler/javanano/javanano_helpers.h
index 753a4bd..886bff8 100644
--- a/src/google/protobuf/compiler/javanano/javanano_helpers.h
+++ b/src/google/protobuf/compiler/javanano/javanano_helpers.h
@@ -39,6 +39,7 @@
 #include <google/protobuf/compiler/javanano/javanano_params.h>
 #include <google/protobuf/descriptor.pb.h>
 #include <google/protobuf/descriptor.h>
+#include <google/protobuf/io/printer.h>
 
 namespace google {
 namespace protobuf {
@@ -111,6 +112,9 @@ string FieldConstantName(const FieldDescriptor *field);
 
 string FieldDefaultConstantName(const FieldDescriptor *field);
 
+// Print the field's proto-syntax definition as a comment.
+void PrintFieldComment(io::Printer* printer, const FieldDescriptor* field);
+
 enum JavaType {
   JAVATYPE_INT,
   JAVATYPE_LONG,
@@ -129,10 +133,12 @@ inline JavaType GetJavaType(const FieldDescriptor* field) {
   return GetJavaType(field->type());
 }
 
+string PrimitiveTypeName(JavaType type);
+
 // Get the fully-qualified class name for a boxed primitive type, e.g.
 // "java.lang.Integer" for JAVATYPE_INT.  Returns NULL for enum and message
 // types.
-const char* BoxedPrimitiveTypeName(JavaType type);
+string BoxedPrimitiveTypeName(JavaType type);
 
 string EmptyArrayName(const Params& params, const FieldDescriptor* field);
 
diff --git a/src/google/protobuf/compiler/javanano/javanano_message.cc b/src/google/protobuf/compiler/javanano/javanano_message.cc
index c09670a..008bec2 100644
--- a/src/google/protobuf/compiler/javanano/javanano_message.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_message.cc
@@ -54,14 +54,6 @@ using internal::WireFormatLite;
 
 namespace {
 
-void PrintFieldComment(io::Printer* printer, const FieldDescriptor* field) {
-  // Print the field's proto-syntax definition as a comment.  We don't want to
-  // print group bodies so we cut off after the first line.
-  string def = field->DebugString();
-  printer->Print("// $def$\n",
-    "def", def.substr(0, def.find_first_of('\n')));
-}
-
 struct FieldOrderingByNumber {
   inline bool operator()(const FieldDescriptor* a,
                          const FieldDescriptor* b) const {
@@ -82,13 +74,6 @@ const FieldDescriptor** SortFieldsByNumber(const Descriptor* descriptor) {
   return fields;
 }
 
-// Get an identifier that uniquely identifies this type within the file.
-// This is used to declare static variables related to this type at the
-// outermost file scope.
-string UniqueFileScopeIdentifier(const Descriptor* descriptor) {
-  return "static_" + StringReplace(descriptor->full_name(), ".", "_", true);
-}
-
 }  // namespace
 
 // ===================================================================
@@ -149,7 +134,8 @@ void MessageGenerator::Generate(io::Printer* printer) {
   }
   if (params_.store_unknown_fields()) {
     printer->Print(
-      "    com.google.protobuf.nano.ExtendableMessageNano {\n");
+      "    com.google.protobuf.nano.ExtendableMessageNano<$classname$> {\n",
+      "classname", descriptor_->name());
   } else {
     printer->Print(
       "    com.google.protobuf.nano.MessageNano {\n");
@@ -285,22 +271,20 @@ void MessageGenerator::Generate(io::Printer* printer) {
 
 void MessageGenerator::
 GenerateMessageSerializationMethods(io::Printer* printer) {
+  // Rely on the parent implementations of writeTo() and getSerializedSize()
+  // if there are no fields to serialize in this message.
+  if (descriptor_->field_count() == 0) {
+    return;
+  }
+
   scoped_array<const FieldDescriptor*> sorted_fields(
     SortFieldsByNumber(descriptor_));
 
-  // writeTo only throws an exception if it contains one or more fields to write
-  if (descriptor_->field_count() > 0 || params_.store_unknown_fields()) {
-    printer->Print(
-      "\n"
-      "@Override\n"
-      "public void writeTo(com.google.protobuf.nano.CodedOutputByteBufferNano output)\n"
-      "    throws java.io.IOException {\n");
-  } else {
-    printer->Print(
-      "\n"
-      "@Override\n"
-      "public void writeTo(com.google.protobuf.nano.CodedOutputByteBufferNano output) {\n");
-  }
+  printer->Print(
+    "\n"
+    "@Override\n"
+    "public void writeTo(com.google.protobuf.nano.CodedOutputByteBufferNano output)\n"
+    "    throws java.io.IOException {\n");
   printer->Indent();
 
   // Output the fields in sorted order
@@ -308,36 +292,31 @@ GenerateMessageSerializationMethods(io::Printer* printer) {
     GenerateSerializeOneField(printer, sorted_fields[i]);
   }
 
-  // Write unknown fields.
-  if (params_.store_unknown_fields()) {
-    printer->Print(
-      "com.google.protobuf.nano.WireFormatNano.writeUnknownFields(\n"
-      "    unknownFieldData, output);\n");
-  }
+  // The parent implementation will write any unknown fields if necessary.
+  printer->Print(
+    "super.writeTo(output);\n");
 
   printer->Outdent();
   printer->Print("}\n");
 
-  // Rely on the parent implementation of getSerializedSize if there are no fields to
-  // serialize in this MessageNano.
-  if (descriptor_->field_count() != 0) {
-    printer->Print(
-      "\n"
-      "@Override\n"
-      "public int getSerializedSize() {\n"
-      "  int size = super.getSerializedSize();\n");
-    printer->Indent();
-
-    for (int i = 0; i < descriptor_->field_count(); i++) {
-      field_generators_.get(sorted_fields[i]).GenerateSerializedSizeCode(printer);
-    }
+  // The parent implementation will get the serialized size for unknown
+  // fields if necessary.
+  printer->Print(
+    "\n"
+    "@Override\n"
+    "public int getSerializedSize() {\n"
+    "  int size = super.getSerializedSize();\n");
+  printer->Indent();
 
-    printer->Outdent();
-    printer->Print(
-      "  cachedSize = size;\n"
-      "  return size;\n"
-      "}\n");
+  for (int i = 0; i < descriptor_->field_count(); i++) {
+    field_generators_.get(sorted_fields[i]).GenerateSerializedSizeCode(printer);
   }
+
+  printer->Outdent();
+  printer->Print(
+    "  cachedSize = size;\n"
+    "  return size;\n"
+    "}\n");
 }
 
 void MessageGenerator::GenerateMergeFromMethods(io::Printer* printer) {
@@ -371,12 +350,7 @@ void MessageGenerator::GenerateMergeFromMethods(io::Printer* printer) {
   printer->Indent();
   if (params_.store_unknown_fields()) {
     printer->Print(
-        "if (unknownFieldData == null) {\n"
-        "  unknownFieldData =\n"
-        "      new java.util.ArrayList<com.google.protobuf.nano.UnknownFieldData>();\n"
-        "}\n"
-        "if (!com.google.protobuf.nano.WireFormatNano.storeUnknownField(\n"
-        "    unknownFieldData, input, tag)) {\n"
+        "if (!storeUnknownField(input, tag)) {\n"
         "  return this;\n"
         "}\n");
   } else {
diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
index e044e89..a3bc3a8 100644
--- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
@@ -54,26 +54,6 @@ using internal::WireFormatLite;
 
 namespace {
 
-const char* PrimitiveTypeName(JavaType type) {
-  switch (type) {
-    case JAVATYPE_INT    : return "int";
-    case JAVATYPE_LONG   : return "long";
-    case JAVATYPE_FLOAT  : return "float";
-    case JAVATYPE_DOUBLE : return "double";
-    case JAVATYPE_BOOLEAN: return "boolean";
-    case JAVATYPE_STRING : return "java.lang.String";
-    case JAVATYPE_BYTES  : return "byte[]";
-    case JAVATYPE_ENUM   : return NULL;
-    case JAVATYPE_MESSAGE: return NULL;
-
-    // No default because we want the compiler to complain if any new
-    // JavaTypes are added.
-  }
-
-  GOOGLE_LOG(FATAL) << "Can't get here.";
-  return NULL;
-}
-
 bool IsReferenceType(JavaType type) {
   switch (type) {
     case JAVATYPE_INT    : return false;
diff --git a/src/google/protobuf/unittest_extension_nano.proto b/src/google/protobuf/unittest_extension_nano.proto
index 104cfa7..0a775f4 100644
--- a/src/google/protobuf/unittest_extension_nano.proto
+++ b/src/google/protobuf/unittest_extension_nano.proto
@@ -18,31 +18,14 @@ message AnotherMessage {
   optional bool value = 2;
 }
 
-extend ExtendableMessage {
-  optional string some_string = 10;
-  optional int32 some_int = 11;
-  optional int64 some_long = 12;
-  optional float some_float = 13;
-  optional double some_double = 14;
-  optional bool some_bool = 15;
-  optional AnEnum some_enum = 16;
-  optional AnotherMessage some_message = 17;
-  repeated string some_repeated_string = 18;
-  repeated int32 some_repeated_int = 19;
-  repeated int64 some_repeated_long = 20;
-  repeated float some_repeated_float = 21;
-  repeated double some_repeated_double = 22;
-  repeated bool some_repeated_bool = 23;
-  repeated AnEnum some_repeated_enum = 24;
-  repeated AnotherMessage some_repeated_message = 25;
-}
-
 message ContainerMessage {
   extend ExtendableMessage {
     optional bool another_thing = 100;
   }
 }
 
+// For testNanoOptionalGroupWithUnknownFieldsEnabled;
+// not part of the extensions tests.
 message MessageWithGroup {
   optional group Group = 1 {
     optional int32 a = 2;
diff --git a/src/google/protobuf/unittest_extension_packed_nano.proto b/src/google/protobuf/unittest_extension_packed_nano.proto
new file mode 100644
index 0000000..3586de7
--- /dev/null
+++ b/src/google/protobuf/unittest_extension_packed_nano.proto
@@ -0,0 +1,29 @@
+syntax = "proto2";
+
+option java_multiple_files = true;
+option java_package = "com.google.protobuf.nano";
+
+import "google/protobuf/unittest_extension_nano.proto";
+
+// Must be compiled separately due to extension number reuse.
+// The reuse is deliberate, for testing wire compatibility.
+
+message PackedExtensions {
+  extend ExtendableMessage {
+    repeated int32    packed_int32    = 10 [ packed = true ];
+    repeated uint32   packed_uint32   = 11 [ packed = true ];
+    repeated sint32   packed_sint32   = 12 [ packed = true ];
+    repeated int64    packed_int64    = 13 [ packed = true ];
+    repeated uint64   packed_uint64   = 14 [ packed = true ];
+    repeated sint64   packed_sint64   = 15 [ packed = true ];
+    repeated fixed32  packed_fixed32  = 16 [ packed = true ];
+    repeated sfixed32 packed_sfixed32 = 17 [ packed = true ];
+    repeated fixed64  packed_fixed64  = 18 [ packed = true ];
+    repeated sfixed64 packed_sfixed64 = 19 [ packed = true ];
+    repeated bool     packed_bool     = 20 [ packed = true ];
+    repeated float    packed_float    = 21 [ packed = true ];
+    repeated double   packed_double   = 22 [ packed = true ];
+    repeated AnEnum   packed_enum     = 23 [ packed = true ];
+    // Non-packable types omitted.
+  }
+}
diff --git a/src/google/protobuf/unittest_extension_repeated_nano.proto b/src/google/protobuf/unittest_extension_repeated_nano.proto
new file mode 100644
index 0000000..546c2df
--- /dev/null
+++ b/src/google/protobuf/unittest_extension_repeated_nano.proto
@@ -0,0 +1,34 @@
+syntax = "proto2";
+
+option java_multiple_files = true;
+option java_package = "com.google.protobuf.nano";
+
+import "google/protobuf/unittest_extension_nano.proto";
+
+// Must be compiled separately due to extension number reuse.
+// The reuse is deliberate, for testing wire compatibility.
+
+message RepeatedExtensions {
+  extend ExtendableMessage {
+    repeated int32          repeated_int32    = 10;
+    repeated uint32         repeated_uint32   = 11;
+    repeated sint32         repeated_sint32   = 12;
+    repeated int64          repeated_int64    = 13;
+    repeated uint64         repeated_uint64   = 14;
+    repeated sint64         repeated_sint64   = 15;
+    repeated fixed32        repeated_fixed32  = 16;
+    repeated sfixed32       repeated_sfixed32 = 17;
+    repeated fixed64        repeated_fixed64  = 18;
+    repeated sfixed64       repeated_sfixed64 = 19;
+    repeated bool           repeated_bool     = 20;
+    repeated float          repeated_float    = 21;
+    repeated double         repeated_double   = 22;
+    repeated AnEnum         repeated_enum     = 23;
+    repeated string         repeated_string   = 24;
+    repeated bytes          repeated_bytes    = 25;
+    repeated AnotherMessage repeated_message  = 26;
+    repeated group          RepeatedGroup     = 27 {
+      optional int32 a = 1;
+    }
+  }
+}
diff --git a/src/google/protobuf/unittest_extension_singular_nano.proto b/src/google/protobuf/unittest_extension_singular_nano.proto
new file mode 100644
index 0000000..35d9e6e
--- /dev/null
+++ b/src/google/protobuf/unittest_extension_singular_nano.proto
@@ -0,0 +1,34 @@
+syntax = "proto2";
+
+option java_multiple_files = true;
+option java_package = "com.google.protobuf.nano";
+
+import "google/protobuf/unittest_extension_nano.proto";
+
+// Must be compiled separately due to extension number reuse.
+// The reuse is deliberate, for testing wire compatibility.
+
+message SingularExtensions {
+  extend ExtendableMessage {
+    optional int32          some_int32    = 10;
+    optional uint32         some_uint32   = 11;
+    optional sint32         some_sint32   = 12;
+    optional int64          some_int64    = 13;
+    optional uint64         some_uint64   = 14;
+    optional sint64         some_sint64   = 15;
+    optional fixed32        some_fixed32  = 16;
+    optional sfixed32       some_sfixed32 = 17;
+    optional fixed64        some_fixed64  = 18;
+    optional sfixed64       some_sfixed64 = 19;
+    optional bool           some_bool     = 20;
+    optional float          some_float    = 21;
+    optional double         some_double   = 22;
+    optional AnEnum         some_enum     = 23;
+    optional string         some_string   = 24;
+    optional bytes          some_bytes    = 25;
+    optional AnotherMessage some_message  = 26;
+    optional group          SomeGroup     = 27 {
+      optional int32 a = 1;
+    }
+  }
+}