From ec2f2445546b3e63e4102c3541d4330e97cef07e Mon Sep 17 00:00:00 2001 From: Brian Duff Date: Fri, 3 Oct 2014 14:41:43 -0700 Subject: Fix bug with large extension field numbers. Previously, extensions with field numbers greater than 268435455 would result in a compile time error in generated code that looks something like this: Foo.java:3178: error: integer number too large: 3346754610 3346754610); This is because we were trying to represent the tag number (an unsigned int) using a java int constant, but java int constants are signed, and can't exceed Integer.MAX_VALUE. Fixed by declaring it as a long instead, and casting it down to an int in the implementation. This is safe, because the tag value always fits in 32 bis. Change-Id: If2017bacb4e20af667eaeaf9b65ddc2c30a7709f --- src/google/protobuf/compiler/javanano/javanano_extension.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/google/protobuf/compiler/javanano/javanano_extension.cc b/src/google/protobuf/compiler/javanano/javanano_extension.cc index 754ed550..0b9d1d8d 100644 --- a/src/google/protobuf/compiler/javanano/javanano_extension.cc +++ b/src/google/protobuf/compiler/javanano/javanano_extension.cc @@ -140,7 +140,7 @@ void ExtensionGenerator::Generate(io::Printer* printer) const { " com.google.protobuf.nano.Extension.create$repeated$$ext_type$(\n" " com.google.protobuf.nano.Extension.$type$,\n" " $class$.class,\n" - " $tag_params$);\n"); + " $tag_params$L);\n"); } } // namespace javanano -- cgit v1.2.3 From fb96026b8deb79aa023c9f5c460582e8fea8f331 Mon Sep 17 00:00:00 2001 From: Brian Duff Date: Fri, 9 Jan 2015 13:32:38 -0800 Subject: When no clear() is generated, still initialize fields. https://android-review.googlesource.com/#/c/67890/ removed field initialization from the ctor, making it just call clear() instead. When I added the generate_clear option back (as part of the reftypes compat mode) in https://android-review.googlesource.com/#/c/109530/, I forgot to ensure that what clear() used to do was inlined in the constructor. This change fixes NPEs that are happening for users of reftypes_compat_mode who rely on unset repeated fields being empty arrays rather than null. Change-Id: Idb58746c60f4a4054b7ebb5c3b0e76b16ff88184 --- javanano/pom.xml | 9 ++++++ .../java/com/google/protobuf/nano/NanoTest.java | 6 ++++ .../protobuf/compiler/javanano/javanano_message.cc | 32 ++++++++++++++-------- .../protobuf/compiler/javanano/javanano_message.h | 1 + 4 files changed, 36 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/javanano/pom.xml b/javanano/pom.xml index 7a2be9df..64ed4372 100644 --- a/javanano/pom.xml +++ b/javanano/pom.xml @@ -139,6 +139,15 @@ + + + + + target/generated-test-sources diff --git a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java index d60e94ff..c81846e5 100644 --- a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java +++ b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java @@ -36,6 +36,7 @@ import com.google.protobuf.nano.NanoAccessorsOuterClass.TestNanoAccessors; import com.google.protobuf.nano.NanoHasOuterClass.TestAllTypesNanoHas; import com.google.protobuf.nano.NanoOuterClass.TestAllTypesNano; import com.google.protobuf.nano.UnittestRecursiveNano.RecursiveMessageNano; +import com.google.protobuf.nano.NanoReferenceTypesCompat; import com.google.protobuf.nano.UnittestSimpleNano.SimpleMessageNano; import com.google.protobuf.nano.UnittestSingleNano.SingleMessageNano; import com.google.protobuf.nano.testext.Extensions; @@ -4381,6 +4382,11 @@ public class NanoTest extends TestCase { assertMapSet(testMap.sfixed64ToSfixed64Field, int64Values, int64Values); } + public void testRepeatedFieldInitializedInReftypesCompatMode() { + NanoReferenceTypesCompat.TestAllTypesNano proto = new NanoReferenceTypesCompat.TestAllTypesNano(); + assertNotNull(proto.repeatedString); + } + private void assertRepeatedPackablesEqual( NanoRepeatedPackables.NonPacked nonPacked, NanoRepeatedPackables.Packed packed) { // Not using MessageNano.equals() -- that belongs to a separate test. diff --git a/src/google/protobuf/compiler/javanano/javanano_message.cc b/src/google/protobuf/compiler/javanano/javanano_message.cc index 707f6b84..d5cbe9ce 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message.cc @@ -288,14 +288,18 @@ void MessageGenerator::Generate(io::Printer* printer) { } printer->Print("}\n"); } else { + printer->Print( + "\n" + "public $classname$() {\n", + "classname", descriptor_->name()); if (params_.generate_clear()) { - printer->Print( - "\n" - "public $classname$() {\n" - " clear();\n" - "}\n", - "classname", descriptor_->name()); + printer->Print(" clear();\n"); + } else { + printer->Indent(); + GenerateFieldInitializers(printer); + printer->Outdent(); } + printer->Print("}\n"); } // Other methods in this class @@ -495,6 +499,15 @@ void MessageGenerator::GenerateClear(io::Printer* printer) { "classname", descriptor_->name()); printer->Indent(); + GenerateFieldInitializers(printer); + + printer->Outdent(); + printer->Print( + " return this;\n" + "}\n"); +} + +void MessageGenerator::GenerateFieldInitializers(io::Printer* printer) { // Clear bit fields. int totalInts = (field_generators_.total_bits() + 31) / 32; for (int i = 0; i < totalInts; i++) { @@ -520,12 +533,7 @@ void MessageGenerator::GenerateClear(io::Printer* printer) { if (params_.store_unknown_fields()) { printer->Print("unknownFieldData = null;\n"); } - - printer->Outdent(); - printer->Print( - " cachedSize = -1;\n" - " return this;\n" - "}\n"); + printer->Print("cachedSize = -1;\n"); } void MessageGenerator::GenerateEquals(io::Printer* printer) { diff --git a/src/google/protobuf/compiler/javanano/javanano_message.h b/src/google/protobuf/compiler/javanano/javanano_message.h index 6f25a3a0..8504aa83 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.h +++ b/src/google/protobuf/compiler/javanano/javanano_message.h @@ -77,6 +77,7 @@ class MessageGenerator { const FieldDescriptor* field); void GenerateClear(io::Printer* printer); + void GenerateFieldInitializers(io::Printer* printer); void GenerateEquals(io::Printer* printer); void GenerateHashCode(io::Printer* printer); -- cgit v1.2.3 From d099a88685bf4b2df1689eb4cc56e75065cb87b1 Mon Sep 17 00:00:00 2001 From: Brian Duff Date: Wed, 1 Oct 2014 13:33:40 -0700 Subject: Add clone() method support for nano. Upstreamed from Another Place (cr/57247854). Change-Id: I2aaf59544c0f5ae21a51891d8a5eeda1dc722c90 --- javanano/pom.xml | 6 +-- .../protobuf/nano/ExtendableMessageNano.java | 7 +++ .../java/com/google/protobuf/nano/FieldArray.java | 17 ++++++- .../java/com/google/protobuf/nano/FieldData.java | 52 +++++++++++++++++++++- .../com/google/protobuf/nano/InternalNano.java | 8 ++++ .../java/com/google/protobuf/nano/MessageNano.java | 8 ++++ .../com/google/protobuf/nano/UnknownFieldData.java | 4 ++ .../java/com/google/protobuf/nano/NanoTest.java | 20 +++++++++ .../protobuf/nano/unittest_extension_nano.proto | 1 + .../compiler/javanano/javanano_enum_field.cc | 8 ++++ .../compiler/javanano/javanano_enum_field.h | 1 + .../protobuf/compiler/javanano/javanano_field.h | 1 + .../compiler/javanano/javanano_generator.cc | 2 + .../protobuf/compiler/javanano/javanano_message.cc | 44 ++++++++++++++++-- .../protobuf/compiler/javanano/javanano_message.h | 1 + .../compiler/javanano/javanano_message_field.cc | 29 ++++++++++++ .../compiler/javanano/javanano_message_field.h | 3 ++ .../protobuf/compiler/javanano/javanano_params.h | 11 ++++- .../compiler/javanano/javanano_primitive_field.cc | 8 ++++ .../compiler/javanano/javanano_primitive_field.h | 1 + 20 files changed, 222 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/javanano/pom.xml b/javanano/pom.xml index 64ed4372..3d8cfb9f 100644 --- a/javanano/pom.xml +++ b/javanano/pom.xml @@ -97,19 +97,19 @@ - + - + - + diff --git a/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java b/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java index b979390d..aeacbbf3 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java +++ b/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java @@ -184,4 +184,11 @@ public abstract class ExtendableMessageNano> return (unknownFieldData == null || unknownFieldData.isEmpty() ? 0 : unknownFieldData.hashCode()); } + + @Override + public M clone() throws CloneNotSupportedException { + M cloned = (M) super.clone(); + InternalNano.cloneUnknownFieldData(this, cloned); + return cloned; + } } diff --git a/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java b/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java index cdb66da2..c2044f6a 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java +++ b/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java @@ -37,7 +37,7 @@ package com.google.protobuf.nano; * * Based on {@link android.support.v4.util.SpareArrayCompat}. */ -class FieldArray { +class FieldArray implements Cloneable { private static final FieldData DELETED = new FieldData(); private boolean mGarbage = false; @@ -270,4 +270,19 @@ class FieldArray { } return true; } + + @Override + public final FieldArray clone() { + // Trigger GC so we compact and don't copy DELETED elements. + int size = size(); + FieldArray clone = new FieldArray(size); + System.arraycopy(mFieldNumbers, 0, clone.mFieldNumbers, 0, size); + for (int i = 0; i < size; i++) { + if (mData[i] != null) { + clone.mData[i] = mData[i].clone(); + } + } + clone.mSize = size; + return clone; + } } diff --git a/javanano/src/main/java/com/google/protobuf/nano/FieldData.java b/javanano/src/main/java/com/google/protobuf/nano/FieldData.java index 21ead88b..ebebabc8 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/FieldData.java +++ b/javanano/src/main/java/com/google/protobuf/nano/FieldData.java @@ -39,7 +39,7 @@ import java.util.List; * Stores unknown fields. These might be extensions or fields that the generated API doesn't * know about yet. */ -class FieldData { +class FieldData implements Cloneable { private Extension cachedExtension; private Object value; /** The serialised values for this object. Will be cleared if getValue is called */ @@ -187,4 +187,54 @@ class FieldData { return result; } + @Override + public final FieldData clone() { + FieldData clone = new FieldData(); + try { + clone.cachedExtension = cachedExtension; + if (unknownFieldData == null) { + clone.unknownFieldData = null; + } else { + clone.unknownFieldData.addAll(unknownFieldData); + } + + // Whether we need to deep clone value depends on its type. Primitive reference types + // (e.g. Integer, Long etc.) are ok, since they're immutable. We need to clone arrays + // and messages. + if (value == null) { + // No cloning required. + } else if (value instanceof MessageNano) { + clone.value = ((MessageNano) value).clone(); + } else if (value instanceof byte[]) { + clone.value = ((byte[]) value).clone(); + } else if (value instanceof byte[][]) { + byte[][] valueArray = (byte[][]) value; + byte[][] cloneArray = new byte[valueArray.length][]; + clone.value = cloneArray; + for (int i = 0; i < valueArray.length; i++) { + cloneArray[i] = valueArray[i].clone(); + } + } else if (value instanceof boolean[]) { + clone.value = ((boolean[]) value).clone(); + } else if (value instanceof int[]) { + clone.value = ((int[]) value).clone(); + } else if (value instanceof long[]) { + clone.value = ((long[]) value).clone(); + } else if (value instanceof float[]) { + clone.value = ((float[]) value).clone(); + } else if (value instanceof double[]) { + clone.value = ((double[]) value).clone(); + } else if (value instanceof MessageNano[]) { + MessageNano[] valueArray = (MessageNano[]) value; + MessageNano[] cloneArray = new MessageNano[valueArray.length]; + clone.value = cloneArray; + for (int i = 0; i < valueArray.length; i++) { + cloneArray[i] = valueArray[i].clone(); + } + } + return clone; + } catch (CloneNotSupportedException e) { + throw new AssertionError(e); + } + } } diff --git a/javanano/src/main/java/com/google/protobuf/nano/InternalNano.java b/javanano/src/main/java/com/google/protobuf/nano/InternalNano.java index e7ba8d12..f1263df5 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/InternalNano.java +++ b/javanano/src/main/java/com/google/protobuf/nano/InternalNano.java @@ -536,4 +536,12 @@ public final class InternalNano { } return o.hashCode(); } + + // This avoids having to make FieldArray public. + public static void cloneUnknownFieldData(ExtendableMessageNano original, + ExtendableMessageNano cloned) { + if (original.unknownFieldData != null) { + cloned.unknownFieldData = (FieldArray) original.unknownFieldData.clone(); + } + } } diff --git a/javanano/src/main/java/com/google/protobuf/nano/MessageNano.java b/javanano/src/main/java/com/google/protobuf/nano/MessageNano.java index 81e58571..23475027 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/MessageNano.java +++ b/javanano/src/main/java/com/google/protobuf/nano/MessageNano.java @@ -187,4 +187,12 @@ public abstract class MessageNano { public String toString() { return MessageNanoPrinter.print(this); } + + /** + * Provides support for cloning. This only works if you specify the generate_clone method. + */ + @Override + public MessageNano clone() throws CloneNotSupportedException { + return (MessageNano) super.clone(); + } } diff --git a/javanano/src/main/java/com/google/protobuf/nano/UnknownFieldData.java b/javanano/src/main/java/com/google/protobuf/nano/UnknownFieldData.java index a17fccf3..b1678d1b 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/UnknownFieldData.java +++ b/javanano/src/main/java/com/google/protobuf/nano/UnknownFieldData.java @@ -42,6 +42,10 @@ import java.util.Arrays; final class UnknownFieldData { final int tag; + /** + * Important: this should be treated as immutable, even though it's possible + * to change the array values. + */ final byte[] bytes; UnknownFieldData(int tag, byte[] bytes) { diff --git a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java index c81846e5..91cc385f 100644 --- a/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java +++ b/javanano/src/test/java/com/google/protobuf/nano/NanoTest.java @@ -3023,6 +3023,10 @@ public class NanoTest extends TestCase { assertTrue(Arrays.equals(floats, message.getExtension(RepeatedExtensions.repeatedFloat))); assertTrue(Arrays.equals(doubles, message.getExtension(RepeatedExtensions.repeatedDouble))); assertTrue(Arrays.equals(enums, message.getExtension(RepeatedExtensions.repeatedEnum))); + + // Clone the message and ensure it's still equal. + Extensions.ExtendableMessage clone = message.clone(); + assertEquals(clone, message); } public void testNullExtensions() throws Exception { @@ -4406,6 +4410,22 @@ public class NanoTest extends TestCase { assertTrue(Arrays.equals(nonPacked.enums, packed.enums)); } + public void testClone() throws Exception { + // A simple message. + AnotherMessage anotherMessage = new AnotherMessage(); + anotherMessage.string = "Hello"; + anotherMessage.value = true; + anotherMessage.integers = new int[] { 1, 2, 3 }; + + AnotherMessage clone = anotherMessage.clone(); + assertEquals(clone, anotherMessage); + + // Verify it was a deep clone - changes to the clone shouldn't affect the + // original. + clone.integers[1] = 100; + assertFalse(clone.equals(anotherMessage)); + } + private void assertHasWireData(MessageNano message, boolean expected) { byte[] bytes = MessageNano.toByteArray(message); int wireLength = bytes.length; diff --git a/javanano/src/test/java/com/google/protobuf/nano/unittest_extension_nano.proto b/javanano/src/test/java/com/google/protobuf/nano/unittest_extension_nano.proto index d1c5766d..ca56b3dd 100644 --- a/javanano/src/test/java/com/google/protobuf/nano/unittest_extension_nano.proto +++ b/javanano/src/test/java/com/google/protobuf/nano/unittest_extension_nano.proto @@ -16,6 +16,7 @@ enum AnEnum { message AnotherMessage { optional string string = 1; optional bool value = 2; + repeated int32 integers = 3; } message ContainerMessage { diff --git a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc index 8a59d323..99b316bf 100644 --- a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc @@ -498,6 +498,14 @@ GenerateSerializedSizeCode(io::Printer* printer) const { "}\n"); } +void RepeatedEnumFieldGenerator:: +GenerateFixClonedCode(io::Printer* printer) const { + printer->Print(variables_, + "if (this.$name$ != null && this.$name$.length > 0) {\n" + " cloned.$name$ = this.$name$.clone();\n" + "}\n"); +} + void RepeatedEnumFieldGenerator:: GenerateEqualsCode(io::Printer* printer) const { printer->Print(variables_, diff --git a/src/google/protobuf/compiler/javanano/javanano_enum_field.h b/src/google/protobuf/compiler/javanano/javanano_enum_field.h index 00adc61f..b94790d6 100644 --- a/src/google/protobuf/compiler/javanano/javanano_enum_field.h +++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.h @@ -106,6 +106,7 @@ class RepeatedEnumFieldGenerator : public FieldGenerator { void GenerateSerializedSizeCode(io::Printer* printer) const; void GenerateEqualsCode(io::Printer* printer) const; void GenerateHashCodeCode(io::Printer* printer) const; + void GenerateFixClonedCode(io::Printer* printer) const; private: void GenerateRepeatedDataSizeCode(io::Printer* printer) const; diff --git a/src/google/protobuf/compiler/javanano/javanano_field.h b/src/google/protobuf/compiler/javanano/javanano_field.h index c2cf091c..57c221f4 100644 --- a/src/google/protobuf/compiler/javanano/javanano_field.h +++ b/src/google/protobuf/compiler/javanano/javanano_field.h @@ -83,6 +83,7 @@ class FieldGenerator { virtual void GenerateSerializedSizeCode(io::Printer* printer) const = 0; virtual void GenerateEqualsCode(io::Printer* printer) const = 0; virtual void GenerateHashCodeCode(io::Printer* printer) const = 0; + virtual void GenerateFixClonedCode(io::Printer* printer) const {} protected: const Params& params_; diff --git a/src/google/protobuf/compiler/javanano/javanano_generator.cc b/src/google/protobuf/compiler/javanano/javanano_generator.cc index b5fbcd5f..5ccac946 100644 --- a/src/google/protobuf/compiler/javanano/javanano_generator.cc +++ b/src/google/protobuf/compiler/javanano/javanano_generator.cc @@ -152,6 +152,8 @@ bool JavaNanoGenerator::Generate(const FileDescriptor* file, params.set_ignore_services(option_value == "true"); } else if (option_name == "parcelable_messages") { params.set_parcelable_messages(option_value == "true"); + } else if (option_name == "generate_clone") { + params.set_generate_clone(option_value == "true"); } else { *error = "Ignore unknown javanano generator option: " + option_name; } diff --git a/src/google/protobuf/compiler/javanano/javanano_message.cc b/src/google/protobuf/compiler/javanano/javanano_message.cc index d5cbe9ce..b28ec082 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message.cc @@ -136,18 +136,23 @@ void MessageGenerator::Generate(io::Printer* printer) { } if (params_.store_unknown_fields() && params_.parcelable_messages()) { printer->Print( - " com.google.protobuf.nano.android.ParcelableExtendableMessageNano<$classname$> {\n", + " com.google.protobuf.nano.android.ParcelableExtendableMessageNano<$classname$>", "classname", descriptor_->name()); } else if (params_.store_unknown_fields()) { printer->Print( - " com.google.protobuf.nano.ExtendableMessageNano<$classname$> {\n", + " com.google.protobuf.nano.ExtendableMessageNano<$classname$>", "classname", descriptor_->name()); } else if (params_.parcelable_messages()) { printer->Print( - " com.google.protobuf.nano.android.ParcelableMessageNano {\n"); + " com.google.protobuf.nano.android.ParcelableMessageNano"); } else { printer->Print( - " com.google.protobuf.nano.MessageNano {\n"); + " com.google.protobuf.nano.MessageNano"); + } + if (params_.generate_clone()) { + printer->Print(" implements java.lang.Cloneable {\n"); + } else { + printer->Print(" {\n"); } printer->Indent(); @@ -306,6 +311,10 @@ void MessageGenerator::Generate(io::Printer* printer) { GenerateClear(printer); + if (params_.generate_clone()) { + GenerateClone(printer); + } + if (params_.generate_equals()) { GenerateEquals(printer); GenerateHashCode(printer); @@ -536,6 +545,33 @@ void MessageGenerator::GenerateFieldInitializers(io::Printer* printer) { printer->Print("cachedSize = -1;\n"); } +void MessageGenerator::GenerateClone(io::Printer* printer) { + printer->Print( + "@Override\n" + "public $classname$ clone() {\n", + "classname", descriptor_->name()); + printer->Indent(); + + printer->Print( + "$classname$ cloned;\n" + "try {\n" + " cloned = ($classname$) super.clone();\n" + "} catch (java.lang.CloneNotSupportedException e) {\n" + " throw new java.lang.AssertionError(e);\n" + "}\n", + "classname", descriptor_->name()); + + for (int i = 0; i < descriptor_->field_count(); i++) { + field_generators_.get(descriptor_->field(i)).GenerateFixClonedCode(printer); + } + + printer->Outdent(); + printer->Print( + " return cloned;\n" + "}\n" + "\n"); +} + void MessageGenerator::GenerateEquals(io::Printer* printer) { // Don't override if there are no fields. We could generate an // equals method that compares types, but often empty messages diff --git a/src/google/protobuf/compiler/javanano/javanano_message.h b/src/google/protobuf/compiler/javanano/javanano_message.h index 8504aa83..281ec64f 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.h +++ b/src/google/protobuf/compiler/javanano/javanano_message.h @@ -80,6 +80,7 @@ class MessageGenerator { void GenerateFieldInitializers(io::Printer* printer); void GenerateEquals(io::Printer* printer); void GenerateHashCode(io::Printer* printer); + void GenerateClone(io::Printer* printer); const Params& params_; const Descriptor* descriptor_; diff --git a/src/google/protobuf/compiler/javanano/javanano_message_field.cc b/src/google/protobuf/compiler/javanano/javanano_message_field.cc index 181c4060..d1d04b52 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message_field.cc @@ -126,6 +126,14 @@ GenerateSerializedSizeCode(io::Printer* printer) const { "}\n"); } +void MessageFieldGenerator:: +GenerateFixClonedCode(io::Printer* printer) const { + printer->Print(variables_, + "if (this.$name$ != null) {\n" + " cloned.$name$ = this.$name$.clone();\n" + "}\n"); +} + void MessageFieldGenerator:: GenerateEqualsCode(io::Printer* printer) const { printer->Print(variables_, @@ -212,6 +220,14 @@ GenerateSerializedSizeCode(io::Printer* printer) const { "}\n"); } +void MessageOneofFieldGenerator:: +GenerateFixClonedCode(io::Printer* printer) const { + printer->Print(variables_, + "if (this.$oneof_name$ != null) {\n" + " cloned.$oneof_name$ = this.$oneof_name$.clone();\n" + "}\n"); +} + void MessageOneofFieldGenerator:: GenerateEqualsCode(io::Printer* printer) const { GenerateOneofFieldEquals(descriptor_, variables_, printer); @@ -312,6 +328,19 @@ GenerateSerializedSizeCode(io::Printer* printer) const { "}\n"); } +void RepeatedMessageFieldGenerator:: +GenerateFixClonedCode(io::Printer* printer) const { + printer->Print(variables_, + "if (this.$name$ != null && this.$name$.length > 0) {\n" + " cloned.$name$ = new $type$[this.$name$.length];\n" + " for (int i = 0; i < this.$name$.length; i++) {\n" + " if (this.$name$[i] != null) {\n" + " cloned.$name$[i] = this.$name$[i].clone();\n" + " }\n" + " }\n" + "}\n"); +} + void RepeatedMessageFieldGenerator:: GenerateEqualsCode(io::Printer* printer) const { printer->Print(variables_, diff --git a/src/google/protobuf/compiler/javanano/javanano_message_field.h b/src/google/protobuf/compiler/javanano/javanano_message_field.h index 6c615f5e..e074735c 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message_field.h +++ b/src/google/protobuf/compiler/javanano/javanano_message_field.h @@ -58,6 +58,7 @@ class MessageFieldGenerator : public FieldGenerator { void GenerateSerializedSizeCode(io::Printer* printer) const; void GenerateEqualsCode(io::Printer* printer) const; void GenerateHashCodeCode(io::Printer* printer) const; + void GenerateFixClonedCode(io::Printer* printer) const; private: const FieldDescriptor* descriptor_; @@ -80,6 +81,7 @@ class MessageOneofFieldGenerator : public FieldGenerator { void GenerateSerializedSizeCode(io::Printer* printer) const; void GenerateEqualsCode(io::Printer* printer) const; void GenerateHashCodeCode(io::Printer* printer) const; + void GenerateFixClonedCode(io::Printer* printer) const; private: const FieldDescriptor* descriptor_; @@ -102,6 +104,7 @@ class RepeatedMessageFieldGenerator : public FieldGenerator { void GenerateSerializedSizeCode(io::Printer* printer) const; void GenerateEqualsCode(io::Printer* printer) const; void GenerateHashCodeCode(io::Printer* printer) const; + void GenerateFixClonedCode(io::Printer* printer) const; private: const FieldDescriptor* descriptor_; diff --git a/src/google/protobuf/compiler/javanano/javanano_params.h b/src/google/protobuf/compiler/javanano/javanano_params.h index 4691f360..77bc717d 100644 --- a/src/google/protobuf/compiler/javanano/javanano_params.h +++ b/src/google/protobuf/compiler/javanano/javanano_params.h @@ -66,6 +66,7 @@ class Params { bool parcelable_messages_; bool reftypes_primitive_enums_; bool generate_clear_; + bool generate_clone_; public: Params(const string & base_name) : @@ -81,7 +82,8 @@ class Params { ignore_services_(false), parcelable_messages_(false), reftypes_primitive_enums_(false), - generate_clear_(true) { + generate_clear_(true), + generate_clone_(false) { } const string& base_name() const { @@ -231,6 +233,13 @@ class Params { bool generate_clear() const { return generate_clear_; } + + void set_generate_clone(bool value) { + generate_clone_ = value; + } + bool generate_clone() const { + return generate_clone_; + } }; } // namespace javanano diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc index 41bad0a6..978abf2c 100644 --- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc @@ -364,6 +364,14 @@ GenerateSerializedSizeCode(io::Printer* printer) const { } } +void RepeatedPrimitiveFieldGenerator:: +GenerateFixClonedCode(io::Printer* printer) const { + printer->Print(variables_, + "if (this.$name$ != null && this.$name$.length > 0) {\n" + " cloned.$name$ = this.$name$.clone();\n" + "}\n"); +} + void PrimitiveFieldGenerator:: GenerateEqualsCode(io::Printer* printer) const { // We define equality as serialized form equality. If generate_has(), diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.h b/src/google/protobuf/compiler/javanano/javanano_primitive_field.h index ca7116ff..a01981dd 100644 --- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.h +++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.h @@ -131,6 +131,7 @@ class RepeatedPrimitiveFieldGenerator : public FieldGenerator { void GenerateSerializedSizeCode(io::Printer* printer) const; void GenerateEqualsCode(io::Printer* printer) const; void GenerateHashCodeCode(io::Printer* printer) const; + void GenerateFixClonedCode(io::Printer* printer) const; private: void GenerateRepeatedDataSizeCode(io::Printer* printer) const; -- cgit v1.2.3 From ec19be2f3c7a95f3b7d6d0ff7055daead9284d8d Mon Sep 17 00:00:00 2001 From: Jeff Davidson Date: Wed, 11 Feb 2015 13:12:14 -0800 Subject: Generate @IntDef annotations for nanoproto enums. @IntDef is a support library annotation which allows build tools to determine the valid set of values for a given integer field when that field is intended to be restricted like an enum. This avoids the overhead of enums while still allowing for compile-time type checking in most circumstances. Change-Id: Iee02e0b49a8e069f6456572f538e0a0d301fdfd5 --- javanano/README.md | 23 +++++++++++++ .../protobuf/compiler/javanano/javanano_enum.cc | 40 +++++++++++++++++++--- .../compiler/javanano/javanano_enum_field.cc | 24 ++++++++++--- .../compiler/javanano/javanano_generator.cc | 2 ++ .../protobuf/compiler/javanano/javanano_params.h | 11 +++++- 5 files changed, 91 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/javanano/README.md b/javanano/README.md index 7d696aa7..e19b90b1 100644 --- a/javanano/README.md +++ b/javanano/README.md @@ -145,6 +145,7 @@ optional_field_style -> default or accessors enum_style -> c or java ignore_services -> true or false parcelable_messages -> true or false +generate_intdefs -> true or false ``` **java_package=\|\** (no default) @@ -302,6 +303,28 @@ parcelable_messages -> true or false Android-specific option to generate Parcelable messages. +**generate_intdefs={true,false}** (default: false) + Android-specific option to generate @IntDef annotations for enums. + + If turned on, an '@IntDef' annotation (a public @interface) will be + generated for each enum, and every integer parameter and return + value in the generated code meant for this enum will be annotated + with it. This interface is generated with the same name and at the + same place as the enum members' container interfaces described + above under 'enum_style=java', regardless of the enum_style option + used. When this is combined with enum_style=java, the interface + will be both the '@IntDef' annotation and the container of the enum + members; otherwise the interface has an empty body. + + Your app must declare a compile-time dependency on the + android-support-annotations library. + + For more information on how these @IntDef annotations help with + compile-time type safety, see: + https://sites.google.com/a/android.com/tools/tech-docs/support-annotations + and + https://developer.android.com/reference/android/support/annotation/IntDef.html + To use nano protobufs within the Android repo: ---------------------------------------------- diff --git a/src/google/protobuf/compiler/javanano/javanano_enum.cc b/src/google/protobuf/compiler/javanano/javanano_enum.cc index f934b05f..c6e8dfe9 100644 --- a/src/google/protobuf/compiler/javanano/javanano_enum.cc +++ b/src/google/protobuf/compiler/javanano/javanano_enum.cc @@ -73,13 +73,45 @@ void EnumGenerator::Generate(io::Printer* printer) { "// enum $classname$\n", "classname", descriptor_->name()); + const string classname = RenameJavaKeywords(descriptor_->name()); + // Start of container interface + // If generating intdefs, we use the container interface as the intdef if + // present. Otherwise, we just make an empty @interface parallel to the + // constants. + bool use_intdef = params_.generate_intdefs(); bool use_shell_class = params_.java_enum_style(); - if (use_shell_class) { - printer->Print( - "public interface $classname$ {\n", - "classname", RenameJavaKeywords(descriptor_->name())); + if (use_intdef) { + // @IntDef annotation so tools can enforce correctness + // Annotations will be discarded by the compiler + printer->Print("@java.lang.annotation.Retention(" + "java.lang.annotation.RetentionPolicy.SOURCE)\n" + "@android.support.annotation.IntDef({\n"); printer->Indent(); + for (int i = 0; i < canonical_values_.size(); i++) { + const string constant_name = + RenameJavaKeywords(canonical_values_[i]->name()); + if (use_shell_class) { + printer->Print("$classname$.$name$,\n", + "classname", classname, + "name", constant_name); + } else { + printer->Print("$name$,\n", "name", constant_name); + } + } + printer->Outdent(); + printer->Print("})\n"); + } + if (use_shell_class || use_intdef) { + printer->Print( + "public $at_for_intdef$interface $classname$ {\n", + "classname", classname, + "at_for_intdef", use_intdef ? "@" : ""); + if (use_shell_class) { + printer->Indent(); + } else { + printer->Print("}\n\n"); + } } // Canonical values diff --git a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc index 99b316bf..7666db38 100644 --- a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc @@ -76,6 +76,10 @@ void SetEnumVariables(const Params& params, internal::WireFormatLite::MakeTag(descriptor->number(), internal::WireFormat::WireTypeForFieldType(descriptor->type()))); (*variables)["message_name"] = descriptor->containing_type()->name(); + const EnumDescriptor* enum_type = descriptor->enum_type(); + (*variables)["message_type_intdef"] = "@" + + ToJavaName(params, enum_type->name(), true, + enum_type->containing_type(), enum_type->file()); } void LoadEnumValues(const Params& params, @@ -116,8 +120,10 @@ EnumFieldGenerator::~EnumFieldGenerator() {} void EnumFieldGenerator:: GenerateMembers(io::Printer* printer, bool /* unused lazy_init */) const { - printer->Print(variables_, - "public $type$ $name$;\n"); + if (params_.generate_intdefs()) { + printer->Print(variables_, "$message_type_intdef$\n"); + } + printer->Print(variables_, "public $type$ $name$;\n"); if (params_.generate_has()) { printer->Print(variables_, @@ -256,12 +262,22 @@ AccessorEnumFieldGenerator::~AccessorEnumFieldGenerator() {} void AccessorEnumFieldGenerator:: GenerateMembers(io::Printer* printer, bool /* unused lazy_init */) const { + printer->Print(variables_, "private int $name$_;\n"); + if (params_.generate_intdefs()) { + printer->Print(variables_, "$message_type_intdef$\n"); + } printer->Print(variables_, - "private int $name$_;\n" "public int get$capitalized_name$() {\n" " return $name$_;\n" "}\n" - "public $message_name$ set$capitalized_name$(int value) {\n" + "public $message_name$ set$capitalized_name$("); + if (params_.generate_intdefs()) { + printer->Print(variables_, + "\n" + " $message_type_intdef$ "); + } + printer->Print(variables_, + "int value) {\n" " $name$_ = value;\n" " $set_has$;\n" " return this;\n" diff --git a/src/google/protobuf/compiler/javanano/javanano_generator.cc b/src/google/protobuf/compiler/javanano/javanano_generator.cc index 5ccac946..67b25748 100644 --- a/src/google/protobuf/compiler/javanano/javanano_generator.cc +++ b/src/google/protobuf/compiler/javanano/javanano_generator.cc @@ -154,6 +154,8 @@ bool JavaNanoGenerator::Generate(const FileDescriptor* file, params.set_parcelable_messages(option_value == "true"); } else if (option_name == "generate_clone") { params.set_generate_clone(option_value == "true"); + } else if (option_name == "generate_intdefs") { + params.set_generate_intdefs(option_value == "true"); } else { *error = "Ignore unknown javanano generator option: " + option_name; } diff --git a/src/google/protobuf/compiler/javanano/javanano_params.h b/src/google/protobuf/compiler/javanano/javanano_params.h index 77bc717d..e3b4bb93 100644 --- a/src/google/protobuf/compiler/javanano/javanano_params.h +++ b/src/google/protobuf/compiler/javanano/javanano_params.h @@ -67,6 +67,7 @@ class Params { bool reftypes_primitive_enums_; bool generate_clear_; bool generate_clone_; + bool generate_intdefs_; public: Params(const string & base_name) : @@ -83,7 +84,8 @@ class Params { parcelable_messages_(false), reftypes_primitive_enums_(false), generate_clear_(true), - generate_clone_(false) { + generate_clone_(false), + generate_intdefs_(false) { } const string& base_name() const { @@ -240,6 +242,13 @@ class Params { bool generate_clone() const { return generate_clone_; } + + void set_generate_intdefs(bool value) { + generate_intdefs_ = value; + } + bool generate_intdefs() const { + return generate_intdefs_; + } }; } // namespace javanano -- cgit v1.2.3 From dac7e02d2b9942953481bbe88241d4bf914ef30c Mon Sep 17 00:00:00 2001 From: Brian Duff Date: Sat, 21 Feb 2015 15:22:43 -0800 Subject: Expose generate_clear as an option. I wasn't able to get the clear() method to inline into the constructor when optimizations are on in proguard. As a result, every message has an extra superfluous kept method assuming the app never uses clear() directly. There are a couple of instances where setting this option false is necessary in order to get code dexing successfully without hitting the method limit, e.g. https://goto.google.com/tltzq In this example, I tried turning on the method/inlining/unique and method/inlining/short optimizations before resorting to adding the generate_clear option, but the method count did not decrease. The clear() methods were contributing over a thousand extra methods. Change-Id: If6a9651d6a59cdf70b1040d8248779710ac73105 --- src/google/protobuf/compiler/javanano/javanano_generator.cc | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/google/protobuf/compiler/javanano/javanano_generator.cc b/src/google/protobuf/compiler/javanano/javanano_generator.cc index 67b25748..ad215cb7 100644 --- a/src/google/protobuf/compiler/javanano/javanano_generator.cc +++ b/src/google/protobuf/compiler/javanano/javanano_generator.cc @@ -156,6 +156,8 @@ bool JavaNanoGenerator::Generate(const FileDescriptor* file, params.set_generate_clone(option_value == "true"); } else if (option_name == "generate_intdefs") { params.set_generate_intdefs(option_value == "true"); + } else if (option_name == "generate_clear") { + params.set_generate_clear(option_value == "true"); } else { *error = "Ignore unknown javanano generator option: " + option_name; } -- cgit v1.2.3 From a69b461e1eee43a277839825f1153b8260a28e87 Mon Sep 17 00:00:00 2001 From: Brian Duff Date: Fri, 20 Mar 2015 11:53:33 -0700 Subject: Inline unknownFieldData{Equals,HashCode} to generated code. It turns out dex (apparently) was inlining these protected final methods from ExtendableMessageNano into every message class. Removing these methods from the base class and inlining their code reduces the method count by 2 methods / message when the store_unknown_fields option is on. Change-Id: I0aa09f2016d39939c4c8b8219601793b8fab301f --- .../protobuf/nano/ExtendableMessageNano.java | 25 ---------------------- .../java/com/google/protobuf/nano/FieldArray.java | 19 +++++++++------- .../protobuf/compiler/javanano/javanano_message.cc | 10 +++++++-- 3 files changed, 19 insertions(+), 35 deletions(-) (limited to 'src') diff --git a/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java b/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java index aeacbbf3..87973d76 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java +++ b/javanano/src/main/java/com/google/protobuf/nano/ExtendableMessageNano.java @@ -160,31 +160,6 @@ public abstract class ExtendableMessageNano> return true; } - /** - * Returns whether the stored unknown field data in this message is equivalent to that in the - * other message. - * - * @param other the other message. - * @return whether the two sets of unknown field data are equal. - */ - protected final boolean unknownFieldDataEquals(M other) { - if (unknownFieldData == null || unknownFieldData.isEmpty()) { - return other.unknownFieldData == null || other.unknownFieldData.isEmpty(); - } else { - return unknownFieldData.equals(other.unknownFieldData); - } - } - - /** - * Computes the hashcode representing the unknown field data stored in this message. - * - * @return the hashcode for the unknown field data. - */ - protected final int unknownFieldDataHashCode() { - return (unknownFieldData == null || unknownFieldData.isEmpty() - ? 0 : unknownFieldData.hashCode()); - } - @Override public M clone() throws CloneNotSupportedException { M cloned = (M) super.clone(); diff --git a/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java b/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java index c2044f6a..eca9c0d9 100644 --- a/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java +++ b/javanano/src/main/java/com/google/protobuf/nano/FieldArray.java @@ -35,9 +35,12 @@ package com.google.protobuf.nano; * A custom version of {@link android.util.SparseArray} with the minimal API * for storing {@link FieldData} objects. * + *

This class is an internal implementation detail of nano and should not + * be called directly by clients. + * * Based on {@link android.support.v4.util.SpareArrayCompat}. */ -class FieldArray implements Cloneable { +public final class FieldArray implements Cloneable { private static final FieldData DELETED = new FieldData(); private boolean mGarbage = false; @@ -48,7 +51,7 @@ class FieldArray implements Cloneable { /** * Creates a new FieldArray containing no fields. */ - public FieldArray() { + FieldArray() { this(10); } @@ -57,7 +60,7 @@ class FieldArray implements Cloneable { * require any additional memory allocation to store the specified * number of mappings. */ - public FieldArray(int initialCapacity) { + FieldArray(int initialCapacity) { initialCapacity = idealIntArraySize(initialCapacity); mFieldNumbers = new int[initialCapacity]; mData = new FieldData[initialCapacity]; @@ -68,7 +71,7 @@ class FieldArray implements Cloneable { * Gets the FieldData mapped from the specified fieldNumber, or null * if no such mapping has been made. */ - public FieldData get(int fieldNumber) { + FieldData get(int fieldNumber) { int i = binarySearch(fieldNumber); if (i < 0 || mData[i] == DELETED) { @@ -81,7 +84,7 @@ class FieldArray implements Cloneable { /** * Removes the data from the specified fieldNumber, if there was any. */ - public void remove(int fieldNumber) { + void remove(int fieldNumber) { int i = binarySearch(fieldNumber); if (i >= 0 && mData[i] != DELETED) { @@ -118,7 +121,7 @@ class FieldArray implements Cloneable { * Adds a mapping from the specified fieldNumber to the specified data, * replacing the previous mapping if there was one. */ - public void put(int fieldNumber, FieldData data) { + void put(int fieldNumber, FieldData data) { int i = binarySearch(fieldNumber); if (i >= 0) { @@ -167,7 +170,7 @@ class FieldArray implements Cloneable { * Returns the number of key-value mappings that this FieldArray * currently stores. */ - public int size() { + int size() { if (mGarbage) { gc(); } @@ -184,7 +187,7 @@ class FieldArray implements Cloneable { * the value from the indexth key-value mapping that this * FieldArray stores. */ - public FieldData dataAt(int index) { + FieldData dataAt(int index) { if (mGarbage) { gc(); } diff --git a/src/google/protobuf/compiler/javanano/javanano_message.cc b/src/google/protobuf/compiler/javanano/javanano_message.cc index b28ec082..060d25f0 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message.cc @@ -612,7 +612,11 @@ void MessageGenerator::GenerateEquals(io::Printer* printer) { if (params_.store_unknown_fields()) { printer->Print( - "return unknownFieldDataEquals(other);\n"); + "if (unknownFieldData == null || unknownFieldData.isEmpty()) {\n" + " return other.unknownFieldData == null || other.unknownFieldData.isEmpty();\n" + "} else {\n" + " return unknownFieldData.equals(other.unknownFieldData);\n" + "}"); } else { printer->Print( "return true;\n"); @@ -642,7 +646,9 @@ void MessageGenerator::GenerateHashCode(io::Printer* printer) { if (params_.store_unknown_fields()) { printer->Print( - "result = 31 * result + unknownFieldDataHashCode();\n"); + "result = 31 * result + \n" + " (unknownFieldData == null || unknownFieldData.isEmpty() ? 0 : \n" + " unknownFieldData.hashCode());\n"); } printer->Print("return result;\n"); -- cgit v1.2.3 From 9d546c85bda48c59ba10e240afbf731bd0775bc4 Mon Sep 17 00:00:00 2001 From: Jeff Davidson Date: Thu, 2 Apr 2015 14:46:35 -0700 Subject: Generate a CREATOR for each Parcelable message. This is less ideal from a dex count perspective because it requires a new variable for each message, and because most apps have proguard rules that will ensure that CREATOR classes are retained. However, it is required to be able to use nano protos inside of AIDL files, as the autogenerated AIDL code fails to compile otherwise. This is a substantial benefit as it allows for backwards-compatible parameters and return types in AIDL methods along the lines of safeparcel. Bug: 19084705 Change-Id: I66a2c0424b96cf8ff6b631b186cc4f9407dfc1f4 --- src/google/protobuf/compiler/javanano/javanano_message.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'src') diff --git a/src/google/protobuf/compiler/javanano/javanano_message.cc b/src/google/protobuf/compiler/javanano/javanano_message.cc index 060d25f0..a41da5ae 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message.cc @@ -156,6 +156,17 @@ void MessageGenerator::Generate(io::Printer* printer) { } printer->Indent(); + if (params_.parcelable_messages()) { + printer->Print( + "\n" + "// Used by Parcelable\n" + "@SuppressWarnings({\"unused\"})\n" + "public static final android.os.Parcelable.Creator<$classname$> CREATOR =\n" + " new com.google.protobuf.nano.android.ParcelableMessageNanoCreator<\n" + " $classname$>($classname$.class);\n", + "classname", descriptor_->name()); + } + // Nested types and extensions for (int i = 0; i < descriptor_->extension_count(); i++) { ExtensionGenerator(descriptor_->extension(i), params_).Generate(printer); -- cgit v1.2.3