From b337f25628ceb3649df5c4e89b5125e7698a9b47 Mon Sep 17 00:00:00 2001 From: Max Cai Date: Fri, 20 Sep 2013 18:29:40 +0100 Subject: [PATCH] Accessor style for optional fields. This CL implements the 'optional_field_style=accessors' option. All optional fields will now be 1 Java field and 1 bit in a shared bitfield behind get/set/has/clear accessor methods. The setter performs null check for reference types (Strings and byte[]s). Also decentralized the clear code generation. Change-Id: I60ac78329e352e76c2f8139fba1f292383080ad3 --- java/README.txt | 45 +++++- .../java/com/google/protobuf/NanoTest.java | 150 +++++++++++++++++- .../compiler/javanano/javanano_enum_field.cc | 86 +++++++++- .../compiler/javanano/javanano_enum_field.h | 28 +++- .../compiler/javanano/javanano_field.cc | 26 ++- .../compiler/javanano/javanano_field.h | 8 +- .../compiler/javanano/javanano_helpers.cc | 7 + .../compiler/javanano/javanano_helpers.h | 5 + .../compiler/javanano/javanano_message.cc | 45 +++--- .../javanano/javanano_message_field.cc | 95 ++++++++++- .../javanano/javanano_message_field.h | 28 +++- .../javanano/javanano_primitive_field.cc | 109 +++++++++++-- .../javanano/javanano_primitive_field.h | 28 +++- .../protobuf/unittest_accessors_nano.proto | 116 ++++++++++++++ 14 files changed, 713 insertions(+), 63 deletions(-) create mode 100644 src/google/protobuf/unittest_accessors_nano.proto diff --git a/java/README.txt b/java/README.txt index 8bfaab0799..9728f48e4c 100644 --- a/java/README.txt +++ b/java/README.txt @@ -411,9 +411,10 @@ Nano version Nano is even smaller than micro, especially in the number of generated functions. It is like micro except: -- No setter/getter/hazzer functions. -- Has state is not available. Outputs all fields not equal to their - default. (See important implications below.) +- Setter/getter/hazzer/clearer functions are opt-in. +- If not opted in, has state is not available. Serialization outputs + all fields not equal to their default. (See important implications + below.) - CodedInputStreamMicro is renamed to CodedInputByteBufferNano and can only take byte[] (not InputStream). - Similar rename from CodedOutputStreamMicro to @@ -426,7 +427,7 @@ functions. It is like micro except: MessageNano. - "bytes" are of java type byte[]. -IMPORTANT: If you have fields with defaults +IMPORTANT: If you have fields with defaults and opt out of accessors How fields with defaults are serialized has changed. Because we don't keep "has" state, any field equal to its default is assumed to be not @@ -435,7 +436,8 @@ change the default value of a field. Senders compiled against an older version of the proto continue to match against the old default, and don't send values to the receiver even though the receiver assumes the new default value. Therefore, think carefully about the implications -of changing the default value. +of changing the default value. Alternatively, turn on accessors and +enjoy the benefit of the explicit has() checks. IMPORTANT: If you have "bytes" fields with non-empty defaults @@ -451,7 +453,8 @@ Nano Generator options java_package -> | java_outer_classname -> | java_multiple_files -> true or false -java_nano_generate_has -> true or false +java_nano_generate_has -> true or false [DEPRECATED] +optional_field_style -> default or accessors java_package: java_outer_classname: @@ -459,6 +462,8 @@ java_multiple_files: Same as Micro version. java_nano_generate_has={true,false} (default: false) + DEPRECATED. Use optional_field_style=accessors. + If true, generates a public boolean variable has accompanying each optional or required field (not present for repeated fields, groups or messages). It is set to false initially @@ -473,6 +478,34 @@ java_nano_generate_has={true,false} (default: false) many cases reading the default works and determining whether the field was received over the wire is irrelevant. +optional_field_style={default,accessors} (default: default) + Defines the style of the generated code for _optional_ fields only. + In the default style, optional fields translate into public mutable + Java fields, and the serialization process is as discussed in the + "IMPORTANT" section above. When set to 'accessors', each optional + field is encapsulated behind 4 accessors, namely get(), + set(), has() and clear() methods, + with the standard semantics. The hazzer's return value determines + whether a field is serialized, so this style is useful when you need + to serialize a field with the default value, or check if a field has + been explicitly set to its default value from the wire. + + Required fields are still translated to one public mutable Java + field each, and repeated fields are still translated to arrays. No + accessors are generated for them. + + optional_field_style=accessors cannot be used together with + java_nano_generate_has=true. If you need the 'has' flag for any + required field (you have no reason to), you can only use + java_nano_generate_has=true. + + IMPORTANT: When using the 'accessor' style, ProGuard should always + be enabled with optimization (don't use -dontoptimize) and allowing + access modification (use -allowaccessmodification). This removes the + unused accessors and maybe inline the rest at the call sites, + reducing the final code size. + TODO(maxtroy): find ProGuard config that would work the best. + To use nano protobufs: - Link with the generated jar file diff --git a/java/src/test/java/com/google/protobuf/NanoTest.java b/java/src/test/java/com/google/protobuf/NanoTest.java index b00f289223..fb6cccb2ff 100644 --- a/java/src/test/java/com/google/protobuf/NanoTest.java +++ b/java/src/test/java/com/google/protobuf/NanoTest.java @@ -40,6 +40,7 @@ import com.google.protobuf.nano.MessageScopeEnumRefNano; import com.google.protobuf.nano.MultipleImportingNonMultipleNano1; import com.google.protobuf.nano.MultipleImportingNonMultipleNano2; import com.google.protobuf.nano.MultipleNameClashNano; +import com.google.protobuf.nano.NanoAccessorsOuterClass.TestNanoAccessors; import com.google.protobuf.nano.NanoHasOuterClass.TestAllTypesNanoHas; import com.google.protobuf.nano.NanoOuterClass; import com.google.protobuf.nano.NanoOuterClass.TestAllTypesNano; @@ -48,7 +49,6 @@ import com.google.protobuf.nano.UnittestMultipleNano; import com.google.protobuf.nano.UnittestRecursiveNano.RecursiveMessageNano; import com.google.protobuf.nano.UnittestSimpleNano.SimpleMessageNano; import com.google.protobuf.nano.UnittestSingleNano.SingleMessageNano; -import com.google.protobuf.nano.UnittestStringutf8Nano.StringUtf8; import junit.framework.TestCase; @@ -62,6 +62,7 @@ import java.util.List; * @author ulas@google.com Ulas Kirazci */ public class NanoTest extends TestCase { + @Override public void setUp() throws Exception { } @@ -2243,6 +2244,153 @@ public class NanoTest extends TestCase { assertEquals(0, newMsg.id); } + public void testNanoWithAccessorsBasic() throws Exception { + TestNanoAccessors msg = new TestNanoAccessors(); + + // Makes sure required and repeated fields are still public fields + msg.id = 3; + msg.repeatedBytes = new byte[2][3]; + + // Test accessors + assertEquals(0, msg.getOptionalInt32()); + assertFalse(msg.hasOptionalInt32()); + msg.setOptionalInt32(135); + assertEquals(135, msg.getOptionalInt32()); + assertTrue(msg.hasOptionalInt32()); + msg.clearOptionalInt32(); + assertFalse(msg.hasOptionalInt32()); + msg.setOptionalInt32(0); // default value + assertTrue(msg.hasOptionalInt32()); + + // Test NPE + try { + msg.setOptionalBytes(null); + fail(); + } catch (NullPointerException expected) {} + try { + msg.setOptionalString(null); + fail(); + } catch (NullPointerException expected) {} + try { + msg.setOptionalNestedMessage(null); + fail(); + } catch (NullPointerException expected) {} + + // Test has bit on bytes field with defaults and clear() re-clones the default array + assertFalse(msg.hasDefaultBytes()); + byte[] defaultBytes = msg.getDefaultBytes(); + msg.setDefaultBytes(defaultBytes); + assertTrue(msg.hasDefaultBytes()); + msg.clearDefaultBytes(); + assertFalse(msg.hasDefaultBytes()); + defaultBytes[0]++; // modify original array + assertFalse(Arrays.equals(defaultBytes, msg.getDefaultBytes())); + + // Test has bits that require additional bit fields + assertFalse(msg.hasBitFieldCheck()); + msg.setBitFieldCheck(0); + assertTrue(msg.hasBitFieldCheck()); + assertFalse(msg.hasBeforeBitFieldCheck()); // checks bit field does not leak + assertFalse(msg.hasAfterBitFieldCheck()); + + // Test clear() clears has bits + msg.setOptionalString("hi"); + msg.setDefaultString("there"); + msg.clear(); + assertFalse(msg.hasOptionalString()); + assertFalse(msg.hasDefaultString()); + assertFalse(msg.hasBitFieldCheck()); + } + + public void testNanoWithAccessorsParseFrom() throws Exception { + TestNanoAccessors msg = null; + // Test false on creation, after clear and upon empty parse. + for (int i = 0; i < 3; i++) { + if (i == 0) { + msg = new TestNanoAccessors(); + } else if (i == 1) { + msg.clear(); + } else if (i == 2) { + msg = TestNanoAccessors.parseFrom(new byte[0]); + } + assertFalse(msg.hasOptionalInt32()); + assertFalse(msg.hasOptionalString()); + assertFalse(msg.hasOptionalBytes()); + assertFalse(msg.hasOptionalNestedEnum()); + assertFalse(msg.hasDefaultInt32()); + assertFalse(msg.hasDefaultString()); + assertFalse(msg.hasDefaultBytes()); + assertFalse(msg.hasDefaultFloatNan()); + assertFalse(msg.hasDefaultNestedEnum()); + msg.setOptionalNestedMessage(new TestNanoAccessors.NestedMessage()); + msg.getOptionalNestedMessage().setBb(2); + msg.setOptionalNestedEnum(TestNanoAccessors.BAZ); + msg.setDefaultInt32(msg.getDefaultInt32()); + } + + byte [] result = MessageNano.toByteArray(msg); + int msgSerializedSize = msg.getSerializedSize(); + //System.out.printf("mss=%d result.length=%d\n", msgSerializedSize, result.length); + assertTrue(msgSerializedSize == 14); + assertEquals(result.length, msgSerializedSize); + + // Has fields true upon parse. + TestNanoAccessors newMsg = TestNanoAccessors.parseFrom(result); + assertEquals(2, newMsg.getOptionalNestedMessage().getBb()); + assertTrue(newMsg.getOptionalNestedMessage().hasBb()); + assertEquals(TestNanoAccessors.BAZ, newMsg.getOptionalNestedEnum()); + assertTrue(newMsg.hasOptionalNestedEnum()); + + // Has field true on fields with explicit default values from wire. + assertTrue(newMsg.hasDefaultInt32()); + assertEquals(41, newMsg.getDefaultInt32()); + } + + public void testNanoWithAccessorsSerialize() throws Exception { + TestNanoAccessors msg = new TestNanoAccessors(); + msg.setOptionalInt32(msg.getOptionalInt32()); + msg.setOptionalString(msg.getOptionalString()); + msg.setOptionalBytes(msg.getOptionalBytes()); + TestNanoAccessors.NestedMessage nestedMessage = new TestNanoAccessors.NestedMessage(); + nestedMessage.setBb(nestedMessage.getBb()); + msg.setOptionalNestedMessage(nestedMessage); + msg.setOptionalNestedEnum(msg.getOptionalNestedEnum()); + msg.setDefaultInt32(msg.getDefaultInt32()); + msg.setDefaultString(msg.getDefaultString()); + msg.setDefaultBytes(msg.getDefaultBytes()); + msg.setDefaultFloatNan(msg.getDefaultFloatNan()); + msg.setDefaultNestedEnum(msg.getDefaultNestedEnum()); + + byte [] result = MessageNano.toByteArray(msg); + int msgSerializedSize = msg.getSerializedSize(); + assertEquals(result.length, msgSerializedSize); + + // Now deserialize and find that all fields are set and equal to their defaults. + TestNanoAccessors newMsg = TestNanoAccessors.parseFrom(result); + assertTrue(newMsg.hasOptionalInt32()); + assertTrue(newMsg.hasOptionalString()); + assertTrue(newMsg.hasOptionalBytes()); + assertTrue(newMsg.hasOptionalNestedMessage()); + assertTrue(newMsg.getOptionalNestedMessage().hasBb()); + assertTrue(newMsg.hasOptionalNestedEnum()); + assertTrue(newMsg.hasDefaultInt32()); + assertTrue(newMsg.hasDefaultString()); + assertTrue(newMsg.hasDefaultBytes()); + assertTrue(newMsg.hasDefaultFloatNan()); + assertTrue(newMsg.hasDefaultNestedEnum()); + assertEquals(0, newMsg.getOptionalInt32()); + assertEquals(0, newMsg.getOptionalString().length()); + assertEquals(0, newMsg.getOptionalBytes().length); + assertEquals(0, newMsg.getOptionalNestedMessage().getBb()); + assertEquals(TestNanoAccessors.FOO, newMsg.getOptionalNestedEnum()); + assertEquals(41, newMsg.getDefaultInt32()); + assertEquals("hello", newMsg.getDefaultString()); + assertEquals("world", new String(newMsg.getDefaultBytes(), "UTF-8")); + assertEquals(TestNanoAccessors.BAR, newMsg.getDefaultNestedEnum()); + assertEquals(Float.NaN, newMsg.getDefaultFloatNan()); + assertEquals(0, newMsg.id); + } + /** * Tests that fields with a default value of NaN are not serialized when * set to NaN. This is a special case as NaN != NaN, so normal equality diff --git a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc index 9c4acb929c..b30b9fbf71 100644 --- a/src/google/protobuf/compiler/javanano/javanano_enum_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.cc @@ -90,7 +90,18 @@ GenerateMembers(io::Printer* printer) const { } void EnumFieldGenerator:: -GenerateParsingCode(io::Printer* printer) const { +GenerateClearCode(io::Printer* printer) const { + printer->Print(variables_, + "$name$ = $default$;\n"); + + if (params_.generate_has()) { + printer->Print(variables_, + "has$capitalized_name$ = false;\n"); + } +} + +void EnumFieldGenerator:: +GenerateMergingCode(io::Printer* printer) const { printer->Print(variables_, " this.$name$ = input.readInt32();\n"); @@ -146,6 +157,71 @@ string EnumFieldGenerator::GetBoxedType() const { // =================================================================== +AccessorEnumFieldGenerator:: +AccessorEnumFieldGenerator(const FieldDescriptor* descriptor, + const Params& params, int has_bit_index) + : FieldGenerator(params), descriptor_(descriptor) { + SetEnumVariables(params, descriptor, &variables_); + SetBitOperationVariables("has", has_bit_index, &variables_); +} + +AccessorEnumFieldGenerator::~AccessorEnumFieldGenerator() {} + +void AccessorEnumFieldGenerator:: +GenerateMembers(io::Printer* printer) const { + printer->Print(variables_, + "private int $name$_ = $default$;\n" + "public int get$capitalized_name$() {\n" + " return $name$_;\n" + "}\n" + "public void set$capitalized_name$(int value) {\n" + " $name$_ = value;\n" + " $set_has$;\n" + "}\n" + "public boolean has$capitalized_name$() {\n" + " return $get_has$;\n" + "}\n" + "public void clear$capitalized_name$() {\n" + " $name$_ = $default$;\n" + " $clear_has$;\n" + "}\n"); +} + +void AccessorEnumFieldGenerator:: +GenerateClearCode(io::Printer* printer) const { + printer->Print(variables_, + "$name$_ = $default$;\n"); +} + +void AccessorEnumFieldGenerator:: +GenerateMergingCode(io::Printer* printer) const { + printer->Print(variables_, + "set$capitalized_name$(input.readInt32());\n"); +} + +void AccessorEnumFieldGenerator:: +GenerateSerializationCode(io::Printer* printer) const { + printer->Print(variables_, + "if (has$capitalized_name$()) {\n" + " output.writeInt32($number$, $name$_);\n" + "}\n"); +} + +void AccessorEnumFieldGenerator:: +GenerateSerializedSizeCode(io::Printer* printer) const { + printer->Print(variables_, + "if (has$capitalized_name$()) {\n" + " size += com.google.protobuf.nano.CodedOutputByteBufferNano\n" + " .computeInt32Size($number$, $name$_);\n" + "}\n"); +} + +string AccessorEnumFieldGenerator::GetBoxedType() const { + return ClassName(params_, descriptor_->enum_type()); +} + +// =================================================================== + RepeatedEnumFieldGenerator:: RepeatedEnumFieldGenerator(const FieldDescriptor* descriptor, const Params& params) : FieldGenerator(params), descriptor_(descriptor) { @@ -165,7 +241,13 @@ GenerateMembers(io::Printer* printer) const { } void RepeatedEnumFieldGenerator:: -GenerateParsingCode(io::Printer* printer) const { +GenerateClearCode(io::Printer* printer) const { + printer->Print(variables_, + "$name$ = com.google.protobuf.nano.WireFormatNano.EMPTY_INT_ARRAY;\n"); +} + +void RepeatedEnumFieldGenerator:: +GenerateMergingCode(io::Printer* printer) const { // First, figure out the length of the array, then parse. if (descriptor_->options().packed()) { 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 6e1a4431d4..934aaaab53 100644 --- a/src/google/protobuf/compiler/javanano/javanano_enum_field.h +++ b/src/google/protobuf/compiler/javanano/javanano_enum_field.h @@ -51,7 +51,8 @@ class EnumFieldGenerator : public FieldGenerator { // implements FieldGenerator --------------------------------------- void GenerateMembers(io::Printer* printer) const; - void GenerateParsingCode(io::Printer* printer) const; + void GenerateClearCode(io::Printer* printer) const; + void GenerateMergingCode(io::Printer* printer) const; void GenerateSerializationCode(io::Printer* printer) const; void GenerateSerializedSizeCode(io::Printer* printer) const; @@ -64,6 +65,28 @@ class EnumFieldGenerator : public FieldGenerator { GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(EnumFieldGenerator); }; +class AccessorEnumFieldGenerator : public FieldGenerator { + public: + explicit AccessorEnumFieldGenerator(const FieldDescriptor* descriptor, + const Params& params, int has_bit_index); + ~AccessorEnumFieldGenerator(); + + // implements FieldGenerator --------------------------------------- + void GenerateMembers(io::Printer* printer) const; + void GenerateClearCode(io::Printer* printer) const; + void GenerateMergingCode(io::Printer* printer) const; + void GenerateSerializationCode(io::Printer* printer) const; + void GenerateSerializedSizeCode(io::Printer* printer) const; + + string GetBoxedType() const; + + private: + const FieldDescriptor* descriptor_; + map variables_; + + GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(AccessorEnumFieldGenerator); +}; + class RepeatedEnumFieldGenerator : public FieldGenerator { public: explicit RepeatedEnumFieldGenerator(const FieldDescriptor* descriptor, const Params& params); @@ -71,7 +94,8 @@ class RepeatedEnumFieldGenerator : public FieldGenerator { // implements FieldGenerator --------------------------------------- void GenerateMembers(io::Printer* printer) const; - void GenerateParsingCode(io::Printer* printer) const; + void GenerateClearCode(io::Printer* printer) const; + void GenerateMergingCode(io::Printer* printer) const; void GenerateSerializationCode(io::Printer* printer) const; void GenerateSerializedSizeCode(io::Printer* printer) const; diff --git a/src/google/protobuf/compiler/javanano/javanano_field.cc b/src/google/protobuf/compiler/javanano/javanano_field.cc index ea25786cc2..6629f96576 100644 --- a/src/google/protobuf/compiler/javanano/javanano_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_field.cc @@ -53,16 +53,21 @@ FieldGeneratorMap::FieldGeneratorMap(const Descriptor* descriptor, const Params extension_generators_( new scoped_ptr[descriptor->extension_count()]) { + int next_has_bit_index = 0; // Construct all the FieldGenerators. for (int i = 0; i < descriptor->field_count(); i++) { - field_generators_[i].reset(MakeGenerator(descriptor->field(i), params)); + field_generators_[i].reset( + MakeGenerator(descriptor->field(i), params, &next_has_bit_index)); } for (int i = 0; i < descriptor->extension_count(); i++) { - extension_generators_[i].reset(MakeGenerator(descriptor->extension(i), params)); + extension_generators_[i].reset( + MakeGenerator(descriptor->extension(i), params, &next_has_bit_index)); } + total_bits_ = next_has_bit_index; } -FieldGenerator* FieldGeneratorMap::MakeGenerator(const FieldDescriptor* field, const Params ¶ms) { +FieldGenerator* FieldGeneratorMap::MakeGenerator(const FieldDescriptor* field, + const Params ¶ms, int* next_has_bit_index) { if (field->is_repeated()) { switch (GetJavaType(field)) { case JAVATYPE_MESSAGE: @@ -72,6 +77,21 @@ FieldGenerator* FieldGeneratorMap::MakeGenerator(const FieldDescriptor* field, c default: return new RepeatedPrimitiveFieldGenerator(field, params); } + } else if (params.optional_field_accessors() && field->is_optional()) { + // We need a has-bit for each primitive/enum field because their default + // values could be same as explicitly set values. But we don't need it + // for a message field because they have no defaults and Nano uses 'null' + // for unset messages, which cannot be set explicitly. + switch (GetJavaType(field)) { + case JAVATYPE_MESSAGE: + return new AccessorMessageFieldGenerator(field, params); + case JAVATYPE_ENUM: + return new AccessorEnumFieldGenerator( + field, params, (*next_has_bit_index)++); + default: + return new AccessorPrimitiveFieldGenerator( + field, params, (*next_has_bit_index)++); + } } else { switch (GetJavaType(field)) { case JAVATYPE_MESSAGE: diff --git a/src/google/protobuf/compiler/javanano/javanano_field.h b/src/google/protobuf/compiler/javanano/javanano_field.h index 7db48a57e5..dad2e7aa33 100644 --- a/src/google/protobuf/compiler/javanano/javanano_field.h +++ b/src/google/protobuf/compiler/javanano/javanano_field.h @@ -58,7 +58,8 @@ class FieldGenerator { virtual ~FieldGenerator(); virtual void GenerateMembers(io::Printer* printer) const = 0; - virtual void GenerateParsingCode(io::Printer* printer) const = 0; + virtual void GenerateClearCode(io::Printer* printer) const = 0; + virtual void GenerateMergingCode(io::Printer* printer) const = 0; virtual void GenerateSerializationCode(io::Printer* printer) const = 0; virtual void GenerateSerializedSizeCode(io::Printer* printer) const = 0; @@ -78,13 +79,16 @@ class FieldGeneratorMap { const FieldGenerator& get(const FieldDescriptor* field) const; const FieldGenerator& get_extension(int index) const; + int total_bits() const { return total_bits_; } private: const Descriptor* descriptor_; scoped_array > field_generators_; scoped_array > extension_generators_; + int total_bits_; - static FieldGenerator* MakeGenerator(const FieldDescriptor* field, const Params ¶ms); + static FieldGenerator* MakeGenerator(const FieldDescriptor* field, + const Params ¶ms, int* next_has_bit_index); GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(FieldGeneratorMap); }; diff --git a/src/google/protobuf/compiler/javanano/javanano_helpers.cc b/src/google/protobuf/compiler/javanano/javanano_helpers.cc index ebc9c63575..630ecd10cd 100644 --- a/src/google/protobuf/compiler/javanano/javanano_helpers.cc +++ b/src/google/protobuf/compiler/javanano/javanano_helpers.cc @@ -484,6 +484,13 @@ string GenerateClearBit(int bit_index) { return result; } +void SetBitOperationVariables(const string name, + int bitIndex, map* variables) { + (*variables)["get_" + name] = GenerateGetBit(bitIndex); + (*variables)["set_" + name] = GenerateSetBit(bitIndex); + (*variables)["clear_" + name] = GenerateClearBit(bitIndex); +} + } // namespace javanano } // namespace compiler } // namespace protobuf diff --git a/src/google/protobuf/compiler/javanano/javanano_helpers.h b/src/google/protobuf/compiler/javanano/javanano_helpers.h index 03fd434756..cddc0777f8 100644 --- a/src/google/protobuf/compiler/javanano/javanano_helpers.h +++ b/src/google/protobuf/compiler/javanano/javanano_helpers.h @@ -163,6 +163,11 @@ string GenerateSetBit(int bit_index); // Example: "bitField1_ = (bitField1_ & ~0x04)" string GenerateClearBit(int bit_index); +// Sets the 'get_*', 'set_*' and 'clear_*' variables, where * is the given bit +// field name, to the appropriate Java expressions for the given bit index. +void SetBitOperationVariables(const string name, + int bitIndex, map* variables); + } // namespace javanano } // namespace compiler } // namespace protobuf diff --git a/src/google/protobuf/compiler/javanano/javanano_message.cc b/src/google/protobuf/compiler/javanano/javanano_message.cc index 851c4434c0..c9ea654ec1 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message.cc @@ -137,9 +137,9 @@ void MessageGenerator::Generate(io::Printer* printer) { // warnings here in the class declaration. printer->Print( "@SuppressWarnings(\"hiding\")\n" - "public $modifiers$ final class $classname$ extends\n" + "public $modifiers$final class $classname$ extends\n" " com.google.protobuf.nano.MessageNano {\n", - "modifiers", is_own_file ? "" : "static", + "modifiers", is_own_file ? "" : "static ", "classname", descriptor_->name()); printer->Indent(); printer->Print( @@ -167,6 +167,13 @@ void MessageGenerator::Generate(io::Printer* printer) { MessageGenerator(descriptor_->nested_type(i), params_).Generate(printer); } + // Integers for bit fields + int totalInts = (field_generators_.total_bits() + 31) / 32; + for (int i = 0; i < totalInts; i++) { + printer->Print("private int $bit_field_name$;\n", + "bit_field_name", GetBitFieldName(i)); + } + // Fields for (int i = 0; i < descriptor_->field_count(); i++) { PrintFieldComment(printer, descriptor_->field(i)); @@ -327,7 +334,7 @@ void MessageGenerator::GenerateMergeFromMethods(io::Printer* printer) { "tag", SimpleItoa(tag)); printer->Indent(); - field_generators_.get(field).GenerateParsingCode(printer); + field_generators_.get(field).GenerateMergingCode(printer); printer->Outdent(); printer->Print( @@ -376,33 +383,17 @@ void MessageGenerator::GenerateClear(io::Printer* printer) { "classname", descriptor_->name()); printer->Indent(); + // Clear bit fields. + int totalInts = (field_generators_.total_bits() + 31) / 32; + for (int i = 0; i < totalInts; i++) { + printer->Print("$bit_field_name$ = 0;\n", + "bit_field_name", GetBitFieldName(i)); + } + // Call clear for all of the fields. for (int i = 0; i < descriptor_->field_count(); i++) { const FieldDescriptor* field = descriptor_->field(i); - - if (field->type() == FieldDescriptor::TYPE_BYTES && - !field->default_value_string().empty()) { - // Need to clone the default value because it is of a mutable - // type. - printer->Print( - "$name$ = $default$.clone();\n", - "name", RenameJavaKeywords(UnderscoresToCamelCase(field)), - "default", DefaultValue(params_, field)); - } else { - printer->Print( - "$name$ = $default$;\n", - "name", RenameJavaKeywords(UnderscoresToCamelCase(field)), - "default", DefaultValue(params_, field)); - } - - if (params_.generate_has() && - field->label() != FieldDescriptor::LABEL_REPEATED && - field->type() != FieldDescriptor::TYPE_GROUP && - field->type() != FieldDescriptor::TYPE_MESSAGE) { - printer->Print( - "has$capitalized_name$ = false;\n", - "capitalized_name", UnderscoresToCapitalizedCamelCase(field)); - } + field_generators_.get(field).GenerateClearCode(printer); } // Clear unknown fields. diff --git a/src/google/protobuf/compiler/javanano/javanano_message_field.cc b/src/google/protobuf/compiler/javanano/javanano_message_field.cc index 7d9a281e9e..02dbdb1742 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_message_field.cc @@ -88,9 +88,17 @@ GenerateMembers(io::Printer* printer) const { } void MessageFieldGenerator:: -GenerateParsingCode(io::Printer* printer) const { +GenerateClearCode(io::Printer* printer) const { printer->Print(variables_, - "this.$name$ = new $type$();\n"); + "$name$ = null;\n"); +} + +void MessageFieldGenerator:: +GenerateMergingCode(io::Printer* printer) const { + printer->Print(variables_, + "if (this.$name$ == null) {\n" + " this.$name$ = new $type$();\n" + "}"); if (descriptor_->type() == FieldDescriptor::TYPE_GROUP) { printer->Print(variables_, @@ -124,6 +132,81 @@ string MessageFieldGenerator::GetBoxedType() const { // =================================================================== +AccessorMessageFieldGenerator:: +AccessorMessageFieldGenerator( + const FieldDescriptor* descriptor, const Params& params) + : FieldGenerator(params), descriptor_(descriptor) { + SetMessageVariables(params, descriptor, &variables_); +} + +AccessorMessageFieldGenerator::~AccessorMessageFieldGenerator() {} + +void AccessorMessageFieldGenerator:: +GenerateMembers(io::Printer* printer) const { + printer->Print(variables_, + "private $type$ $name$_ = null;\n" + "public $type$ get$capitalized_name$() {\n" + " return $name$_;\n" + "}\n" + "public void set$capitalized_name$($type$ value) {\n" + " if (value == null) {\n" + " throw new java.lang.NullPointerException();\n" + " }\n" + " $name$_ = value;\n" + "}\n" + "public boolean has$capitalized_name$() {\n" + " return $name$_ != null;\n" + "}\n" + "public void clear$capitalized_name$() {\n" + " $name$_ = null;\n" + "}\n"); +} + +void AccessorMessageFieldGenerator:: +GenerateClearCode(io::Printer* printer) const { + printer->Print(variables_, + "$name$_ = null;\n"); +} + +void AccessorMessageFieldGenerator:: +GenerateMergingCode(io::Printer* printer) const { + printer->Print(variables_, + "if (!has$capitalized_name$()) {\n" + " set$capitalized_name$(new $type$());\n" + "}"); + + if (descriptor_->type() == FieldDescriptor::TYPE_GROUP) { + printer->Print(variables_, + "input.readGroup($name$_, $number$);\n"); + } else { + printer->Print(variables_, + "input.readMessage($name$_);\n"); + } +} + +void AccessorMessageFieldGenerator:: +GenerateSerializationCode(io::Printer* printer) const { + printer->Print(variables_, + "if (has$capitalized_name$()) {\n" + " output.write$group_or_message$($number$, $name$_);\n" + "}\n"); +} + +void AccessorMessageFieldGenerator:: +GenerateSerializedSizeCode(io::Printer* printer) const { + printer->Print(variables_, + "if (has$capitalized_name$()) {\n" + " size += com.google.protobuf.nano.CodedOutputByteBufferNano\n" + " .compute$group_or_message$Size($number$, $name$_);\n" + "}\n"); +} + +string AccessorMessageFieldGenerator::GetBoxedType() const { + return ClassName(params_, descriptor_->message_type()); +} + +// =================================================================== + RepeatedMessageFieldGenerator:: RepeatedMessageFieldGenerator(const FieldDescriptor* descriptor, const Params& params) : FieldGenerator(params), descriptor_(descriptor) { @@ -139,7 +222,13 @@ GenerateMembers(io::Printer* printer) const { } void RepeatedMessageFieldGenerator:: -GenerateParsingCode(io::Printer* printer) const { +GenerateClearCode(io::Printer* printer) const { + printer->Print(variables_, + "$name$ = $type$.EMPTY_ARRAY;\n"); +} + +void RepeatedMessageFieldGenerator:: +GenerateMergingCode(io::Printer* printer) const { // First, figure out the length of the array, then parse. printer->Print(variables_, "int arrayLength = com.google.protobuf.nano.WireFormatNano.getRepeatedFieldArrayLength(input, $tag$);\n" diff --git a/src/google/protobuf/compiler/javanano/javanano_message_field.h b/src/google/protobuf/compiler/javanano/javanano_message_field.h index 943a8322cf..9807513f31 100644 --- a/src/google/protobuf/compiler/javanano/javanano_message_field.h +++ b/src/google/protobuf/compiler/javanano/javanano_message_field.h @@ -51,7 +51,8 @@ class MessageFieldGenerator : public FieldGenerator { // implements FieldGenerator --------------------------------------- void GenerateMembers(io::Printer* printer) const; - void GenerateParsingCode(io::Printer* printer) const; + void GenerateClearCode(io::Printer* printer) const; + void GenerateMergingCode(io::Printer* printer) const; void GenerateSerializationCode(io::Printer* printer) const; void GenerateSerializedSizeCode(io::Printer* printer) const; @@ -64,6 +65,28 @@ class MessageFieldGenerator : public FieldGenerator { GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(MessageFieldGenerator); }; +class AccessorMessageFieldGenerator : public FieldGenerator { + public: + explicit AccessorMessageFieldGenerator( + const FieldDescriptor* descriptor, const Params& params); + ~AccessorMessageFieldGenerator(); + + // implements FieldGenerator --------------------------------------- + void GenerateMembers(io::Printer* printer) const; + void GenerateClearCode(io::Printer* printer) const; + void GenerateMergingCode(io::Printer* printer) const; + void GenerateSerializationCode(io::Printer* printer) const; + void GenerateSerializedSizeCode(io::Printer* printer) const; + + string GetBoxedType() const; + + private: + const FieldDescriptor* descriptor_; + map variables_; + + GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(AccessorMessageFieldGenerator); +}; + class RepeatedMessageFieldGenerator : public FieldGenerator { public: explicit RepeatedMessageFieldGenerator(const FieldDescriptor* descriptor, @@ -72,7 +95,8 @@ class RepeatedMessageFieldGenerator : public FieldGenerator { // implements FieldGenerator --------------------------------------- void GenerateMembers(io::Printer* printer) const; - void GenerateParsingCode(io::Printer* printer) const; + void GenerateClearCode(io::Printer* printer) const; + void GenerateMergingCode(io::Printer* printer) const; void GenerateSerializationCode(io::Printer* printer) const; void GenerateSerializedSizeCode(io::Printer* printer) const; diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc index 79357e9d82..69664d30f2 100644 --- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc +++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc @@ -260,6 +260,7 @@ void SetPrimitiveVariables(const FieldDescriptor* descriptor, const Params param default_value = strings::Substitute( "com.google.protobuf.nano.InternalNano.bytesDefaultValue(\"$0\")", CEscape(descriptor->default_value_string())); + (*variables)["default_copy_if_needed"] = (*variables)["default"] + ".clone()"; } else { if (AllAscii(descriptor->default_value_string())) { // All chars are ASCII. In this case CEscape() works fine. @@ -269,8 +270,11 @@ void SetPrimitiveVariables(const FieldDescriptor* descriptor, const Params param "com.google.protobuf.nano.InternalNano.stringDefaultValue(\"$0\")", CEscape(descriptor->default_value_string())); } + (*variables)["default_copy_if_needed"] = (*variables)["default"]; } (*variables)["default_constant_value"] = default_value; + } else { + (*variables)["default_copy_if_needed"] = (*variables)["default"]; } (*variables)["boxed_type"] = BoxedPrimitiveTypeName(GetJavaType(descriptor)); (*variables)["capitalized_type"] = GetCapitalizedType(descriptor); @@ -280,7 +284,7 @@ void SetPrimitiveVariables(const FieldDescriptor* descriptor, const Params param if (IsReferenceType(GetJavaType(descriptor))) { (*variables)["null_check"] = " if (value == null) {\n" - " throw new NullPointerException();\n" + " throw new java.lang.NullPointerException();\n" " }\n"; } else { (*variables)["null_check"] = ""; @@ -310,17 +314,9 @@ GenerateMembers(io::Printer* printer) const { // Those primitive types that need a saved default. printer->Print(variables_, "private static final $type$ $default_constant$ = $default_constant_value$;\n"); - if (descriptor_->type() == FieldDescriptor::TYPE_BYTES) { - printer->Print(variables_, - "public $type$ $name$ = $default$.clone();\n"); - } else { - printer->Print(variables_, - "public $type$ $name$ = $default$;\n"); - } - } else { - printer->Print(variables_, - "public $type$ $name$ = $default$;\n"); } + printer->Print(variables_, + "public $type$ $name$ = $default_copy_if_needed$;\n"); if (params_.generate_has()) { printer->Print(variables_, @@ -329,7 +325,18 @@ GenerateMembers(io::Printer* printer) const { } void PrimitiveFieldGenerator:: -GenerateParsingCode(io::Printer* printer) const { +GenerateClearCode(io::Printer* printer) const { + printer->Print(variables_, + "$name$ = $default_copy_if_needed$;\n"); + + if (params_.generate_has()) { + printer->Print(variables_, + "has$capitalized_name$ = false;\n"); + } +} + +void PrimitiveFieldGenerator:: +GenerateMergingCode(io::Printer* printer) const { printer->Print(variables_, "this.$name$ = input.read$capitalized_type$();\n"); @@ -397,6 +404,76 @@ string PrimitiveFieldGenerator::GetBoxedType() const { // =================================================================== +AccessorPrimitiveFieldGenerator:: +AccessorPrimitiveFieldGenerator(const FieldDescriptor* descriptor, + const Params& params, int has_bit_index) + : FieldGenerator(params), descriptor_(descriptor) { + SetPrimitiveVariables(descriptor, params, &variables_); + SetBitOperationVariables("has", has_bit_index, &variables_); +} + +AccessorPrimitiveFieldGenerator::~AccessorPrimitiveFieldGenerator() {} + +void AccessorPrimitiveFieldGenerator:: +GenerateMembers(io::Printer* printer) const { + if (variables_.find("default_constant_value") != variables_.end()) { + printer->Print(variables_, + "private static final $type$ $default_constant$ = $default_constant_value$;\n"); + } + printer->Print(variables_, + "private $type$ $name$_ = $default_copy_if_needed$;\n" + "public $type$ get$capitalized_name$() {\n" + " return $name$_;\n" + "}\n" + "public void set$capitalized_name$($type$ value) {\n" + "$null_check$" + " $name$_ = value;\n" + " $set_has$;\n" + "}\n" + "public boolean has$capitalized_name$() {\n" + " return $get_has$;\n" + "}\n" + "public void clear$capitalized_name$() {\n" + " $name$_ = $default_copy_if_needed$;\n" + " $clear_has$;\n" + "}\n"); +} + +void AccessorPrimitiveFieldGenerator:: +GenerateClearCode(io::Printer* printer) const { + printer->Print(variables_, + "$name$_ = $default_copy_if_needed$;\n"); +} + +void AccessorPrimitiveFieldGenerator:: +GenerateMergingCode(io::Printer* printer) const { + printer->Print(variables_, + "set$capitalized_name$(input.read$capitalized_type$());\n"); +} + +void AccessorPrimitiveFieldGenerator:: +GenerateSerializationCode(io::Printer* printer) const { + printer->Print(variables_, + "if (has$capitalized_name$()) {\n" + " output.write$capitalized_type$($number$, $name$_);\n" + "}\n"); +} + +void AccessorPrimitiveFieldGenerator:: +GenerateSerializedSizeCode(io::Printer* printer) const { + printer->Print(variables_, + "if (has$capitalized_name$()) {\n" + " size += com.google.protobuf.nano.CodedOutputByteBufferNano\n" + " .compute$capitalized_type$Size($number$, $name$_);\n" + "}\n"); +} + +string AccessorPrimitiveFieldGenerator::GetBoxedType() const { + return BoxedPrimitiveTypeName(GetJavaType(descriptor_)); +} + +// =================================================================== + RepeatedPrimitiveFieldGenerator:: RepeatedPrimitiveFieldGenerator(const FieldDescriptor* descriptor, const Params& params) : FieldGenerator(params), descriptor_(descriptor) { @@ -412,7 +489,13 @@ GenerateMembers(io::Printer* printer) const { } void RepeatedPrimitiveFieldGenerator:: -GenerateParsingCode(io::Printer* printer) const { +GenerateClearCode(io::Printer* printer) const { + printer->Print(variables_, + "$name$ = $default$;\n"); +} + +void RepeatedPrimitiveFieldGenerator:: +GenerateMergingCode(io::Printer* printer) const { // First, figure out the length of the array, then parse. if (descriptor_->options().packed()) { printer->Print(variables_, diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.h b/src/google/protobuf/compiler/javanano/javanano_primitive_field.h index a56082d729..b42e2f3e47 100644 --- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.h +++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.h @@ -51,7 +51,8 @@ class PrimitiveFieldGenerator : public FieldGenerator { // implements FieldGenerator --------------------------------------- void GenerateMembers(io::Printer* printer) const; - void GenerateParsingCode(io::Printer* printer) const; + void GenerateClearCode(io::Printer* printer) const; + void GenerateMergingCode(io::Printer* printer) const; void GenerateSerializationCode(io::Printer* printer) const; void GenerateSerializedSizeCode(io::Printer* printer) const; @@ -66,6 +67,28 @@ class PrimitiveFieldGenerator : public FieldGenerator { GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(PrimitiveFieldGenerator); }; +class AccessorPrimitiveFieldGenerator : public FieldGenerator { + public: + explicit AccessorPrimitiveFieldGenerator( const FieldDescriptor* descriptor, + const Params ¶ms, int has_bit_index); + ~AccessorPrimitiveFieldGenerator(); + + // implements FieldGenerator --------------------------------------- + void GenerateMembers(io::Printer* printer) const; + void GenerateClearCode(io::Printer* printer) const; + void GenerateMergingCode(io::Printer* printer) const; + void GenerateSerializationCode(io::Printer* printer) const; + void GenerateSerializedSizeCode(io::Printer* printer) const; + + string GetBoxedType() const; + + private: + const FieldDescriptor* descriptor_; + map variables_; + + GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(AccessorPrimitiveFieldGenerator); +}; + class RepeatedPrimitiveFieldGenerator : public FieldGenerator { public: explicit RepeatedPrimitiveFieldGenerator(const FieldDescriptor* descriptor, const Params& params); @@ -73,7 +96,8 @@ class RepeatedPrimitiveFieldGenerator : public FieldGenerator { // implements FieldGenerator --------------------------------------- void GenerateMembers(io::Printer* printer) const; - void GenerateParsingCode(io::Printer* printer) const; + void GenerateClearCode(io::Printer* printer) const; + void GenerateMergingCode(io::Printer* printer) const; void GenerateSerializationCode(io::Printer* printer) const; void GenerateSerializedSizeCode(io::Printer* printer) const; diff --git a/src/google/protobuf/unittest_accessors_nano.proto b/src/google/protobuf/unittest_accessors_nano.proto new file mode 100644 index 0000000000..875af25cbc --- /dev/null +++ b/src/google/protobuf/unittest_accessors_nano.proto @@ -0,0 +1,116 @@ +// 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; + +option java_package = "com.google.protobuf.nano"; +option java_outer_classname = "NanoAccessorsOuterClass"; + +message TestNanoAccessors { + + message NestedMessage { + optional int32 bb = 1; + } + + enum NestedEnum { + FOO = 1; + BAR = 2; + BAZ = 3; + } + + // Singular + optional int32 optional_int32 = 1; + optional string optional_string = 14; + optional bytes optional_bytes = 15; + + optional NestedMessage optional_nested_message = 18; + + optional NestedEnum optional_nested_enum = 21; + + // Repeated + repeated int32 repeated_int32 = 31; + repeated string repeated_string = 44; + repeated bytes repeated_bytes = 45; + + repeated NestedMessage repeated_nested_message = 48; + + repeated NestedEnum repeated_nested_enum = 51; + + // Singular with defaults + optional int32 default_int32 = 61 [default = 41 ]; + optional string default_string = 74 [default = "hello"]; + optional bytes default_bytes = 75 [default = "world"]; + + optional float default_float_nan = 99 [default = nan]; + + optional NestedEnum default_nested_enum = 81 [default = BAR]; + + // Required + required int32 id = 86; + + // Add enough optional fields to make 2 bit fields in total + optional int32 filler100 = 100; + optional int32 filler101 = 101; + optional int32 filler102 = 102; + optional int32 filler103 = 103; + optional int32 filler104 = 104; + optional int32 filler105 = 105; + optional int32 filler106 = 106; + optional int32 filler107 = 107; + optional int32 filler108 = 108; + optional int32 filler109 = 109; + optional int32 filler110 = 110; + optional int32 filler111 = 111; + optional int32 filler112 = 112; + optional int32 filler113 = 113; + optional int32 filler114 = 114; + optional int32 filler115 = 115; + optional int32 filler116 = 116; + optional int32 filler117 = 117; + optional int32 filler118 = 118; + optional int32 filler119 = 119; + optional int32 filler120 = 120; + optional int32 filler121 = 121; + optional int32 filler122 = 122; + optional int32 filler123 = 123; + optional int32 filler124 = 124; + optional int32 filler125 = 125; + optional int32 filler126 = 126; + optional int32 filler127 = 127; + optional int32 filler128 = 128; + optional int32 filler129 = 129; + optional int32 filler130 = 130; + + optional int32 before_bit_field_check = 139; + optional int32 bit_field_check = 140; + optional int32 after_bit_field_check = 141; +}