Fix repeated field merging semantics.

The public doc states that repeated fields are simply concatenated
and doesn't impose a different semantics for packed fields. This
CL fixes this for packed fields and adds tests covering all cases.

Also fixed a bit of missed null-repeated-field treatments.

Change-Id: Ie35277bb1a9f0b8171dc9d07b6adf9b9d3308de2

4 files changed, 95 insertions, 33 deletions

diff --git a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc
index cdb3d09..fe67d76 100644
--- a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc
@@ -269,24 +269,32 @@ GenerateMergingCode(io::Printer* printer) const {
       "  arrayLength++;\n"
       "}\n"
       "input.rewindToPosition(startPos);\n"
-      "this.$name$ = new $type$[arrayLength];\n"
-      "for (int i = 0; i < arrayLength; i++) {\n"
-      "  this.$name$[i] = input.readInt32();\n"
+      "int i = this.$name$ == null ? 0 : this.$name$.length;\n"
+      "int[] newArray = new int[i + arrayLength];\n"
+      "if (i != 0) {\n"
+      "  java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
+      "}\n"
+      "for (; i < newArray.length; i++) {\n"
+      "  newArray[i] = input.readInt32();\n"
       "}\n"
+      "this.$name$ = newArray;\n"
       "input.popLimit(limit);\n");
   } else {
     printer->Print(variables_,
-      "int arrayLength = com.google.protobuf.nano.WireFormatNano.getRepeatedFieldArrayLength(input, $tag$);\n"
-      "int i = this.$name$.length;\n"
+      "int arrayLength = com.google.protobuf.nano.WireFormatNano.\n"
+      "    getRepeatedFieldArrayLength(input, $tag$);\n"
+      "int i = this.$name$ == null ? 0 : this.$name$.length;\n"
       "int[] newArray = new int[i + arrayLength];\n"
-      "System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
-      "this.$name$ = newArray;\n"
-      "for (; i < this.$name$.length - 1; i++) {\n"
-      "  this.$name$[i] = input.readInt32();\n"
+      "if (i != 0) {\n"
+      "  java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
+      "}\n"
+      "for (; i < newArray.length - 1; i++) {\n"
+      "  newArray[i] = input.readInt32();\n"
       "  input.readTag();\n"
       "}\n"
       "// Last one without readTag.\n"
-      "this.$name$[i] = input.readInt32();\n");
+      "newArray[i] = input.readInt32();\n"
+      "this.$name$ = newArray;\n");
   }
 }
 
diff --git a/src/google/protobuf/compiler/javanano/javanano_message_field.cc b/src/google/protobuf/compiler/javanano/javanano_message_field.cc
index 04f1c14..027428f 100644
--- a/src/google/protobuf/compiler/javanano/javanano_message_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_message_field.cc
@@ -235,34 +235,36 @@ GenerateMergingCode(io::Printer* printer) const {
     "    .getRepeatedFieldArrayLength(input, $tag$);\n"
     "int i = this.$name$ == null ? 0 : this.$name$.length;\n"
     "$type$[] newArray = new $type$[i + arrayLength];\n"
-    "if (this.$name$ != null) {\n"
-    "  System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
+    "if (i != 0) {\n"
+    "  java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
     "}\n"
-    "this.$name$ = newArray;\n"
-    "for (; i < this.$name$.length - 1; i++) {\n"
-    "  this.$name$[i] = new $type$();\n");
+    "for (; i < newArray.length - 1; i++) {\n"
+    "  newArray[i] = new $type$();\n");
 
   if (descriptor_->type() == FieldDescriptor::TYPE_GROUP) {
     printer->Print(variables_,
-      "  input.readGroup(this.$name$[i], $number$);\n");
+      "  input.readGroup(newArray[i], $number$);\n");
   } else {
     printer->Print(variables_,
-      "  input.readMessage(this.$name$[i]);\n");
+      "  input.readMessage(newArray[i]);\n");
   }
 
   printer->Print(variables_,
     "  input.readTag();\n"
     "}\n"
     "// Last one without readTag.\n"
-    "this.$name$[i] = new $type$();\n");
+    "newArray[i] = new $type$();\n");
 
   if (descriptor_->type() == FieldDescriptor::TYPE_GROUP) {
     printer->Print(variables_,
-      "input.readGroup(this.$name$[i], $number$);\n");
+      "input.readGroup(newArray[i], $number$);\n");
   } else {
     printer->Print(variables_,
-      "input.readMessage(this.$name$[i]);\n");
+      "input.readMessage(newArray[i]);\n");
   }
+
+  printer->Print(variables_,
+    "this.$name$ = newArray;\n");
 }
 
 void RepeatedMessageFieldGenerator::
diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
index 8097be8..0562a6e 100644
--- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
@@ -519,35 +519,40 @@ GenerateMergingCode(io::Printer* printer) const {
       "  arrayLength++;\n"
       "}\n"
       "input.rewindToPosition(startPos);\n"
-      "this.$name$ = new $type$[arrayLength];\n"
-      "for (int i = 0; i < arrayLength; i++) {\n"
-      "  this.$name$[i] = input.read$capitalized_type$();\n"
+      "int i = this.$name$ == null ? 0 : this.$name$.length;\n"
+      "$type$[] newArray = new $type$[i + arrayLength];\n"
+      "if (i != 0) {\n"
+      "  java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
       "}\n"
+      "for (; i < newArray.length; i++) {\n"
+      "  newArray[i] = input.read$capitalized_type$();\n"
+      "}\n"
+      "this.$name$ = newArray;\n"
       "input.popLimit(limit);\n");
   } else {
     printer->Print(variables_,
       "int arrayLength = com.google.protobuf.nano.WireFormatNano\n"
       "    .getRepeatedFieldArrayLength(input, $tag$);\n"
-      "int i = this.$name$.length;\n");
+      "int i = this.$name$ == null ? 0 : this.$name$.length;\n");
 
     if (GetJavaType(descriptor_) == JAVATYPE_BYTES) {
       printer->Print(variables_,
-        "byte[][] newArray = new byte[i + arrayLength][];\n"
-        "System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
-        "this.$name$ = newArray;\n");
+        "byte[][] newArray = new byte[i + arrayLength][];\n");
     } else {
       printer->Print(variables_,
-        "$type$[] newArray = new $type$[i + arrayLength];\n"
-        "System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
-        "this.$name$ = newArray;\n");
+        "$type$[] newArray = new $type$[i + arrayLength];\n");
     }
     printer->Print(variables_,
-      "for (; i < this.$name$.length - 1; i++) {\n"
-      "  this.$name$[i] = input.read$capitalized_type$();\n"
+      "if (i != 0) {\n"
+      "  java.lang.System.arraycopy(this.$name$, 0, newArray, 0, i);\n"
+      "}\n"
+      "for (; i < newArray.length - 1; i++) {\n"
+      "  newArray[i] = input.read$capitalized_type$();\n"
       "  input.readTag();\n"
       "}\n"
       "// Last one without readTag.\n"
-      "this.$name$[i] = input.read$capitalized_type$();\n");
+      "newArray[i] = input.read$capitalized_type$();\n"
+      "this.$name$ = newArray;\n");
   }
 }
 
diff --git a/src/google/protobuf/unittest_repeated_merge_nano.proto b/src/google/protobuf/unittest_repeated_merge_nano.proto
new file mode 100644
index 0000000..4f03224
--- /dev/null
+++ b/src/google/protobuf/unittest_repeated_merge_nano.proto
@@ -0,0 +1,47 @@
+// Protocol Buffers - Google's data interchange format
+// Copyright 2008 Google Inc.  All rights reserved.
+// http://code.google.com/p/protobuf/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Author: maxtroy@google.com (Max Cai)
+
+package protobuf_unittest;
+
+import "google/protobuf/unittest_nano.proto";
+
+option java_package = "com.google.protobuf.nano";
+option java_multiple_files = true;
+
+// A container message for testing the merging of repeated fields at a
+// nested level. Other tests will be done using the repeated fields in
+// TestAllTypesNano.
+message TestRepeatedMergeNano {
+
+  optional TestAllTypesNano contained = 1;
+
+}