From 0ca0154ae5800ae5e73d49f7ac4f8f2f95c7c49a Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Tue, 9 Jul 2019 11:53:58 -0400 Subject: [PATCH 01/27] Update to Guava 28 --- java/pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/pom.xml b/java/pom.xml index 46d00c4891..383ff5d0e6 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -93,12 +93,12 @@ com.google.guava guava - 26.0-android + 28.0-android com.google.guava guava-testlib - 26.0-jre + 28.0-jre test From da5740082ef7c39a5503e4d57ee73e11d3b43586 Mon Sep 17 00:00:00 2001 From: Tristan Pratt Date: Sat, 17 Aug 2019 19:12:00 -0400 Subject: [PATCH 02/27] c# feature(RepeatedField): Capacity property to resize the internal array This property is useful for efficiently initializing long RepeatedFields. I used System.Collections.Generic.List's design as a model. It should feel the most familiar to C# developers. --- .../Collections/RepeatedFieldTest.cs | 48 +++++++++++++++++++ .../Collections/RepeatedField.cs | 36 +++++++++++++- 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs b/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs index b3863a4965..527c7d404c 100644 --- a/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs +++ b/csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs @@ -33,6 +33,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.IO; using System.Linq; using System.Text; @@ -756,5 +757,52 @@ namespace Google.Protobuf.Collections Assert.True(list1.Contains(SampleNaNs.SignallingFlipped)); Assert.False(list2.Contains(SampleNaNs.SignallingFlipped)); } + + [Test] + public void Capacity_Increase() + { + // Unfortunately this case tests implementation details of RepeatedField. This is necessary + + var list = new RepeatedField() { 1, 2, 3 }; + + Assert.AreEqual(8, list.Capacity); + Assert.AreEqual(3, list.Count); + + list.Capacity = 10; // Set capacity to a larger value to trigger growth + Assert.AreEqual(10, list.Capacity, "Capacity increased"); + Assert.AreEqual(3, list.Count); + + CollectionAssert.AreEqual(new int[] {1, 2, 3}, list.ToArray(), "We didn't lose our data in the resize"); + } + + [Test] + public void Capacity_Decrease() + { + var list = new RepeatedField() { 1, 2, 3 }; + + Assert.AreEqual(8, list.Capacity); + Assert.DoesNotThrow(() => list.Capacity = 5, "Can decrease capacity if new capacity is greater than list.Count"); + Assert.AreEqual(5, list.Capacity); + + Assert.DoesNotThrow(() => list.Capacity = 3, "Can set capacity exactly to list.Count" ); + + Assert.Throws(() => list.Capacity = 2, "Can't set the capacity smaller than list.Count" ); + + Assert.Throws(() => list.Capacity = 0, "Can't set the capacity to zero" ); + + Assert.Throws(() => list.Capacity = -1, "Can't set the capacity to negative" ); + } + + [Test] + public void Capacity_Zero() + { + var list = new RepeatedField() { 1 }; + list.RemoveAt(0); + Assert.AreEqual(0, list.Count); + Assert.AreEqual(8, list.Capacity); + + Assert.DoesNotThrow(() => list.Capacity = 0, "Can set Capacity to 0"); + Assert.AreEqual(0, list.Capacity); + } } } diff --git a/csharp/src/Google.Protobuf/Collections/RepeatedField.cs b/csharp/src/Google.Protobuf/Collections/RepeatedField.cs index 827bc71183..9f239d91c7 100644 --- a/csharp/src/Google.Protobuf/Collections/RepeatedField.cs +++ b/csharp/src/Google.Protobuf/Collections/RepeatedField.cs @@ -216,14 +216,46 @@ namespace Google.Protobuf.Collections } } + /// + /// Gets and sets the capacity of the RepeatedField's internal array. WHen set, the internal array is reallocated to the given capacity. + /// The new value is less than Count -or- when Count is less than 0. + /// + public int Capacity + { + get { return array.Length; } + set + { + if (value < count) + { + throw new ArgumentOutOfRangeException("Capacity", value, + $"Cannot set Capacity to a value smaller than the current item count, {count}"); + } + + if (value >= 0 && value != array.Length) + { + SetSize(value); + } + } + } + + // May increase the size of the internal array, but will never shrink it. private void EnsureSize(int size) { if (array.Length < size) { size = Math.Max(size, MinArraySize); int newSize = Math.Max(array.Length * 2, size); - var tmp = new T[newSize]; - Array.Copy(array, 0, tmp, 0, array.Length); + SetSize(newSize); + } + } + + // Sets the internal array to an exact size. + private void SetSize(int size) + { + if (size != array.Length) + { + var tmp = new T[size]; + Array.Copy(array, 0, tmp, 0, count); array = tmp; } } From 6c92f9dff1807c142edf6780d775b58a3b078591 Mon Sep 17 00:00:00 2001 From: Rafi Kamal Date: Fri, 11 Oct 2019 12:05:46 -0700 Subject: [PATCH 03/27] Down integrate to GitHub --- conformance/binary_json_conformance_suite.cc | 570 +++++++++--------- conformance/binary_json_conformance_suite.h | 8 +- conformance/failure_list_js.txt | 114 +--- conformance/failure_list_python.txt | 11 - .../protobuf/ExtensionRegistryFactory.java | 38 +- .../protobuf/ExtensionRegistryLite.java | 24 +- .../google/protobuf/GeneratedMessageLite.java | 25 +- .../google/protobuf/GeneratedMessageV3.java | 291 +-------- .../com/google/protobuf/RopeByteString.java | 21 +- .../google/protobuf/GeneratedMessageTest.java | 34 +- .../protobuf/LiteralByteStringTest.java | 7 + python/google/protobuf/internal/decoder.py | 11 +- .../google/protobuf/internal/message_test.py | 12 + .../protobuf/internal/python_message.py | 2 + .../protobuf/internal/text_format_test.py | 85 ++- .../protobuf/internal/unknown_fields_test.py | 7 +- python/google/protobuf/pyext/descriptor.cc | 10 +- .../protobuf/pyext/descriptor_containers.cc | 76 +-- .../protobuf/pyext/descriptor_database.cc | 8 +- .../protobuf/pyext/descriptor_database.h | 9 +- .../google/protobuf/pyext/descriptor_pool.cc | 24 +- .../google/protobuf/pyext/extension_dict.cc | 6 +- python/google/protobuf/pyext/map_container.cc | 6 +- python/google/protobuf/pyext/message.cc | 37 +- python/google/protobuf/pyext/message.h | 2 +- .../pyext/repeated_scalar_container.cc | 4 +- python/google/protobuf/text_format.py | 49 +- src/google/protobuf/any.h | 4 +- src/google/protobuf/any.pb.cc | 12 - src/google/protobuf/any.pb.h | 24 +- src/google/protobuf/arena.h | 1 - .../compiler/command_line_interface.cc | 4 +- .../compiler/command_line_interface.h | 2 +- .../command_line_interface_unittest.cc | 68 +++ .../compiler/cpp/cpp_bootstrap_unittest.cc | 5 +- .../protobuf/compiler/cpp/cpp_helpers.cc | 31 +- .../protobuf/compiler/cpp/cpp_helpers.h | 14 +- .../protobuf/compiler/cpp/cpp_message.cc | 59 +- .../compiler/cpp/cpp_message_field.cc | 5 +- .../protobuf/compiler/cpp/metadata_test.cc | 2 +- .../compiler/csharp/csharp_field_base.cc | 4 +- .../compiler/csharp/csharp_helpers.cc | 18 +- .../compiler/csharp/csharp_message.cc | 69 ++- .../csharp/csharp_reflection_class.cc | 6 +- src/google/protobuf/compiler/importer.cc | 2 +- .../compiler/java/java_enum_field_lite.cc | 22 +- .../protobuf/compiler/java/java_file.cc | 1 + .../compiler/java/java_map_field_lite.cc | 10 +- .../protobuf/compiler/java/java_message.cc | 3 +- .../compiler/java/java_message_field_lite.cc | 90 +-- .../java/java_primitive_field_lite.cc | 7 +- .../compiler/java/java_string_field_lite.cc | 20 +- .../protobuf/compiler/js/js_generator.cc | 16 +- .../protobuf/compiler/mock_code_generator.cc | 2 +- src/google/protobuf/compiler/parser.cc | 23 +- src/google/protobuf/compiler/parser.h | 10 +- .../protobuf/compiler/parser_unittest.cc | 14 +- src/google/protobuf/descriptor.cc | 77 ++- src/google/protobuf/descriptor.h | 6 + src/google/protobuf/map_entry_lite.h | 3 +- src/google/protobuf/map_test.cc | 52 ++ src/google/protobuf/map_test_util.h | 7 + src/google/protobuf/message.h | 1 + src/google/protobuf/parse_context.h | 6 +- src/google/protobuf/port_def.inc | 4 + src/google/protobuf/port_undef.inc | 2 + .../protobuf/proto3_arena_lite_unittest.cc | 11 +- src/google/protobuf/proto3_lite_unittest.inc | 12 +- src/google/protobuf/repeated_field.cc | 1 + src/google/protobuf/unknown_field_set.h | 36 ++ .../protobuf/unknown_field_set_unittest.cc | 45 +- .../protobuf/util/internal/datapiece.cc | 2 +- .../util/internal/json_objectwriter_test.cc | 2 +- .../util/internal/protostream_objectsource.cc | 1 - .../internal/protostream_objectsource_test.cc | 2 + .../util/internal/protostream_objectwriter.cc | 10 + .../util/internal/protostream_objectwriter.h | 6 +- src/google/protobuf/util/time_util.h | 2 +- src/google/protobuf/wire_format.cc | 12 +- 79 files changed, 1087 insertions(+), 1252 deletions(-) diff --git a/conformance/binary_json_conformance_suite.cc b/conformance/binary_json_conformance_suite.cc index 07d8e91ad1..4f598ebe09 100644 --- a/conformance/binary_json_conformance_suite.cc +++ b/conformance/binary_json_conformance_suite.cc @@ -176,6 +176,8 @@ string GetDefaultValue(FieldDescriptor::Type type) { case FieldDescriptor::TYPE_BYTES: case FieldDescriptor::TYPE_MESSAGE: return delim(""); + default: + return ""; } return ""; } @@ -208,6 +210,8 @@ string GetNonDefaultValue(FieldDescriptor::Type type) { return delim("a"); case FieldDescriptor::TYPE_MESSAGE: return delim(cat(tag(1, WireFormatLite::WIRETYPE_VARINT), varint(1234))); + default: + return ""; } return ""; } @@ -252,12 +256,11 @@ const FieldDescriptor* GetFieldForType(FieldDescriptor::Type type, return nullptr; } -const FieldDescriptor* GetFieldForMapType( - FieldDescriptor::Type key_type, - FieldDescriptor::Type value_type, - bool is_proto3) { - const Descriptor* d = is_proto3 ? - TestAllTypesProto3().GetDescriptor() : TestAllTypesProto2().GetDescriptor(); +const FieldDescriptor* GetFieldForMapType(FieldDescriptor::Type key_type, + FieldDescriptor::Type value_type, + bool is_proto3) { + const Descriptor* d = is_proto3 ? TestAllTypesProto3().GetDescriptor() + : TestAllTypesProto2().GetDescriptor(); for (int i = 0; i < d->field_count(); i++) { const FieldDescriptor* f = d->field(i); if (f->is_map()) { @@ -272,18 +275,17 @@ const FieldDescriptor* GetFieldForMapType( const string proto_string = is_proto3 ? "Proto3" : "Proto2"; GOOGLE_LOG(FATAL) << "Couldn't find map field with type: " - << FieldDescriptor::TypeName(key_type) - << " and " - << FieldDescriptor::TypeName(key_type) - << " for " + << FieldDescriptor::TypeName(key_type) << " and " + << FieldDescriptor::TypeName(key_type) << " for " << proto_string.c_str(); return nullptr; } -const FieldDescriptor* GetFieldForOneofType( - FieldDescriptor::Type type, bool is_proto3, bool exclusive = false) { - const Descriptor* d = is_proto3 ? - TestAllTypesProto3().GetDescriptor() : TestAllTypesProto2().GetDescriptor(); +const FieldDescriptor* GetFieldForOneofType(FieldDescriptor::Type type, + bool is_proto3, + bool exclusive = false) { + const Descriptor* d = is_proto3 ? TestAllTypesProto3().GetDescriptor() + : TestAllTypesProto2().GetDescriptor(); for (int i = 0; i < d->field_count(); i++) { const FieldDescriptor* f = d->field(i); if (f->containing_oneof() && ((f->type() == type) ^ exclusive)) { @@ -293,8 +295,7 @@ const FieldDescriptor* GetFieldForOneofType( const string proto_string = is_proto3 ? "Proto3" : "Proto2"; GOOGLE_LOG(FATAL) << "Couldn't find oneof field with type: " - << FieldDescriptor::TypeName(type) - << " for " + << FieldDescriptor::TypeName(type) << " for " << proto_string.c_str(); return nullptr; } @@ -764,9 +765,9 @@ void BinaryAndJsonConformanceSuite::TestValidDataForType( string proto = cat(tag(field->number(), wire_type), values[i].first); // In proto3, default primitive fields should not be encoded. string expected_proto = - is_proto3 && IsProto3Default(field->type(), values[i].second) ? - "" : - cat(tag(field->number(), wire_type), values[i].second); + is_proto3 && IsProto3Default(field->type(), values[i].second) + ? "" + : cat(tag(field->number(), wire_type), values[i].second); std::unique_ptr test_message = NewTestMessage(is_proto3); test_message->MergeFromString(expected_proto); string text = test_message->DebugString(); @@ -774,10 +775,8 @@ void BinaryAndJsonConformanceSuite::TestValidDataForType( RunValidProtobufTest(StrCat("ValidDataScalar", type_name, "[", i, "]"), REQUIRED, proto, text, is_proto3); RunValidBinaryProtobufTest( - StrCat("ValidDataScalarBinary", type_name, "[", i, "]"), - RECOMMENDED, - proto, - expected_proto, is_proto3); + StrCat("ValidDataScalarBinary", type_name, "[", i, "]"), RECOMMENDED, + proto, expected_proto, is_proto3); } // Test repeated data for singular fields. @@ -947,25 +946,21 @@ void BinaryAndJsonConformanceSuite::TestValidDataForRepeatedScalarMessage() { } void BinaryAndJsonConformanceSuite::TestValidDataForMapType( - FieldDescriptor::Type key_type, - FieldDescriptor::Type value_type) { + FieldDescriptor::Type key_type, FieldDescriptor::Type value_type) { const string key_type_name = UpperCase(string(".") + FieldDescriptor::TypeName(key_type)); const string value_type_name = UpperCase(string(".") + FieldDescriptor::TypeName(value_type)); - WireFormatLite::WireType key_wire_type = - WireFormatLite::WireTypeForFieldType( - static_cast(key_type)); + WireFormatLite::WireType key_wire_type = WireFormatLite::WireTypeForFieldType( + static_cast(key_type)); WireFormatLite::WireType value_wire_type = WireFormatLite::WireTypeForFieldType( static_cast(value_type)); - string key1_data = - cat(tag(1, key_wire_type), GetDefaultValue(key_type)); + string key1_data = cat(tag(1, key_wire_type), GetDefaultValue(key_type)); string value1_data = cat(tag(2, value_wire_type), GetDefaultValue(value_type)); - string key2_data = - cat(tag(1, key_wire_type), GetNonDefaultValue(key_type)); + string key2_data = cat(tag(1, key_wire_type), GetNonDefaultValue(key_type)); string value2_data = cat(tag(2, value_wire_type), GetNonDefaultValue(value_type)); @@ -975,118 +970,97 @@ void BinaryAndJsonConformanceSuite::TestValidDataForMapType( { // Tests map with default key and value. - string proto = cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(cat(key1_data, value1_data))); + string proto = + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(cat(key1_data, value1_data))); std::unique_ptr test_message = NewTestMessage(is_proto3); test_message->MergeFromString(proto); string text = test_message->DebugString(); RunValidProtobufTest( - StrCat("ValidDataMap", - key_type_name, - value_type_name, - ".Default"), + StrCat("ValidDataMap", key_type_name, value_type_name, ".Default"), REQUIRED, proto, text, is_proto3); } { // Tests map with missing default key and value. - string proto = cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim("")); + string proto = + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim("")); std::unique_ptr test_message = NewTestMessage(is_proto3); test_message->MergeFromString(proto); string text = test_message->DebugString(); - RunValidProtobufTest( - StrCat("ValidDataMap", - key_type_name, - value_type_name, - ".MissingDefault"), - REQUIRED, proto, text, is_proto3); + RunValidProtobufTest(StrCat("ValidDataMap", key_type_name, + value_type_name, ".MissingDefault"), + REQUIRED, proto, text, is_proto3); } { // Tests map with non-default key and value. - string proto = cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(cat(key2_data, value2_data))); + string proto = + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(cat(key2_data, value2_data))); std::unique_ptr test_message = NewTestMessage(is_proto3); test_message->MergeFromString(proto); string text = test_message->DebugString(); RunValidProtobufTest( - StrCat("ValidDataMap", - key_type_name, - value_type_name, - ".NonDefault"), + StrCat("ValidDataMap", key_type_name, value_type_name, ".NonDefault"), REQUIRED, proto, text, is_proto3); } { // Tests map with unordered key and value. - string proto = cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(cat(value2_data, key2_data))); + string proto = + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(cat(value2_data, key2_data))); std::unique_ptr test_message = NewTestMessage(is_proto3); test_message->MergeFromString(proto); string text = test_message->DebugString(); RunValidProtobufTest( - StrCat("ValidDataMap", - key_type_name, - value_type_name, - ".Unordered"), + StrCat("ValidDataMap", key_type_name, value_type_name, ".Unordered"), REQUIRED, proto, text, is_proto3); } { // Tests map with duplicate key. - string proto1 = cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(cat(key2_data, value1_data))); - string proto2 = cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(cat(key2_data, value2_data))); + string proto1 = + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(cat(key2_data, value1_data))); + string proto2 = + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(cat(key2_data, value2_data))); string proto = cat(proto1, proto2); std::unique_ptr test_message = NewTestMessage(is_proto3); test_message->MergeFromString(proto2); string text = test_message->DebugString(); - RunValidProtobufTest( - StrCat("ValidDataMap", - key_type_name, - value_type_name, - ".DuplicateKey"), - REQUIRED, proto, text, is_proto3); + RunValidProtobufTest(StrCat("ValidDataMap", key_type_name, + value_type_name, ".DuplicateKey"), + REQUIRED, proto, text, is_proto3); } { // Tests map with duplicate key in map entry. - string proto = cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(cat(key1_data, key2_data, value2_data))); + string proto = + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(cat(key1_data, key2_data, value2_data))); std::unique_ptr test_message = NewTestMessage(is_proto3); test_message->MergeFromString(proto); string text = test_message->DebugString(); - RunValidProtobufTest( - StrCat("ValidDataMap", - key_type_name, - value_type_name, - ".DuplicateKeyInMapEntry"), - REQUIRED, proto, text, is_proto3); + RunValidProtobufTest(StrCat("ValidDataMap", key_type_name, + value_type_name, ".DuplicateKeyInMapEntry"), + REQUIRED, proto, text, is_proto3); } { // Tests map with duplicate value in map entry. - string proto = cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(cat(key2_data, value1_data, value2_data))); + string proto = + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(cat(key2_data, value1_data, value2_data))); std::unique_ptr test_message = NewTestMessage(is_proto3); test_message->MergeFromString(proto); string text = test_message->DebugString(); - RunValidProtobufTest( - StrCat("ValidDataMap", - key_type_name, - value_type_name, - ".DuplicateValueInMapEntry"), - REQUIRED, proto, text, is_proto3); + RunValidProtobufTest(StrCat("ValidDataMap", key_type_name, + value_type_name, ".DuplicateValueInMapEntry"), + REQUIRED, proto, text, is_proto3); } } } @@ -1096,7 +1070,8 @@ void BinaryAndJsonConformanceSuite::TestOverwriteMessageValueMap() { cat(tag(1, WireFormatLite::WIRETYPE_LENGTH_DELIMITED), delim("")); string field1_data = cat(tag(1, WireFormatLite::WIRETYPE_VARINT), varint(1)); string field2_data = cat(tag(2, WireFormatLite::WIRETYPE_VARINT), varint(1)); - string field31_data = cat(tag(31, WireFormatLite::WIRETYPE_VARINT), varint(1)); + string field31_data = + cat(tag(31, WireFormatLite::WIRETYPE_VARINT), varint(1)); string submsg1_data = delim(cat(field1_data, field31_data)); string submsg2_data = delim(cat(field2_data, field31_data)); string value1_data = @@ -1109,24 +1084,21 @@ void BinaryAndJsonConformanceSuite::TestOverwriteMessageValueMap() { submsg2_data))); for (int is_proto3 = 0; is_proto3 < 2; is_proto3++) { - const FieldDescriptor* field = - GetFieldForMapType( - FieldDescriptor::TYPE_STRING, - FieldDescriptor::TYPE_MESSAGE, is_proto3); - - string proto1 = cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(cat(key_data, value1_data))); - string proto2 = cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(cat(key_data, value2_data))); + const FieldDescriptor* field = GetFieldForMapType( + FieldDescriptor::TYPE_STRING, FieldDescriptor::TYPE_MESSAGE, is_proto3); + + string proto1 = + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(cat(key_data, value1_data))); + string proto2 = + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(cat(key_data, value2_data))); string proto = cat(proto1, proto2); std::unique_ptr test_message = NewTestMessage(is_proto3); test_message->MergeFromString(proto2); string text = test_message->DebugString(); - RunValidProtobufTest( - "ValidDataMap.STRING.MESSAGE.MergeValue", - REQUIRED, proto, text, is_proto3); + RunValidProtobufTest("ValidDataMap.STRING.MESSAGE.MergeValue", REQUIRED, + proto, text, is_proto3); } } @@ -1134,9 +1106,8 @@ void BinaryAndJsonConformanceSuite::TestValidDataForOneofType( FieldDescriptor::Type type) { const string type_name = UpperCase(string(".") + FieldDescriptor::TypeName(type)); - WireFormatLite::WireType wire_type = - WireFormatLite::WireTypeForFieldType( - static_cast(type)); + WireFormatLite::WireType wire_type = WireFormatLite::WireTypeForFieldType( + static_cast(type)); for (int is_proto3 = 0; is_proto3 < 2; is_proto3++) { const FieldDescriptor* field = GetFieldForOneofType(type, is_proto3); @@ -1167,8 +1138,8 @@ void BinaryAndJsonConformanceSuite::TestValidDataForOneofType( string text = test_message->DebugString(); RunValidProtobufTest( - StrCat("ValidDataOneof", type_name, ".NonDefaultValue"), - REQUIRED, proto, text, is_proto3); + StrCat("ValidDataOneof", type_name, ".NonDefaultValue"), REQUIRED, + proto, text, is_proto3); RunValidBinaryProtobufTest( StrCat("ValidDataOneofBinary", type_name, ".NonDefaultValue"), RECOMMENDED, proto, proto, is_proto3); @@ -1185,10 +1156,9 @@ void BinaryAndJsonConformanceSuite::TestValidDataForOneofType( RunValidProtobufTest( StrCat("ValidDataOneof", type_name, ".MultipleValuesForSameField"), REQUIRED, proto, text, is_proto3); - RunValidBinaryProtobufTest( - StrCat("ValidDataOneofBinary", type_name, - ".MultipleValuesForSameField"), - RECOMMENDED, proto, expected_proto, is_proto3); + RunValidBinaryProtobufTest(StrCat("ValidDataOneofBinary", type_name, + ".MultipleValuesForSameField"), + RECOMMENDED, proto, expected_proto, is_proto3); } { @@ -1209,14 +1179,12 @@ void BinaryAndJsonConformanceSuite::TestValidDataForOneofType( test_message->MergeFromString(expected_proto); string text = test_message->DebugString(); - RunValidProtobufTest( - StrCat("ValidDataOneof", type_name, - ".MultipleValuesForDifferentField"), - REQUIRED, proto, text, is_proto3); - RunValidBinaryProtobufTest( - StrCat("ValidDataOneofBinary", type_name, - ".MultipleValuesForDifferentField"), - RECOMMENDED, proto, expected_proto, is_proto3); + RunValidProtobufTest(StrCat("ValidDataOneof", type_name, + ".MultipleValuesForDifferentField"), + REQUIRED, proto, text, is_proto3); + RunValidBinaryProtobufTest(StrCat("ValidDataOneofBinary", type_name, + ".MultipleValuesForDifferentField"), + RECOMMENDED, proto, expected_proto, is_proto3); } } } @@ -1225,43 +1193,39 @@ void BinaryAndJsonConformanceSuite::TestMergeOneofMessage() { string field1_data = cat(tag(1, WireFormatLite::WIRETYPE_VARINT), varint(1)); string field2a_data = cat(tag(2, WireFormatLite::WIRETYPE_VARINT), varint(1)); string field2b_data = cat(tag(2, WireFormatLite::WIRETYPE_VARINT), varint(1)); - string field89_data = cat(tag(89, WireFormatLite::WIRETYPE_VARINT), - varint(1)); + string field89_data = + cat(tag(89, WireFormatLite::WIRETYPE_VARINT), varint(1)); string submsg1_data = cat(tag(2, WireFormatLite::WIRETYPE_LENGTH_DELIMITED), delim(cat(field1_data, field2a_data, field89_data))); - string submsg2_data = + string submsg2_data = cat(tag(2, WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(cat(field2b_data, field89_data))); + string merged_data = cat(tag(2, WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(cat(field2b_data, field89_data))); - string merged_data = cat(tag(2, WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(cat(field1_data, field2b_data, - field89_data, field89_data))); + delim(cat(field1_data, field2b_data, field89_data, field89_data))); for (int is_proto3 = 0; is_proto3 < 2; is_proto3++) { const FieldDescriptor* field = GetFieldForOneofType(FieldDescriptor::TYPE_MESSAGE, is_proto3); - string proto1 = cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(submsg1_data)); - string proto2 = cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(submsg2_data)); + string proto1 = + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(submsg1_data)); + string proto2 = + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(submsg2_data)); string proto = cat(proto1, proto2); string expected_proto = - cat(tag(field->number(), - WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - delim(merged_data)); + cat(tag(field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED), + delim(merged_data)); std::unique_ptr test_message = NewTestMessage(is_proto3); test_message->MergeFromString(expected_proto); string text = test_message->DebugString(); - RunValidProtobufTest( - "ValidDataOneof.MESSAGE.Merge", - REQUIRED, proto, text, is_proto3); - RunValidBinaryProtobufTest( - "ValidDataOneofBinary.MESSAGE.Merge", - RECOMMENDED, proto, expected_proto, is_proto3); + RunValidProtobufTest("ValidDataOneof.MESSAGE.Merge", REQUIRED, proto, text, + is_proto3); + RunValidBinaryProtobufTest("ValidDataOneofBinary.MESSAGE.Merge", + RECOMMENDED, proto, expected_proto, is_proto3); } } @@ -1359,103 +1323,124 @@ void BinaryAndJsonConformanceSuite::RunSuiteImpl() { int32 kInt32Min = -2147483648; uint32 kUint32Max = 4294967295UL; - TestValidDataForType(FieldDescriptor::TYPE_DOUBLE, { - {dbl(0), dbl(0)}, - {dbl(0.1), dbl(0.1)}, - {dbl(1.7976931348623157e+308), dbl(1.7976931348623157e+308)}, - {dbl(2.22507385850720138309e-308), dbl(2.22507385850720138309e-308)}, - }); - TestValidDataForType(FieldDescriptor::TYPE_FLOAT, { - {flt(0), flt(0)}, - {flt(0.1), flt(0.1)}, - {flt(1.00000075e-36), flt(1.00000075e-36)}, - {flt(3.402823e+38), flt(3.402823e+38)}, // 3.40282347e+38 - {flt(1.17549435e-38f), flt(1.17549435e-38)}, - }); - TestValidDataForType(FieldDescriptor::TYPE_INT64, { - {varint(0), varint(0)}, - {varint(12345), varint(12345)}, - {varint(kInt64Max), varint(kInt64Max)}, - {varint(kInt64Min), varint(kInt64Min)}, - }); - TestValidDataForType(FieldDescriptor::TYPE_UINT64, { - {varint(0), varint(0)}, - {varint(12345), varint(12345)}, - {varint(kUint64Max), varint(kUint64Max)}, - }); - TestValidDataForType(FieldDescriptor::TYPE_INT32, { - {varint(0), varint(0)}, - {varint(12345), varint(12345)}, - {longvarint(12345, 2), varint(12345)}, - {longvarint(12345, 7), varint(12345)}, - {varint(kInt32Max), varint(kInt32Max)}, - {varint(kInt32Min), varint(kInt32Min)}, - {varint(1LL << 33), varint(0)}, - {varint((1LL << 33) - 1), varint(-1)}, - }); - TestValidDataForType(FieldDescriptor::TYPE_UINT32, { - {varint(0), varint(0)}, - {varint(12345), varint(12345)}, - {longvarint(12345, 2), varint(12345)}, - {longvarint(12345, 7), varint(12345)}, - {varint(kUint32Max), varint(kUint32Max)}, // UINT32_MAX - {varint(1LL << 33), varint(0)}, - {varint((1LL << 33) - 1), varint((1LL << 32) - 1)}, - }); - TestValidDataForType(FieldDescriptor::TYPE_FIXED64, { - {u64(0), u64(0)}, - {u64(12345), u64(12345)}, - {u64(kUint64Max), u64(kUint64Max)}, - }); - TestValidDataForType(FieldDescriptor::TYPE_FIXED32, { - {u32(0), u32(0)}, - {u32(12345), u32(12345)}, - {u32(kUint32Max), u32(kUint32Max)}, // UINT32_MAX - }); - TestValidDataForType(FieldDescriptor::TYPE_SFIXED64, { - {u64(0), u64(0)}, - {u64(12345), u64(12345)}, - {u64(kInt64Max), u64(kInt64Max)}, - {u64(kInt64Min), u64(kInt64Min)}, - }); - TestValidDataForType(FieldDescriptor::TYPE_SFIXED32, { - {u32(0), u32(0)}, - {u32(12345), u32(12345)}, - {u32(kInt32Max), u32(kInt32Max)}, - {u32(kInt32Min), u32(kInt32Min)}, - }); - TestValidDataForType(FieldDescriptor::TYPE_BOOL, { - {varint(0), varint(0)}, - {varint(1), varint(1)}, - {varint(12345678), varint(1)}, - }); - TestValidDataForType(FieldDescriptor::TYPE_SINT32, { - {zz32(0), zz32(0)}, - {zz32(12345), zz32(12345)}, - {zz32(kInt32Max), zz32(kInt32Max)}, - {zz32(kInt32Min), zz32(kInt32Min)}, - }); - TestValidDataForType(FieldDescriptor::TYPE_SINT64, { - {zz64(0), zz64(0)}, - {zz64(12345), zz64(12345)}, - {zz64(kInt64Max), zz64(kInt64Max)}, - {zz64(kInt64Min), zz64(kInt64Min)}, - }); - TestValidDataForType(FieldDescriptor::TYPE_STRING, { - {delim(""), delim("")}, - {delim("Hello world!"), delim("Hello world!")}, - {delim("\'\"\?\\\a\b\f\n\r\t\v"), - delim("\'\"\?\\\a\b\f\n\r\t\v")}, // escape - {delim("谷歌"), delim("谷歌")}, // Google in Chinese - {delim("\u8C37\u6B4C"), delim("谷歌")}, // unicode escape - {delim("\u8c37\u6b4c"), delim("谷歌")}, // lowercase unicode - {delim("\xF0\x9F\x98\x81"), delim("\xF0\x9F\x98\x81")}, // emoji: 😁 - }); - TestValidDataForType(FieldDescriptor::TYPE_BYTES, { - {delim(""), delim("")}, - {delim("\x01\x02"), delim("\x01\x02")}, - {delim("\xfb"), delim("\xfb")}, - }); + TestValidDataForType( + FieldDescriptor::TYPE_DOUBLE, + { + {dbl(0), dbl(0)}, + {dbl(0.1), dbl(0.1)}, + {dbl(1.7976931348623157e+308), dbl(1.7976931348623157e+308)}, + {dbl(2.22507385850720138309e-308), dbl(2.22507385850720138309e-308)}, + }); + TestValidDataForType( + FieldDescriptor::TYPE_FLOAT, + { + {flt(0), flt(0)}, + {flt(0.1), flt(0.1)}, + {flt(1.00000075e-36), flt(1.00000075e-36)}, + {flt(3.402823e+38), flt(3.402823e+38)}, // 3.40282347e+38 + {flt(1.17549435e-38f), flt(1.17549435e-38)}, + }); + TestValidDataForType(FieldDescriptor::TYPE_INT64, + { + {varint(0), varint(0)}, + {varint(12345), varint(12345)}, + {varint(kInt64Max), varint(kInt64Max)}, + {varint(kInt64Min), varint(kInt64Min)}, + }); + TestValidDataForType(FieldDescriptor::TYPE_UINT64, + { + {varint(0), varint(0)}, + {varint(12345), varint(12345)}, + {varint(kUint64Max), varint(kUint64Max)}, + }); + TestValidDataForType(FieldDescriptor::TYPE_INT32, + { + {varint(0), varint(0)}, + {varint(12345), varint(12345)}, + {longvarint(12345, 2), varint(12345)}, + {longvarint(12345, 7), varint(12345)}, + {varint(kInt32Max), varint(kInt32Max)}, + {varint(kInt32Min), varint(kInt32Min)}, + {varint(1LL << 33), varint(0)}, + {varint((1LL << 33) - 1), varint(-1)}, + }); + TestValidDataForType( + FieldDescriptor::TYPE_UINT32, + { + {varint(0), varint(0)}, + {varint(12345), varint(12345)}, + {longvarint(12345, 2), varint(12345)}, + {longvarint(12345, 7), varint(12345)}, + {varint(kUint32Max), varint(kUint32Max)}, // UINT32_MAX + {varint(1LL << 33), varint(0)}, + {varint((1LL << 33) - 1), varint((1LL << 32) - 1)}, + }); + TestValidDataForType(FieldDescriptor::TYPE_FIXED64, + { + {u64(0), u64(0)}, + {u64(12345), u64(12345)}, + {u64(kUint64Max), u64(kUint64Max)}, + }); + TestValidDataForType(FieldDescriptor::TYPE_FIXED32, + { + {u32(0), u32(0)}, + {u32(12345), u32(12345)}, + {u32(kUint32Max), u32(kUint32Max)}, // UINT32_MAX + }); + TestValidDataForType(FieldDescriptor::TYPE_SFIXED64, + { + {u64(0), u64(0)}, + {u64(12345), u64(12345)}, + {u64(kInt64Max), u64(kInt64Max)}, + {u64(kInt64Min), u64(kInt64Min)}, + }); + TestValidDataForType(FieldDescriptor::TYPE_SFIXED32, + { + {u32(0), u32(0)}, + {u32(12345), u32(12345)}, + {u32(kInt32Max), u32(kInt32Max)}, + {u32(kInt32Min), u32(kInt32Min)}, + }); + // Bools should be serialized as 0 for false and 1 for true. Parsers should + // also interpret any nonzero value as true. + TestValidDataForType(FieldDescriptor::TYPE_BOOL, + { + {varint(0), varint(0)}, + {varint(1), varint(1)}, + {varint(12345678), varint(1)}, + }); + TestValidDataForType(FieldDescriptor::TYPE_SINT32, + { + {zz32(0), zz32(0)}, + {zz32(12345), zz32(12345)}, + {zz32(kInt32Max), zz32(kInt32Max)}, + {zz32(kInt32Min), zz32(kInt32Min)}, + }); + TestValidDataForType(FieldDescriptor::TYPE_SINT64, + { + {zz64(0), zz64(0)}, + {zz64(12345), zz64(12345)}, + {zz64(kInt64Max), zz64(kInt64Max)}, + {zz64(kInt64Min), zz64(kInt64Min)}, + }); + TestValidDataForType( + FieldDescriptor::TYPE_STRING, + { + {delim(""), delim("")}, + {delim("Hello world!"), delim("Hello world!")}, + {delim("\'\"\?\\\a\b\f\n\r\t\v"), + delim("\'\"\?\\\a\b\f\n\r\t\v")}, // escape + {delim("谷歌"), delim("谷歌")}, // Google in Chinese + {delim("\u8C37\u6B4C"), delim("谷歌")}, // unicode escape + {delim("\u8c37\u6b4c"), delim("谷歌")}, // lowercase unicode + {delim("\xF0\x9F\x98\x81"), delim("\xF0\x9F\x98\x81")}, // emoji: 😁 + }); + TestValidDataForType(FieldDescriptor::TYPE_BYTES, + { + {delim(""), delim("")}, + {delim("\x01\x02"), delim("\x01\x02")}, + {delim("\xfb"), delim("\xfb")}, + }); TestValidDataForType(FieldDescriptor::TYPE_ENUM, { {varint(0), varint(0)}, {varint(1), varint(1)}, @@ -1463,63 +1448,48 @@ void BinaryAndJsonConformanceSuite::RunSuiteImpl() { {varint(-1), varint(-1)}, }); TestValidDataForRepeatedScalarMessage(); - TestValidDataForType(FieldDescriptor::TYPE_MESSAGE, { - {delim(""), delim("")}, - {delim(cat(tag(1, WireFormatLite::WIRETYPE_VARINT), varint(1234))), - delim(cat(tag(1, WireFormatLite::WIRETYPE_VARINT), varint(1234)))}, - }); - - TestValidDataForMapType( - FieldDescriptor::TYPE_INT32, - FieldDescriptor::TYPE_INT32); - TestValidDataForMapType( - FieldDescriptor::TYPE_INT64, - FieldDescriptor::TYPE_INT64); - TestValidDataForMapType( - FieldDescriptor::TYPE_UINT32, - FieldDescriptor::TYPE_UINT32); - TestValidDataForMapType( - FieldDescriptor::TYPE_UINT64, - FieldDescriptor::TYPE_UINT64); - TestValidDataForMapType( - FieldDescriptor::TYPE_SINT32, - FieldDescriptor::TYPE_SINT32); - TestValidDataForMapType( - FieldDescriptor::TYPE_SINT64, - FieldDescriptor::TYPE_SINT64); - TestValidDataForMapType( - FieldDescriptor::TYPE_FIXED32, - FieldDescriptor::TYPE_FIXED32); - TestValidDataForMapType( - FieldDescriptor::TYPE_FIXED64, - FieldDescriptor::TYPE_FIXED64); - TestValidDataForMapType( - FieldDescriptor::TYPE_SFIXED32, - FieldDescriptor::TYPE_SFIXED32); - TestValidDataForMapType( - FieldDescriptor::TYPE_SFIXED64, - FieldDescriptor::TYPE_SFIXED64); - TestValidDataForMapType( - FieldDescriptor::TYPE_INT32, - FieldDescriptor::TYPE_FLOAT); - TestValidDataForMapType( - FieldDescriptor::TYPE_INT32, - FieldDescriptor::TYPE_DOUBLE); - TestValidDataForMapType( - FieldDescriptor::TYPE_BOOL, - FieldDescriptor::TYPE_BOOL); - TestValidDataForMapType( - FieldDescriptor::TYPE_STRING, - FieldDescriptor::TYPE_STRING); - TestValidDataForMapType( - FieldDescriptor::TYPE_STRING, - FieldDescriptor::TYPE_BYTES); - TestValidDataForMapType( - FieldDescriptor::TYPE_STRING, - FieldDescriptor::TYPE_ENUM); - TestValidDataForMapType( - FieldDescriptor::TYPE_STRING, - FieldDescriptor::TYPE_MESSAGE); + TestValidDataForType( + FieldDescriptor::TYPE_MESSAGE, + { + {delim(""), delim("")}, + {delim(cat(tag(1, WireFormatLite::WIRETYPE_VARINT), varint(1234))), + delim(cat(tag(1, WireFormatLite::WIRETYPE_VARINT), varint(1234)))}, + }); + + TestValidDataForMapType(FieldDescriptor::TYPE_INT32, + FieldDescriptor::TYPE_INT32); + TestValidDataForMapType(FieldDescriptor::TYPE_INT64, + FieldDescriptor::TYPE_INT64); + TestValidDataForMapType(FieldDescriptor::TYPE_UINT32, + FieldDescriptor::TYPE_UINT32); + TestValidDataForMapType(FieldDescriptor::TYPE_UINT64, + FieldDescriptor::TYPE_UINT64); + TestValidDataForMapType(FieldDescriptor::TYPE_SINT32, + FieldDescriptor::TYPE_SINT32); + TestValidDataForMapType(FieldDescriptor::TYPE_SINT64, + FieldDescriptor::TYPE_SINT64); + TestValidDataForMapType(FieldDescriptor::TYPE_FIXED32, + FieldDescriptor::TYPE_FIXED32); + TestValidDataForMapType(FieldDescriptor::TYPE_FIXED64, + FieldDescriptor::TYPE_FIXED64); + TestValidDataForMapType(FieldDescriptor::TYPE_SFIXED32, + FieldDescriptor::TYPE_SFIXED32); + TestValidDataForMapType(FieldDescriptor::TYPE_SFIXED64, + FieldDescriptor::TYPE_SFIXED64); + TestValidDataForMapType(FieldDescriptor::TYPE_INT32, + FieldDescriptor::TYPE_FLOAT); + TestValidDataForMapType(FieldDescriptor::TYPE_INT32, + FieldDescriptor::TYPE_DOUBLE); + TestValidDataForMapType(FieldDescriptor::TYPE_BOOL, + FieldDescriptor::TYPE_BOOL); + TestValidDataForMapType(FieldDescriptor::TYPE_STRING, + FieldDescriptor::TYPE_STRING); + TestValidDataForMapType(FieldDescriptor::TYPE_STRING, + FieldDescriptor::TYPE_BYTES); + TestValidDataForMapType(FieldDescriptor::TYPE_STRING, + FieldDescriptor::TYPE_ENUM); + TestValidDataForMapType(FieldDescriptor::TYPE_STRING, + FieldDescriptor::TYPE_MESSAGE); // Additional test to check overwriting message value map. TestOverwriteMessageValueMap(); diff --git a/conformance/binary_json_conformance_suite.h b/conformance/binary_json_conformance_suite.h index 1720c1dd15..08182b6f58 100644 --- a/conformance/binary_json_conformance_suite.h +++ b/conformance/binary_json_conformance_suite.h @@ -115,11 +115,9 @@ class BinaryAndJsonConformanceSuite : public ConformanceTestSuite { google::protobuf::FieldDescriptor::Type, std::vector> values); void TestValidDataForRepeatedScalarMessage(); - void TestValidDataForMapType( - google::protobuf::FieldDescriptor::Type, - google::protobuf::FieldDescriptor::Type); - void TestValidDataForOneofType( - google::protobuf::FieldDescriptor::Type); + void TestValidDataForMapType(google::protobuf::FieldDescriptor::Type, + google::protobuf::FieldDescriptor::Type); + void TestValidDataForOneofType(google::protobuf::FieldDescriptor::Type); void TestMergeOneofMessage(); void TestOverwriteMessageValueMap(); diff --git a/conformance/failure_list_js.txt b/conformance/failure_list_js.txt index 4cb0a052c5..dd123dd6fe 100644 --- a/conformance/failure_list_js.txt +++ b/conformance/failure_list_js.txt @@ -1,84 +1,16 @@ -Recommended.Proto2.ProtobufInput.ValidDataRepeated.BOOL.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.BOOL.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.BOOL.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.DOUBLE.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.DOUBLE.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.DOUBLE.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.ENUM.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.ENUM.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.ENUM.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.FIXED32.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.FIXED32.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.FIXED32.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.FIXED64.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.FIXED64.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.FIXED64.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.FLOAT.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.FLOAT.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.FLOAT.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.INT32.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.INT32.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.INT32.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.INT64.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.INT64.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.INT64.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.SFIXED32.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.SFIXED32.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.SFIXED32.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.SFIXED64.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.SFIXED64.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.SFIXED64.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.SINT32.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.SINT32.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.SINT32.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.SINT64.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.SINT64.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.SINT64.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.UINT32.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.UINT32.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.UINT32.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.UINT64.PackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.UINT64.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto2.ProtobufInput.ValidDataRepeated.UINT64.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.BOOL.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.BOOL.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.BOOL.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.DOUBLE.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.DOUBLE.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.DOUBLE.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.ENUM.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.ENUM.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.ENUM.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.FIXED32.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.FIXED32.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.FIXED32.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.FIXED64.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.FIXED64.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.FIXED64.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.INT32.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.INT32.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.INT32.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.INT64.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.INT64.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.INT64.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.SFIXED32.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.SFIXED32.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.SFIXED32.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.SFIXED64.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.SFIXED64.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.SFIXED64.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.SINT32.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.SINT32.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.SINT32.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.SINT64.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.SINT64.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.SINT64.UnpackedInput.PackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT32.PackedInput.UnpackedOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT32.UnpackedInput.DefaultOutput.ProtobufOutput -Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT32.UnpackedInput.PackedOutput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.BOOL.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.DOUBLE.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.FIXED32.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.FIXED64.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.INT32.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.INT64.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.SFIXED32.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.SFIXED64.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.SINT32.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.SINT64.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.UINT32.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.UINT64.ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.PackedInput.UnpackedOutput.ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.UnpackedInput.DefaultOutput.ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.UnpackedInput.PackedOutput.ProtobufOutput @@ -87,28 +19,8 @@ Required.Proto3.ProtobufInput.ValidDataMap.STRING.MESSAGE.MergeValue.ProtobufOut Required.Proto2.ProtobufInput.ValidDataRepeated.BOOL.PackedInput.ProtobufOutput Required.Proto2.ProtobufInput.ValidDataRepeated.DOUBLE.PackedInput.ProtobufOutput Required.Proto2.ProtobufInput.ValidDataRepeated.ENUM.PackedInput.ProtobufOutput -Required.Proto2.ProtobufInput.ValidDataRepeated.FIXED32.PackedInput.ProtobufOutput -Required.Proto2.ProtobufInput.ValidDataRepeated.FIXED64.PackedInput.ProtobufOutput -Required.Proto2.ProtobufInput.ValidDataRepeated.FLOAT.PackedInput.ProtobufOutput -Required.Proto2.ProtobufInput.ValidDataRepeated.INT32.PackedInput.ProtobufOutput -Required.Proto2.ProtobufInput.ValidDataRepeated.INT64.PackedInput.ProtobufOutput -Required.Proto2.ProtobufInput.ValidDataRepeated.SFIXED32.PackedInput.ProtobufOutput -Required.Proto2.ProtobufInput.ValidDataRepeated.SFIXED64.PackedInput.ProtobufOutput -Required.Proto2.ProtobufInput.ValidDataRepeated.SINT32.PackedInput.ProtobufOutput Required.Proto2.ProtobufInput.ValidDataRepeated.SINT64.PackedInput.ProtobufOutput Required.Proto2.ProtobufInput.ValidDataRepeated.UINT32.PackedInput.ProtobufOutput Required.Proto2.ProtobufInput.ValidDataRepeated.UINT64.PackedInput.ProtobufOutput Required.Proto3.ProtobufInput.ValidDataRepeated.BOOL.UnpackedInput.ProtobufOutput Required.Proto3.ProtobufInput.ValidDataRepeated.DOUBLE.UnpackedInput.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.ENUM.UnpackedInput.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.FIXED32.UnpackedInput.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.FIXED64.UnpackedInput.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.UnpackedInput.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.INT32.UnpackedInput.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.INT64.UnpackedInput.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.SFIXED32.UnpackedInput.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.SFIXED64.UnpackedInput.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.SINT32.UnpackedInput.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.SINT64.UnpackedInput.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.UINT32.UnpackedInput.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.UINT64.UnpackedInput.ProtobufOutput diff --git a/conformance/failure_list_python.txt b/conformance/failure_list_python.txt index 798c84430f..f55122790b 100644 --- a/conformance/failure_list_python.txt +++ b/conformance/failure_list_python.txt @@ -24,19 +24,8 @@ Recommended.Proto3.ProtobufInput.ValidDataScalarBinary.STRING[0].ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataScalarBinary.UINT32[0].ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataScalarBinary.UINT32[5].ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataScalarBinary.UINT64[0].ProtobufOutput -Required.Proto2.ProtobufInput.IllegalZeroFieldNum_Case_0 -Required.Proto2.ProtobufInput.IllegalZeroFieldNum_Case_1 -Required.Proto2.ProtobufInput.IllegalZeroFieldNum_Case_2 -Required.Proto2.ProtobufInput.IllegalZeroFieldNum_Case_3 -Required.Proto2.ProtobufInput.ValidDataMap.STRING.MESSAGE.MergeValue.ProtobufOutput Required.Proto3.JsonInput.DoubleFieldTooSmall Required.Proto3.JsonInput.FloatFieldTooLarge Required.Proto3.JsonInput.FloatFieldTooSmall Required.Proto3.JsonInput.RepeatedFieldWrongElementTypeExpectingIntegersGotBool Required.Proto3.JsonInput.TimestampJsonInputLowercaseT -Required.Proto3.ProtobufInput.IllegalZeroFieldNum_Case_0 -Required.Proto3.ProtobufInput.IllegalZeroFieldNum_Case_1 -Required.Proto3.ProtobufInput.IllegalZeroFieldNum_Case_2 -Required.Proto3.ProtobufInput.IllegalZeroFieldNum_Case_3 -Required.Proto3.ProtobufInput.ValidDataMap.STRING.MESSAGE.MergeValue.JsonOutput -Required.Proto3.ProtobufInput.ValidDataMap.STRING.MESSAGE.MergeValue.ProtobufOutput diff --git a/java/core/src/main/java/com/google/protobuf/ExtensionRegistryFactory.java b/java/core/src/main/java/com/google/protobuf/ExtensionRegistryFactory.java index f070aae796..241fcbd932 100644 --- a/java/core/src/main/java/com/google/protobuf/ExtensionRegistryFactory.java +++ b/java/core/src/main/java/com/google/protobuf/ExtensionRegistryFactory.java @@ -59,26 +59,16 @@ final class ExtensionRegistryFactory { /** Construct a new, empty instance. */ public static ExtensionRegistryLite create() { - if (EXTENSION_REGISTRY_CLASS != null) { - try { - return invokeSubclassFactory("newInstance"); - } catch (Exception e) { - // return a Lite registry. - } - } - return new ExtensionRegistryLite(); + ExtensionRegistryLite result = invokeSubclassFactory("newInstance"); + + return result != null ? result : new ExtensionRegistryLite(); } /** Get the unmodifiable singleton empty instance. */ public static ExtensionRegistryLite createEmpty() { - if (EXTENSION_REGISTRY_CLASS != null) { - try { - return invokeSubclassFactory("getEmptyRegistry"); - } catch (Exception e) { - // return a Lite registry. - } - } - return EMPTY_REGISTRY_LITE; + ExtensionRegistryLite result = invokeSubclassFactory("getEmptyRegistry"); + + return result != null ? result : EMPTY_REGISTRY_LITE; } @@ -87,9 +77,17 @@ final class ExtensionRegistryFactory { && EXTENSION_REGISTRY_CLASS.isAssignableFrom(registry.getClass()); } - private static final ExtensionRegistryLite invokeSubclassFactory(String methodName) - throws Exception { - return (ExtensionRegistryLite) - EXTENSION_REGISTRY_CLASS.getDeclaredMethod(methodName).invoke(null); + /* @Nullable */ + private static final ExtensionRegistryLite invokeSubclassFactory(String methodName) { + if (EXTENSION_REGISTRY_CLASS == null) { + return null; + } + + try { + return (ExtensionRegistryLite) + EXTENSION_REGISTRY_CLASS.getDeclaredMethod(methodName).invoke(null); + } catch (Exception e) { + return null; + } } } diff --git a/java/core/src/main/java/com/google/protobuf/ExtensionRegistryLite.java b/java/core/src/main/java/com/google/protobuf/ExtensionRegistryLite.java index f07d607212..0c4b20ed74 100644 --- a/java/core/src/main/java/com/google/protobuf/ExtensionRegistryLite.java +++ b/java/core/src/main/java/com/google/protobuf/ExtensionRegistryLite.java @@ -83,19 +83,21 @@ public class ExtensionRegistryLite { // Visible for testing. static final String EXTENSION_CLASS_NAME = "com.google.protobuf.Extension"; - /* @Nullable */ - static Class resolveExtensionClass() { - try { - return Class.forName(EXTENSION_CLASS_NAME); - } catch (ClassNotFoundException e) { - // See comment in ExtensionRegistryFactory on the potential expense of this. - return null; + private static class ExtensionClassHolder { + /* @Nullable */ + static final Class INSTANCE = resolveExtensionClass(); + + /* @Nullable */ + static Class resolveExtensionClass() { + try { + return Class.forName(EXTENSION_CLASS_NAME); + } catch (ClassNotFoundException e) { + // See comment in ExtensionRegistryFactory on the potential expense of this. + return null; + } } } - /* @Nullable */ - private static final Class extensionClass = resolveExtensionClass(); - public static boolean isEagerlyParseMessageSets() { return eagerlyParseMessageSets; } @@ -175,7 +177,7 @@ public class ExtensionRegistryLite { } if (doFullRuntimeInheritanceCheck && ExtensionRegistryFactory.isFullRegistry(this)) { try { - this.getClass().getMethod("add", extensionClass).invoke(this, extension); + this.getClass().getMethod("add", ExtensionClassHolder.INSTANCE).invoke(this, extension); } catch (Exception e) { throw new IllegalArgumentException( String.format("Could not invoke ExtensionRegistry#add for %s", extension), e); diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java index 02588bb7d9..d46b1bac7b 100644 --- a/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java +++ b/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java @@ -115,7 +115,8 @@ public abstract class GeneratedMessageLite< @SuppressWarnings("unchecked") // Guaranteed by isInstance + runtime @Override - public boolean equals(Object other) { + public boolean equals( + Object other) { if (this == other) { return true; } @@ -348,16 +349,20 @@ public abstract class GeneratedMessageLite< * Called before any method that would mutate the builder to ensure that it correctly copies any * state before the write happens to preserve immutability guarantees. */ - protected void copyOnWrite() { + protected final void copyOnWrite() { if (isBuilt) { - MessageType newInstance = - (MessageType) instance.dynamicMethod(MethodToInvoke.NEW_MUTABLE_INSTANCE); - mergeFromInstance(newInstance, instance); - instance = newInstance; + copyOnWriteInternal(); isBuilt = false; } } + protected void copyOnWriteInternal() { + MessageType newInstance = + (MessageType) instance.dynamicMethod(MethodToInvoke.NEW_MUTABLE_INSTANCE); + mergeFromInstance(newInstance, instance); + instance = newInstance; + } + @Override public final boolean isInitialized() { return GeneratedMessageLite.isInitialized(instance, /* shouldMemoize= */ false); @@ -919,12 +924,8 @@ public abstract class GeneratedMessageLite< } @Override - protected void copyOnWrite() { - if (!isBuilt) { - return; - } - - super.copyOnWrite(); + protected void copyOnWriteInternal() { + super.copyOnWriteInternal(); instance.extensions = instance.extensions.clone(); } diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java index 76ab3aa60b..4e00bb4728 100644 --- a/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java +++ b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java @@ -58,8 +58,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.ObjectStreamException; import java.io.Serializable; -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; @@ -81,17 +79,11 @@ import java.util.TreeMap; public abstract class GeneratedMessageV3 extends AbstractMessage implements Serializable { private static final long serialVersionUID = 1L; - // Whether to use reflection for FieldAccessor - private static boolean forTestUseReflection = false; - - static void setForTestUseReflection(boolean useReflection) { - forTestUseReflection = useReflection; - } /** - * For testing. Allows a test to disable the optimization that avoids using - * field builders for nested messages until they are requested. By disabling - * this optimization, existing tests can be reused to test the field builders. + * For testing. Allows a test to disable the optimization that avoids using field builders for + * nested messages until they are requested. By disabling this optimization, existing tests can be + * reused to test the field builders. */ protected static boolean alwaysUseFieldBuilders = false; @@ -1880,32 +1872,14 @@ public abstract class GeneratedMessageV3 extends AbstractMessage } } - /** Calls invoke and throws a RuntimeException if it fails. */ - private static RuntimeException handleException(Throwable e) { - if (e instanceof ClassCastException) { - // Reflection throws a bad param type as an IllegalArgumentException, whereas MethodHandle - // throws it as a ClassCastException; convert for backwards compatibility - throw new IllegalArgumentException(e); - } else if (e instanceof RuntimeException) { - throw (RuntimeException) e; - } else if (e instanceof Error) { - throw (Error) e; - } else { - throw new RuntimeException( - "Unexpected exception thrown by generated accessor method.", e); - } - } - /** - * Gets the map field with the given field number. This method should be - * overridden in the generated message class if the message contains map - * fields. + * Gets the map field with the given field number. This method should be overridden in the + * generated message class if the message contains map fields. * - * Unlike other field types, reflection support for map fields can't be - * implemented based on generated public API because we need to access a - * map field as a list in reflection API but the generated API only allows - * us to access it as a map. This method returns the underlying map field - * directly and thus enables us to access the map field as a list. + *

Unlike other field types, reflection support for map fields can't be implemented based on + * generated public API because we need to access a map field as a list in reflection API but the + * generated API only allows us to access it as a map. This method returns the underlying map + * field directly and thus enables us to access the map field as a list. */ @SuppressWarnings({"rawtypes", "unused"}) protected MapField internalGetMapField(int fieldNumber) { @@ -2237,108 +2211,9 @@ public abstract class GeneratedMessageV3 extends AbstractMessage } } - private static final class MethodHandleInvoker implements MethodInvoker { - protected final MethodHandle getMethod; - protected final MethodHandle getMethodBuilder; - protected final MethodHandle setMethod; - protected final MethodHandle hasMethod; - protected final MethodHandle hasMethodBuilder; - protected final MethodHandle clearMethod; - protected final MethodHandle caseMethod; - protected final MethodHandle caseMethodBuilder; - - MethodHandleInvoker(ReflectionInvoker accessor) throws IllegalAccessException { - MethodHandles.Lookup lookup = MethodHandles.publicLookup(); - - this.getMethod = lookup.unreflect(accessor.getMethod); - this.getMethodBuilder = lookup.unreflect(accessor.getMethodBuilder); - this.setMethod = lookup.unreflect(accessor.setMethod); - this.hasMethod = - (accessor.hasMethod != null) ? lookup.unreflect(accessor.hasMethod) : null; - this.hasMethodBuilder = (accessor.hasMethodBuilder != null) - ? lookup.unreflect(accessor.hasMethodBuilder) : null; - this.clearMethod = lookup.unreflect(accessor.clearMethod); - this.caseMethod = - (accessor.caseMethod != null) ? lookup.unreflect(accessor.caseMethod) : null; - this.caseMethodBuilder = (accessor.caseMethodBuilder != null) - ? lookup.unreflect(accessor.caseMethodBuilder) : null; - } - - @Override - public Object get(final GeneratedMessageV3 message) { - try { - return getMethod.invoke(message); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public Object get(GeneratedMessageV3.Builder builder) { - try { - return getMethodBuilder.invoke(builder); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public int getOneofFieldNumber(final GeneratedMessageV3 message) { - try { - return ((Internal.EnumLite) caseMethod.invoke(message)).getNumber(); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public int getOneofFieldNumber(final GeneratedMessageV3.Builder builder) { - try { - return ((Internal.EnumLite) caseMethodBuilder.invoke(builder)).getNumber(); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public void set(final GeneratedMessageV3.Builder builder, final Object value) { - try { - setMethod.invoke(builder, value); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public boolean has(final GeneratedMessageV3 message) { - try { - return (Boolean) hasMethod.invoke(message); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public boolean has(GeneratedMessageV3.Builder builder) { - try { - return (Boolean) hasMethodBuilder.invoke(builder); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public void clear(final GeneratedMessageV3.Builder builder) { - try { - clearMethod.invoke(builder); - } catch (Throwable e) { - throw handleException(e); - } - } - } - SingularFieldAccessor( - final FieldDescriptor descriptor, final String camelCaseName, + final FieldDescriptor descriptor, + final String camelCaseName, final Class messageClass, final Class builderClass, final String containingOneofCamelCaseName) { @@ -2356,23 +2231,11 @@ public abstract class GeneratedMessageV3 extends AbstractMessage hasHasMethod); field = descriptor; type = reflectionInvoker.getMethod.getReturnType(); - invoker = tryGetMethodHandleInvoke(reflectionInvoker); + invoker = getMethodInvoker(reflectionInvoker); } - static MethodInvoker tryGetMethodHandleInvoke(ReflectionInvoker accessor) { - if (forTestUseReflection) { - return accessor; - } - try { - return new MethodHandleInvoker(accessor); - } catch (NoClassDefFoundError e) { - // Fall back to reflection if MethodHandleInvoker isn't available, - // allowing clients that don't want to use method handles to opt out - // by deleting the class. - return accessor; - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } + static MethodInvoker getMethodInvoker(ReflectionInvoker accessor) { + return accessor; } // Note: We use Java reflection to call public methods rather than @@ -2581,114 +2444,6 @@ public abstract class GeneratedMessageV3 extends AbstractMessage } } - private static final class MethodHandleInvoker implements MethodInvoker { - protected final MethodHandle getMethod; - protected final MethodHandle getMethodBuilder; - protected final MethodHandle getRepeatedMethod; - protected final MethodHandle getRepeatedMethodBuilder; - protected final MethodHandle setRepeatedMethod; - protected final MethodHandle addRepeatedMethod; - protected final MethodHandle getCountMethod; - protected final MethodHandle getCountMethodBuilder; - protected final MethodHandle clearMethod; - - MethodHandleInvoker(ReflectionInvoker accessor) throws IllegalAccessException { - MethodHandles.Lookup lookup = MethodHandles.lookup(); - - this.getMethod = lookup.unreflect(accessor.getMethod); - this.getMethodBuilder = lookup.unreflect(accessor.getMethodBuilder); - this.getRepeatedMethod = lookup.unreflect(accessor.getRepeatedMethod); - this.getRepeatedMethodBuilder = lookup.unreflect(accessor.getRepeatedMethodBuilder); - this.setRepeatedMethod = lookup.unreflect(accessor.setRepeatedMethod); - this.addRepeatedMethod = lookup.unreflect(accessor.addRepeatedMethod); - this.getCountMethod = lookup.unreflect(accessor.getCountMethod); - this.getCountMethodBuilder = lookup.unreflect(accessor.getCountMethodBuilder); - this.clearMethod = lookup.unreflect(accessor.clearMethod); - } - - @Override - public Object get(final GeneratedMessageV3 message) { - try { - return getMethod.invoke(message); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public Object get(GeneratedMessageV3.Builder builder) { - try { - return getMethodBuilder.invoke(builder); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public Object getRepeated(final GeneratedMessageV3 message, final int index) { - try { - return getRepeatedMethod.invoke(message, index); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public Object getRepeated(GeneratedMessageV3.Builder builder, int index) { - try { - return getRepeatedMethodBuilder.invoke(builder, index); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public void setRepeated( - final GeneratedMessageV3.Builder builder, final int index, final Object value) { - try { - setRepeatedMethod.invoke(builder, index, value); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public void addRepeated(final GeneratedMessageV3.Builder builder, final Object value) { - try { - addRepeatedMethod.invoke(builder, value); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public int getRepeatedCount(final GeneratedMessageV3 message) { - try { - return (Integer) getCountMethod.invoke(message); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public int getRepeatedCount(GeneratedMessageV3.Builder builder) { - try { - return (Integer) getCountMethodBuilder.invoke(builder); - } catch (Throwable e) { - throw handleException(e); - } - } - - @Override - public void clear(final GeneratedMessageV3.Builder builder) { - try { - clearMethod.invoke(builder); - } catch (Throwable e) { - throw handleException(e); - } - } - } - protected final Class type; protected final MethodInvoker invoker; @@ -2699,23 +2454,11 @@ public abstract class GeneratedMessageV3 extends AbstractMessage ReflectionInvoker reflectionInvoker = new ReflectionInvoker(descriptor, camelCaseName, messageClass, builderClass); type = reflectionInvoker.getRepeatedMethod.getReturnType(); - invoker = tryGetMethodHandleInvoke(reflectionInvoker); + invoker = getMethodInvoker(reflectionInvoker); } - static MethodInvoker tryGetMethodHandleInvoke(ReflectionInvoker accessor) { - if (forTestUseReflection) { - return accessor; - } - try { - return new MethodHandleInvoker(accessor); - } catch (NoClassDefFoundError e) { - // Fall back to reflection if MethodHandleInvoker isn't available, - // allowing clients that don't want to use method handles to opt out - // by deleting the class. - return accessor; - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } + static MethodInvoker getMethodInvoker(ReflectionInvoker accessor) { + return accessor; } @Override diff --git a/java/core/src/main/java/com/google/protobuf/RopeByteString.java b/java/core/src/main/java/com/google/protobuf/RopeByteString.java index e031839573..c2557ab85b 100644 --- a/java/core/src/main/java/com/google/protobuf/RopeByteString.java +++ b/java/core/src/main/java/com/google/protobuf/RopeByteString.java @@ -811,6 +811,16 @@ final class RopeByteString extends ByteString { initialize(); } + /** + * Reads up to {@code len} bytes of data into array {@code b}. + * + *

Note that {@link InputStream#read(byte[], int, int)} and {@link + * ByteArrayInputStream#read(byte[], int, int)} behave inconsistently when reading 0 bytes at + * EOF; the interface defines the return value to be 0 and the latter returns -1. We use the + * latter behavior so that all ByteString streams are consistent. + * + * @return -1 if at EOF, otherwise the actual number of bytes read. + */ @Override public int read(byte[] b, int offset, int length) { if (b == null) { @@ -818,7 +828,12 @@ final class RopeByteString extends ByteString { } else if (offset < 0 || length < 0 || length > b.length - offset) { throw new IndexOutOfBoundsException(); } - return readSkipInternal(b, offset, length); + int bytesRead = readSkipInternal(b, offset, length); + if (bytesRead == 0) { + return -1; + } else { + return bytesRead; + } } @Override @@ -845,10 +860,6 @@ final class RopeByteString extends ByteString { while (bytesRemaining > 0) { advanceIfCurrentPieceFullyRead(); if (currentPiece == null) { - if (bytesRemaining == length) { - // We didn't manage to read anything - return -1; - } break; } else { // Copy the bytes from this piece. diff --git a/java/core/src/test/java/com/google/protobuf/GeneratedMessageTest.java b/java/core/src/test/java/com/google/protobuf/GeneratedMessageTest.java index 5a43bca0e9..9261a36122 100644 --- a/java/core/src/test/java/com/google/protobuf/GeneratedMessageTest.java +++ b/java/core/src/test/java/com/google/protobuf/GeneratedMessageTest.java @@ -69,7 +69,6 @@ import java.util.Collections; import java.util.Iterator; import java.util.List; import junit.framework.TestCase; -import junit.framework.TestSuite; /** * Unit test for generated messages and generated code. See also {@link MessageTest}, which tests @@ -81,37 +80,8 @@ public class GeneratedMessageTest extends TestCase { TestUtil.ReflectionTester reflectionTester = new TestUtil.ReflectionTester(TestAllTypes.getDescriptor(), null); - public static TestSuite suite() { - TestSuite suite = new TestSuite(); - suite.addTestSuite(ReflectionTest.class); - suite.addTestSuite(FastInvokeTest.class); - return suite; - } - - public static class ReflectionTest extends GeneratedMessageTest { - public ReflectionTest() { - super(true); - } - } - - public static class FastInvokeTest extends GeneratedMessageTest { - public FastInvokeTest() { - super(false); - } - } - - private final boolean useReflection; - - GeneratedMessageTest(boolean useReflection) { - this.useReflection = useReflection; - } - - @Override public void setUp() { - GeneratedMessageV3.setForTestUseReflection(useReflection); - } - - @Override public void tearDown() { - GeneratedMessageV3.setForTestUseReflection(false); + @Override + public void tearDown() { GeneratedMessageV3.setAlwaysUseFieldBuildersForTesting(false); } diff --git a/java/core/src/test/java/com/google/protobuf/LiteralByteStringTest.java b/java/core/src/test/java/com/google/protobuf/LiteralByteStringTest.java index 0131ea2528..cab14c38b7 100644 --- a/java/core/src/test/java/com/google/protobuf/LiteralByteStringTest.java +++ b/java/core/src/test/java/com/google/protobuf/LiteralByteStringTest.java @@ -30,6 +30,8 @@ package com.google.protobuf; +import static com.google.common.truth.Truth.assertThat; + import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.EOFException; @@ -516,6 +518,7 @@ public class LiteralByteStringTest extends TestCase { InputStream input = stringUnderTest.newInput(); int stringSize = stringUnderTest.size(); int nearEndIndex = stringSize * 2 / 3; + long skipped1 = input.skip(nearEndIndex); assertEquals("InputStream.skip()", skipped1, nearEndIndex); assertEquals("InputStream.available()", stringSize - skipped1, input.available()); @@ -524,10 +527,14 @@ public class LiteralByteStringTest extends TestCase { assertEquals( "InputStream.skip(), read()", stringUnderTest.byteAt(nearEndIndex) & 0xFF, input.read()); assertEquals("InputStream.available()", stringSize - skipped1 - 1, input.available()); + long skipped2 = input.skip(stringSize); assertEquals("InputStream.skip() incomplete", skipped2, stringSize - skipped1 - 1); assertEquals("InputStream.skip(), no more input", 0, input.available()); assertEquals("InputStream.skip(), no more input", -1, input.read()); + assertThat(input.skip(1)).isEqualTo(0); + assertThat(input.read(new byte[1], /* off= */ 0, /*len=*/ 0)).isEqualTo(-1); + input.reset(); assertEquals("InputStream.reset() succeded", stringSize - skipped1, input.available()); assertEquals( diff --git a/python/google/protobuf/internal/decoder.py b/python/google/protobuf/internal/decoder.py index fddcf146dc..c04df8e294 100755 --- a/python/google/protobuf/internal/decoder.py +++ b/python/google/protobuf/internal/decoder.py @@ -829,9 +829,10 @@ def MessageSetItemDecoder(descriptor): (MESSAGE_SET_ITEM_TAG, buffer[message_set_item_start:pos].tobytes())) if message._unknown_field_set is None: message._unknown_field_set = containers.UnknownFieldSet() - message._unknown_field_set._add(type_id, - wire_format.WIRETYPE_LENGTH_DELIMITED, - buffer[message_start:message_end]) + message._unknown_field_set._add( + type_id, + wire_format.WIRETYPE_LENGTH_DELIMITED, + buffer[message_start:message_end].tobytes()) # pylint: enable=protected-access return pos @@ -870,7 +871,7 @@ def MapDecoder(field_descriptor, new_default, is_message_map): raise _DecodeError('Unexpected end-group tag.') if is_message_map: - value[submsg.key].MergeFrom(submsg.value) + value[submsg.key].CopyFrom(submsg.value) else: value[submsg.key] = submsg.value @@ -962,7 +963,7 @@ def _DecodeUnknownField(buffer, pos, wire_type): (data, pos) = _DecodeFixed32(buffer, pos) elif wire_type == wire_format.WIRETYPE_LENGTH_DELIMITED: (size, pos) = _DecodeVarint(buffer, pos) - data = buffer[pos:pos+size] + data = buffer[pos:pos+size].tobytes() pos += size elif wire_type == wire_format.WIRETYPE_START_GROUP: (data, pos) = _DecodeUnknownFieldSet(buffer, pos) diff --git a/python/google/protobuf/internal/message_test.py b/python/google/protobuf/internal/message_test.py index 0bd5cf0aac..f12b531f8b 100755 --- a/python/google/protobuf/internal/message_test.py +++ b/python/google/protobuf/internal/message_test.py @@ -164,6 +164,9 @@ class MessageTest(unittest.TestCase): msg.FromString(end_tag) self.assertEqual('Unexpected end-group tag.', str(context.exception)) + # Field number 0 is illegal. + self.assertRaises(message.DecodeError, msg.FromString, b'\3\4') + def testDeterminismParameters(self, message_module): # This message is always deterministically serialized, even if determinism # is disabled, so we can use it to verify that all the determinism @@ -2018,6 +2021,15 @@ class Proto3Test(unittest.TestCase): m1.map_int32_all_types.MergeFrom(m2.map_int32_message) self.assertEqual(10, m1.map_int32_all_types[123].optional_int32) + # Test overwrite message value map + msg = map_unittest_pb2.TestMap() + msg.map_int32_foreign_message[222].c = 123 + msg2 = map_unittest_pb2.TestMap() + msg2.map_int32_foreign_message[222].d = 20 + msg.MergeFromString(msg2.SerializeToString()) + self.assertEqual(msg.map_int32_foreign_message[222].d, 20) + self.assertNotEqual(msg.map_int32_foreign_message[222].c, 123) + def testMergeFromBadType(self): msg = map_unittest_pb2.TestMap() with self.assertRaisesRegexp( diff --git a/python/google/protobuf/internal/python_message.py b/python/google/protobuf/internal/python_message.py index bab1bef452..8fb1946027 100755 --- a/python/google/protobuf/internal/python_message.py +++ b/python/google/protobuf/internal/python_message.py @@ -1173,6 +1173,8 @@ def _AddMergeFromStringMethod(message_descriptor, cls): # pylint: disable=protected-access (tag, _) = decoder._DecodeVarint(tag_bytes, 0) field_number, wire_type = wire_format.UnpackTag(tag) + if field_number == 0: + raise message_mod.DecodeError('Field number 0 is illegal.') # TODO(jieluo): remove old_pos. old_pos = new_pos (data, new_pos) = decoder._DecodeUnknownField( diff --git a/python/google/protobuf/internal/text_format_test.py b/python/google/protobuf/internal/text_format_test.py index f4b9326a73..504af13a12 100755 --- a/python/google/protobuf/internal/text_format_test.py +++ b/python/google/protobuf/internal/text_format_test.py @@ -450,6 +450,46 @@ class TextFormatMessageToStringTests(TextFormatBase): text_format.Parse(expected_text, parsed_proto) self.assertEqual(message_proto, parsed_proto) + def testPrintUnknownFieldsEmbeddedMessageInBytes(self, message_module): + inner_msg = message_module.TestAllTypes() + inner_msg.optional_int32 = 101 + inner_msg.optional_double = 102.0 + inner_msg.optional_string = u'hello' + inner_msg.optional_bytes = b'103' + inner_msg.optional_nested_message.bb = 105 + inner_data = inner_msg.SerializeToString() + outer_message = message_module.TestAllTypes() + outer_message.optional_int32 = 101 + outer_message.optional_bytes = inner_data + all_data = outer_message.SerializeToString() + empty_message = message_module.TestEmptyMessage() + empty_message.ParseFromString(all_data) + + self.assertEqual(' 1: 101\n' + ' 15 {\n' + ' 1: 101\n' + ' 12: 4636878028842991616\n' + ' 14: "hello"\n' + ' 15: "103"\n' + ' 18 {\n' + ' 1: 105\n' + ' }\n' + ' }\n', + text_format.MessageToString(empty_message, + indent=2, + print_unknown_fields=True)) + self.assertEqual('1: 101 ' + '15 { ' + '1: 101 ' + '12: 4636878028842991616 ' + '14: "hello" ' + '15: "103" ' + '18 { 1: 105 } ' + '}', + text_format.MessageToString(empty_message, + print_unknown_fields=True, + as_one_line=True)) + @_parameterized.parameters(unittest_pb2, unittest_proto3_arena_pb2) class TextFormatMessageToTextBytesTests(TextFormatBase): @@ -868,51 +908,6 @@ class OnlyWorksWithProto2RightNowTests(TextFormatBase): print_unknown_fields=True, as_one_line=True)) - def testPrintUnknownFieldsEmbeddedMessageInBytes(self): - inner_msg = unittest_pb2.TestAllTypes() - inner_msg.optional_int32 = 101 - inner_msg.optional_double = 102.0 - inner_msg.optional_string = u'hello' - inner_msg.optional_bytes = b'103' - inner_msg.optionalgroup.a = 104 - inner_msg.optional_nested_message.bb = 105 - inner_data = inner_msg.SerializeToString() - outer_message = unittest_pb2.TestAllTypes() - outer_message.optional_int32 = 101 - outer_message.optional_bytes = inner_data - all_data = outer_message.SerializeToString() - empty_message = unittest_pb2.TestEmptyMessage() - empty_message.ParseFromString(all_data) - - self.assertEqual(' 1: 101\n' - ' 15 {\n' - ' 1: 101\n' - ' 12: 4636878028842991616\n' - ' 14: "hello"\n' - ' 15: "103"\n' - ' 16 {\n' - ' 17: 104\n' - ' }\n' - ' 18 {\n' - ' 1: 105\n' - ' }\n' - ' }\n', - text_format.MessageToString(empty_message, - indent=2, - print_unknown_fields=True)) - self.assertEqual('1: 101 ' - '15 { ' - '1: 101 ' - '12: 4636878028842991616 ' - '14: "hello" ' - '15: "103" ' - '16 { 17: 104 } ' - '18 { 1: 105 } ' - '}', - text_format.MessageToString(empty_message, - print_unknown_fields=True, - as_one_line=True)) - def testPrintInIndexOrder(self): message = unittest_pb2.TestFieldOrderings() # Fields are listed in index order instead of field number. diff --git a/python/google/protobuf/internal/unknown_fields_test.py b/python/google/protobuf/internal/unknown_fields_test.py index fc5f38be16..277603d952 100755 --- a/python/google/protobuf/internal/unknown_fields_test.py +++ b/python/google/protobuf/internal/unknown_fields_test.py @@ -112,8 +112,7 @@ class UnknownFieldsTest(unittest.TestCase): wire_format.WIRETYPE_LENGTH_DELIMITED) d = unknown_fields[0].data message_new = message_set_extensions_pb2.TestMessageSetExtension1() - message_new.ParseFromString(d.tobytes() if isinstance(d, ( - memoryview)) else d) + message_new.ParseFromString(d) self.assertEqual(message1, message_new) # Verify that the unknown extension is serialized unchanged @@ -208,6 +207,8 @@ class UnknownFieldsAccessorsTest(unittest.TestCase): self.assertEqual(expected_value[1], unknown_field.data[0].wire_type) self.assertEqual(expected_value[2], unknown_field.data[0].data) continue + if expected_type == wire_format.WIRETYPE_LENGTH_DELIMITED: + self.assertIn(type(unknown_field.data), (str, bytes)) if field_descriptor.label == descriptor.FieldDescriptor.LABEL_REPEATED: self.assertIn(unknown_field.data, expected_value) else: @@ -250,7 +251,7 @@ class UnknownFieldsAccessorsTest(unittest.TestCase): self.InternalCheckUnknownField('optional_fixed64', self.all_fields.optional_fixed64) - # Test lengthd elimited. + # Test length delimited. self.CheckUnknownField('optional_string', unknown_fields, self.all_fields.optional_string.encode('utf-8')) diff --git a/python/google/protobuf/pyext/descriptor.cc b/python/google/protobuf/pyext/descriptor.cc index 1637f83a30..6b1f4278b5 100644 --- a/python/google/protobuf/pyext/descriptor.cc +++ b/python/google/protobuf/pyext/descriptor.cc @@ -75,7 +75,7 @@ namespace python { // All descriptors are stored here. std::unordered_map* interned_descriptors; -PyObject* PyString_FromCppString(const string& str) { +PyObject* PyString_FromCppString(const std::string& str) { return PyString_FromStringAndSize(str.c_str(), str.size()); } @@ -192,7 +192,7 @@ const FileDescriptor* GetFileDescriptor(const MethodDescriptor* descriptor) { bool Reparse( PyMessageFactory* message_factory, const Message& from, Message* to) { // Reparse message. - string serialized; + std::string serialized; from.SerializeToString(&serialized); io::CodedInputStream input( reinterpret_cast(serialized.c_str()), serialized.size()); @@ -839,7 +839,7 @@ static PyObject* GetDefaultValue(PyBaseDescriptor *self, void *closure) { break; } case FieldDescriptor::CPPTYPE_STRING: { - const string& value = _GetDescriptor(self)->default_value_string(); + const std::string& value = _GetDescriptor(self)->default_value_string(); result = ToStringObject(_GetDescriptor(self), value); break; } @@ -1350,7 +1350,7 @@ static PyObject* GetSerializedPb(PyFileDescriptor *self, void *closure) { } FileDescriptorProto file_proto; _GetDescriptor(self)->CopyTo(&file_proto); - string contents; + std::string contents; file_proto.SerializePartialToString(&contents); self->serialized_pb = PyBytes_FromStringAndSize( contents.c_str(), contents.size()); @@ -1690,7 +1690,7 @@ static PyObject* FindMethodByName(PyBaseDescriptor *self, PyObject* arg) { } const MethodDescriptor* method_descriptor = - _GetDescriptor(self)->FindMethodByName(string(name, name_size)); + _GetDescriptor(self)->FindMethodByName(std::string(name, name_size)); if (method_descriptor == NULL) { PyErr_Format(PyExc_KeyError, "Couldn't find method %.200s", name); return NULL; diff --git a/python/google/protobuf/pyext/descriptor_containers.cc b/python/google/protobuf/pyext/descriptor_containers.cc index 8be8468164..865f0c0a6a 100644 --- a/python/google/protobuf/pyext/descriptor_containers.cc +++ b/python/google/protobuf/pyext/descriptor_containers.cc @@ -80,13 +80,15 @@ struct PyContainer; typedef int (*CountMethod)(PyContainer* self); typedef const void* (*GetByIndexMethod)(PyContainer* self, int index); -typedef const void* (*GetByNameMethod)(PyContainer* self, const string& name); +typedef const void* (*GetByNameMethod)(PyContainer* self, + const std::string& name); typedef const void* (*GetByCamelcaseNameMethod)(PyContainer* self, - const string& name); + const std::string& name); typedef const void* (*GetByNumberMethod)(PyContainer* self, int index); typedef PyObject* (*NewObjectFromItemMethod)(const void* descriptor); -typedef const string& (*GetItemNameMethod)(const void* descriptor); -typedef const string& (*GetItemCamelcaseNameMethod)(const void* descriptor); +typedef const std::string& (*GetItemNameMethod)(const void* descriptor); +typedef const std::string& (*GetItemCamelcaseNameMethod)( + const void* descriptor); typedef int (*GetItemNumberMethod)(const void* descriptor); typedef int (*GetItemIndexMethod)(const void* descriptor); @@ -181,7 +183,7 @@ static bool _GetItemByKey(PyContainer* self, PyObject* key, const void** item) { return false; } *item = self->container_def->get_by_name_fn( - self, string(name, name_size)); + self, std::string(name, name_size)); return true; } case PyContainer::KIND_BYCAMELCASENAME: @@ -198,7 +200,7 @@ static bool _GetItemByKey(PyContainer* self, PyObject* key, const void** item) { return false; } *item = self->container_def->get_by_camelcase_name_fn( - self, string(camelcase_name, name_size)); + self, std::string(camelcase_name, name_size)); return true; } case PyContainer::KIND_BYNUMBER: @@ -229,14 +231,14 @@ static PyObject* _NewKey_ByIndex(PyContainer* self, Py_ssize_t index) { switch (self->kind) { case PyContainer::KIND_BYNAME: { - const string& name(self->container_def->get_item_name_fn(item)); - return PyString_FromStringAndSize(name.c_str(), name.size()); + const std::string& name(self->container_def->get_item_name_fn(item)); + return PyString_FromStringAndSize(name.c_str(), name.size()); } case PyContainer::KIND_BYCAMELCASENAME: { - const string& name( - self->container_def->get_item_camelcase_name_fn(item)); - return PyString_FromStringAndSize(name.c_str(), name.size()); + const std::string& name( + self->container_def->get_item_camelcase_name_fn(item)); + return PyString_FromStringAndSize(name.c_str(), name.size()); } case PyContainer::KIND_BYNUMBER: { @@ -960,12 +962,12 @@ static int Count(PyContainer* self) { return GetDescriptor(self)->field_count(); } -static const void* GetByName(PyContainer* self, const string& name) { +static const void* GetByName(PyContainer* self, const std::string& name) { return GetDescriptor(self)->FindFieldByName(name); } static const void* GetByCamelcaseName(PyContainer* self, - const string& name) { + const std::string& name) { return GetDescriptor(self)->FindFieldByCamelcaseName(name); } @@ -981,11 +983,11 @@ static PyObject* NewObjectFromItem(const void* item) { return PyFieldDescriptor_FromDescriptor(static_cast(item)); } -static const string& GetItemName(const void* item) { +static const std::string& GetItemName(const void* item) { return static_cast(item)->name(); } -static const string& GetItemCamelcaseName(const void* item) { +static const std::string& GetItemCamelcaseName(const void* item) { return static_cast(item)->camelcase_name(); } @@ -1038,7 +1040,7 @@ static int Count(PyContainer* self) { return GetDescriptor(self)->nested_type_count(); } -static const void* GetByName(PyContainer* self, const string& name) { +static const void* GetByName(PyContainer* self, const std::string& name) { return GetDescriptor(self)->FindNestedTypeByName(name); } @@ -1050,7 +1052,7 @@ static PyObject* NewObjectFromItem(const void* item) { return PyMessageDescriptor_FromDescriptor(static_cast(item)); } -static const string& GetItemName(const void* item) { +static const std::string& GetItemName(const void* item) { return static_cast(item)->name(); } @@ -1090,7 +1092,7 @@ static int Count(PyContainer* self) { return GetDescriptor(self)->enum_type_count(); } -static const void* GetByName(PyContainer* self, const string& name) { +static const void* GetByName(PyContainer* self, const std::string& name) { return GetDescriptor(self)->FindEnumTypeByName(name); } @@ -1102,7 +1104,7 @@ static PyObject* NewObjectFromItem(const void* item) { return PyEnumDescriptor_FromDescriptor(static_cast(item)); } -static const string& GetItemName(const void* item) { +static const std::string& GetItemName(const void* item) { return static_cast(item)->name(); } @@ -1153,7 +1155,7 @@ static int Count(PyContainer* self) { return count; } -static const void* GetByName(PyContainer* self, const string& name) { +static const void* GetByName(PyContainer* self, const std::string& name) { return GetDescriptor(self)->FindEnumValueByName(name); } @@ -1181,7 +1183,7 @@ static PyObject* NewObjectFromItem(const void* item) { static_cast(item)); } -static const string& GetItemName(const void* item) { +static const std::string& GetItemName(const void* item) { return static_cast(item)->name(); } @@ -1213,7 +1215,7 @@ static int Count(PyContainer* self) { return GetDescriptor(self)->extension_count(); } -static const void* GetByName(PyContainer* self, const string& name) { +static const void* GetByName(PyContainer* self, const std::string& name) { return GetDescriptor(self)->FindExtensionByName(name); } @@ -1225,7 +1227,7 @@ static PyObject* NewObjectFromItem(const void* item) { return PyFieldDescriptor_FromDescriptor(static_cast(item)); } -static const string& GetItemName(const void* item) { +static const std::string& GetItemName(const void* item) { return static_cast(item)->name(); } @@ -1265,7 +1267,7 @@ static int Count(PyContainer* self) { return GetDescriptor(self)->oneof_decl_count(); } -static const void* GetByName(PyContainer* self, const string& name) { +static const void* GetByName(PyContainer* self, const std::string& name) { return GetDescriptor(self)->FindOneofByName(name); } @@ -1277,7 +1279,7 @@ static PyObject* NewObjectFromItem(const void* item) { return PyOneofDescriptor_FromDescriptor(static_cast(item)); } -static const string& GetItemName(const void* item) { +static const std::string& GetItemName(const void* item) { return static_cast(item)->name(); } @@ -1331,7 +1333,7 @@ static const void* GetByIndex(PyContainer* self, int index) { return GetDescriptor(self)->value(index); } -static const void* GetByName(PyContainer* self, const string& name) { +static const void* GetByName(PyContainer* self, const std::string& name) { return GetDescriptor(self)->FindValueByName(name); } @@ -1344,7 +1346,7 @@ static PyObject* NewObjectFromItem(const void* item) { static_cast(item)); } -static const string& GetItemName(const void* item) { +static const std::string& GetItemName(const void* item) { return static_cast(item)->name(); } @@ -1452,7 +1454,7 @@ static int Count(PyContainer* self) { return GetDescriptor(self)->method_count(); } -static const void* GetByName(PyContainer* self, const string& name) { +static const void* GetByName(PyContainer* self, const std::string& name) { return GetDescriptor(self)->FindMethodByName(name); } @@ -1464,7 +1466,7 @@ static PyObject* NewObjectFromItem(const void* item) { return PyMethodDescriptor_FromDescriptor(static_cast(item)); } -static const string& GetItemName(const void* item) { +static const std::string& GetItemName(const void* item) { return static_cast(item)->name(); } @@ -1514,7 +1516,7 @@ static int Count(PyContainer* self) { return GetDescriptor(self)->message_type_count(); } -static const void* GetByName(PyContainer* self, const string& name) { +static const void* GetByName(PyContainer* self, const std::string& name) { return GetDescriptor(self)->FindMessageTypeByName(name); } @@ -1526,7 +1528,7 @@ static PyObject* NewObjectFromItem(const void* item) { return PyMessageDescriptor_FromDescriptor(static_cast(item)); } -static const string& GetItemName(const void* item) { +static const std::string& GetItemName(const void* item) { return static_cast(item)->name(); } @@ -1562,7 +1564,7 @@ static int Count(PyContainer* self) { return GetDescriptor(self)->enum_type_count(); } -static const void* GetByName(PyContainer* self, const string& name) { +static const void* GetByName(PyContainer* self, const std::string& name) { return GetDescriptor(self)->FindEnumTypeByName(name); } @@ -1574,7 +1576,7 @@ static PyObject* NewObjectFromItem(const void* item) { return PyEnumDescriptor_FromDescriptor(static_cast(item)); } -static const string& GetItemName(const void* item) { +static const std::string& GetItemName(const void* item) { return static_cast(item)->name(); } @@ -1610,7 +1612,7 @@ static int Count(PyContainer* self) { return GetDescriptor(self)->extension_count(); } -static const void* GetByName(PyContainer* self, const string& name) { +static const void* GetByName(PyContainer* self, const std::string& name) { return GetDescriptor(self)->FindExtensionByName(name); } @@ -1622,7 +1624,7 @@ static PyObject* NewObjectFromItem(const void* item) { return PyFieldDescriptor_FromDescriptor(static_cast(item)); } -static const string& GetItemName(const void* item) { +static const std::string& GetItemName(const void* item) { return static_cast(item)->name(); } @@ -1658,7 +1660,7 @@ static int Count(PyContainer* self) { return GetDescriptor(self)->service_count(); } -static const void* GetByName(PyContainer* self, const string& name) { +static const void* GetByName(PyContainer* self, const std::string& name) { return GetDescriptor(self)->FindServiceByName(name); } @@ -1670,7 +1672,7 @@ static PyObject* NewObjectFromItem(const void* item) { return PyServiceDescriptor_FromDescriptor(static_cast(item)); } -static const string& GetItemName(const void* item) { +static const std::string& GetItemName(const void* item) { return static_cast(item)->name(); } diff --git a/python/google/protobuf/pyext/descriptor_database.cc b/python/google/protobuf/pyext/descriptor_database.cc index 0514b35cc7..26f1b5fb42 100644 --- a/python/google/protobuf/pyext/descriptor_database.cc +++ b/python/google/protobuf/pyext/descriptor_database.cc @@ -108,7 +108,7 @@ static bool GetFileDescriptorProto(PyObject* py_descriptor, } // Find a file by file name. -bool PyDescriptorDatabase::FindFileByName(const string& filename, +bool PyDescriptorDatabase::FindFileByName(const std::string& filename, FileDescriptorProto* output) { ScopedPyObjectPtr py_descriptor(PyObject_CallMethod( py_database_, "FindFileByName", "s#", filename.c_str(), filename.size())); @@ -117,7 +117,7 @@ bool PyDescriptorDatabase::FindFileByName(const string& filename, // Find the file that declares the given fully-qualified symbol name. bool PyDescriptorDatabase::FindFileContainingSymbol( - const string& symbol_name, FileDescriptorProto* output) { + const std::string& symbol_name, FileDescriptorProto* output) { ScopedPyObjectPtr py_descriptor( PyObject_CallMethod(py_database_, "FindFileContainingSymbol", "s#", symbol_name.c_str(), symbol_name.size())); @@ -128,7 +128,7 @@ bool PyDescriptorDatabase::FindFileContainingSymbol( // with the given field number. // Python DescriptorDatabases are not required to implement this method. bool PyDescriptorDatabase::FindFileContainingExtension( - const string& containing_type, int field_number, + const std::string& containing_type, int field_number, FileDescriptorProto* output) { ScopedPyObjectPtr py_method( PyObject_GetAttrString(py_database_, "FindFileContainingExtension")); @@ -148,7 +148,7 @@ bool PyDescriptorDatabase::FindFileContainingExtension( // order. // Python DescriptorDatabases are not required to implement this method. bool PyDescriptorDatabase::FindAllExtensionNumbers( - const string& containing_type, std::vector* output) { + const std::string& containing_type, std::vector* output) { ScopedPyObjectPtr py_method( PyObject_GetAttrString(py_database_, "FindAllExtensionNumbers")); if (py_method == NULL) { diff --git a/python/google/protobuf/pyext/descriptor_database.h b/python/google/protobuf/pyext/descriptor_database.h index daf25e0ba6..d2d9f8e506 100644 --- a/python/google/protobuf/pyext/descriptor_database.h +++ b/python/google/protobuf/pyext/descriptor_database.h @@ -48,18 +48,17 @@ class PyDescriptorDatabase : public DescriptorDatabase { // with a copy of FileDescriptorProto. // Find a file by file name. - bool FindFileByName(const string& filename, - FileDescriptorProto* output); + bool FindFileByName(const std::string& filename, FileDescriptorProto* output); // Find the file that declares the given fully-qualified symbol name. - bool FindFileContainingSymbol(const string& symbol_name, + bool FindFileContainingSymbol(const std::string& symbol_name, FileDescriptorProto* output); // Find the file which defines an extension extending the given message type // with the given field number. // Containing_type must be a fully-qualified type name. // Python objects are not required to implement this method. - bool FindFileContainingExtension(const string& containing_type, + bool FindFileContainingExtension(const std::string& containing_type, int field_number, FileDescriptorProto* output); @@ -67,7 +66,7 @@ class PyDescriptorDatabase : public DescriptorDatabase { // containing_type, and appends them to output in an undefined // order. // Python objects are not required to implement this method. - bool FindAllExtensionNumbers(const string& containing_type, + bool FindAllExtensionNumbers(const std::string& containing_type, std::vector* output); private: diff --git a/python/google/protobuf/pyext/descriptor_pool.cc b/python/google/protobuf/pyext/descriptor_pool.cc index 767659204f..2cffea305e 100644 --- a/python/google/protobuf/pyext/descriptor_pool.cc +++ b/python/google/protobuf/pyext/descriptor_pool.cc @@ -73,9 +73,9 @@ class BuildFileErrorCollector : public DescriptorPool::ErrorCollector { public: BuildFileErrorCollector() : error_message(""), had_errors_(false) {} - void AddError(const string& filename, const string& element_name, + void AddError(const std::string& filename, const std::string& element_name, const Message* descriptor, ErrorLocation location, - const string& message) override { + const std::string& message) override { // Replicates the logging behavior that happens in the C++ implementation // when an error collector is not passed in. if (!had_errors_) { @@ -93,7 +93,7 @@ class BuildFileErrorCollector : public DescriptorPool::ErrorCollector { error_message = ""; } - string error_message; + std::string error_message; private: bool had_errors_; @@ -240,7 +240,7 @@ static PyObject* FindMessageByName(PyObject* self, PyObject* arg) { const Descriptor* message_descriptor = reinterpret_cast(self)->pool->FindMessageTypeByName( - string(name, name_size)); + std::string(name, name_size)); if (message_descriptor == NULL) { return SetErrorFromCollector( @@ -264,7 +264,7 @@ static PyObject* FindFileByName(PyObject* self, PyObject* arg) { PyDescriptorPool* py_pool = reinterpret_cast(self); const FileDescriptor* file_descriptor = - py_pool->pool->FindFileByName(string(name, name_size)); + py_pool->pool->FindFileByName(std::string(name, name_size)); if (file_descriptor == NULL) { return SetErrorFromCollector(py_pool->error_collector, name, "file"); @@ -280,7 +280,7 @@ PyObject* FindFieldByName(PyDescriptorPool* self, PyObject* arg) { } const FieldDescriptor* field_descriptor = - self->pool->FindFieldByName(string(name, name_size)); + self->pool->FindFieldByName(std::string(name, name_size)); if (field_descriptor == NULL) { return SetErrorFromCollector(self->error_collector, name, "field"); } @@ -301,7 +301,7 @@ PyObject* FindExtensionByName(PyDescriptorPool* self, PyObject* arg) { } const FieldDescriptor* field_descriptor = - self->pool->FindExtensionByName(string(name, name_size)); + self->pool->FindExtensionByName(std::string(name, name_size)); if (field_descriptor == NULL) { return SetErrorFromCollector(self->error_collector, name, "extension field"); @@ -323,7 +323,7 @@ PyObject* FindEnumTypeByName(PyDescriptorPool* self, PyObject* arg) { } const EnumDescriptor* enum_descriptor = - self->pool->FindEnumTypeByName(string(name, name_size)); + self->pool->FindEnumTypeByName(std::string(name, name_size)); if (enum_descriptor == NULL) { return SetErrorFromCollector(self->error_collector, name, "enum"); } @@ -344,7 +344,7 @@ PyObject* FindOneofByName(PyDescriptorPool* self, PyObject* arg) { } const OneofDescriptor* oneof_descriptor = - self->pool->FindOneofByName(string(name, name_size)); + self->pool->FindOneofByName(std::string(name, name_size)); if (oneof_descriptor == NULL) { return SetErrorFromCollector(self->error_collector, name, "oneof"); } @@ -366,7 +366,7 @@ static PyObject* FindServiceByName(PyObject* self, PyObject* arg) { const ServiceDescriptor* service_descriptor = reinterpret_cast(self)->pool->FindServiceByName( - string(name, name_size)); + std::string(name, name_size)); if (service_descriptor == NULL) { return SetErrorFromCollector( reinterpret_cast(self)->error_collector, name, @@ -386,7 +386,7 @@ static PyObject* FindMethodByName(PyObject* self, PyObject* arg) { const MethodDescriptor* method_descriptor = reinterpret_cast(self)->pool->FindMethodByName( - string(name, name_size)); + std::string(name, name_size)); if (method_descriptor == NULL) { return SetErrorFromCollector( reinterpret_cast(self)->error_collector, name, @@ -406,7 +406,7 @@ static PyObject* FindFileContainingSymbol(PyObject* self, PyObject* arg) { const FileDescriptor* file_descriptor = reinterpret_cast(self)->pool->FindFileContainingSymbol( - string(name, name_size)); + std::string(name, name_size)); if (file_descriptor == NULL) { return SetErrorFromCollector( reinterpret_cast(self)->error_collector, name, diff --git a/python/google/protobuf/pyext/extension_dict.cc b/python/google/protobuf/pyext/extension_dict.cc index bb62d4cd3c..d7226d0c38 100644 --- a/python/google/protobuf/pyext/extension_dict.cc +++ b/python/google/protobuf/pyext/extension_dict.cc @@ -240,11 +240,11 @@ PyObject* _FindExtensionByName(ExtensionDict* self, PyObject* arg) { PyDescriptorPool* pool = cmessage::GetFactoryForMessage(self->parent)->pool; const FieldDescriptor* message_extension = - pool->pool->FindExtensionByName(string(name, name_size)); + pool->pool->FindExtensionByName(std::string(name, name_size)); if (message_extension == NULL) { // Is is the name of a message set extension? - const Descriptor* message_descriptor = pool->pool->FindMessageTypeByName( - string(name, name_size)); + const Descriptor* message_descriptor = + pool->pool->FindMessageTypeByName(std::string(name, name_size)); if (message_descriptor && message_descriptor->extension_count() > 0) { const FieldDescriptor* extension = message_descriptor->extension(0); if (extension->is_extension() && diff --git a/python/google/protobuf/pyext/map_container.cc b/python/google/protobuf/pyext/map_container.cc index 222a0014c5..ed0d31c8b6 100644 --- a/python/google/protobuf/pyext/map_container.cc +++ b/python/google/protobuf/pyext/map_container.cc @@ -108,7 +108,7 @@ Message* MapContainer::GetMutableMessage() { } // Consumes a reference on the Python string object. -static bool PyStringToSTL(PyObject* py_string, string* stl_string) { +static bool PyStringToSTL(PyObject* py_string, std::string* stl_string) { char *value; Py_ssize_t value_len; @@ -155,7 +155,7 @@ static bool PythonToMapKey(PyObject* obj, break; } case FieldDescriptor::CPPTYPE_STRING: { - string str; + std::string str; if (!PyStringToSTL(CheckString(obj, field_descriptor), &str)) { return false; } @@ -268,7 +268,7 @@ static bool PythonToMapValueRef(PyObject* obj, return true;; } case FieldDescriptor::CPPTYPE_STRING: { - string str; + std::string str; if (!PyStringToSTL(CheckString(obj, field_descriptor), &str)) { return false; } diff --git a/python/google/protobuf/pyext/message.cc b/python/google/protobuf/pyext/message.cc index 4fbc9a1f73..7066cba647 100644 --- a/python/google/protobuf/pyext/message.cc +++ b/python/google/protobuf/pyext/message.cc @@ -111,9 +111,9 @@ static int InsertEmptyWeakref(PyTypeObject* base); namespace { // Copied over from internal 'google/protobuf/stubs/strutil.h'. -inline void LowerString(string * s) { - string::iterator end = s->end(); - for (string::iterator i = s->begin(); i != end; ++i) { +inline void LowerString(std::string* s) { + std::string::iterator end = s->end(); + for (std::string::iterator i = s->begin(); i != end; ++i) { // tolower() changes based on locale. We don't want this! if ('A' <= *i && *i <= 'Z') *i += 'a' - 'A'; } @@ -436,7 +436,7 @@ static PyObject* GetClassAttribute(CMessageClass *self, PyObject* name) { static const char kSuffix[] = "_FIELD_NUMBER"; if (PyString_AsStringAndSize(name, &attr, &attr_size) >= 0 && strings::EndsWith(StringPiece(attr, attr_size), kSuffix)) { - string field_name(attr, attr_size - sizeof(kSuffix) + 1); + std::string field_name(attr, attr_size - sizeof(kSuffix) + 1); LowerString(&field_name); // Try to find a field with the given name, without the suffix. @@ -798,7 +798,7 @@ bool CheckAndSetString( } PyObject* ToStringObject(const FieldDescriptor* descriptor, - const string& value) { + const std::string& value) { if (descriptor->type() != FieldDescriptor::TYPE_STRING) { return PyBytes_FromStringAndSize(value.c_str(), value.length()); } @@ -960,7 +960,7 @@ static PyObject* GetIntegerEnumValue(const FieldDescriptor& descriptor, return NULL; } const EnumValueDescriptor* enum_value_descriptor = - enum_descriptor->FindValueByName(string(enum_label, size)); + enum_descriptor->FindValueByName(std::string(enum_label, size)); if (enum_value_descriptor == NULL) { PyErr_Format(PyExc_ValueError, "unknown enum label \"%s\"", enum_label); return NULL; @@ -1374,8 +1374,9 @@ int HasFieldByDescriptor(CMessage* self, return message->GetReflection()->HasField(*message, field_descriptor); } -const FieldDescriptor* FindFieldWithOneofs( - const Message* message, const string& field_name, bool* in_oneof) { +const FieldDescriptor* FindFieldWithOneofs(const Message* message, + const std::string& field_name, + bool* in_oneof) { *in_oneof = false; const Descriptor* descriptor = message->GetDescriptor(); const FieldDescriptor* field_descriptor = @@ -1446,7 +1447,7 @@ PyObject* HasField(CMessage* self, PyObject* arg) { Message* message = self->message; bool is_in_oneof; const FieldDescriptor* field_descriptor = - FindFieldWithOneofs(message, string(field_name, size), &is_in_oneof); + FindFieldWithOneofs(message, std::string(field_name, size), &is_in_oneof); if (field_descriptor == NULL) { if (!is_in_oneof) { PyErr_Format(PyExc_ValueError, "Protocol message %s has no field %s.", @@ -1623,7 +1624,7 @@ PyObject* ClearField(CMessage* self, PyObject* arg) { AssureWritable(self); bool is_in_oneof; const FieldDescriptor* field_descriptor = FindFieldWithOneofs( - self->message, string(field_name, field_size), &is_in_oneof); + self->message, std::string(field_name, field_size), &is_in_oneof); if (field_descriptor == NULL) { if (is_in_oneof) { // We gave the name of a oneof, and none of its fields are set. @@ -1671,7 +1672,7 @@ PyObject* Clear(CMessage* self) { // --------------------------------------------------------------------- -static string GetMessageName(CMessage* self) { +static std::string GetMessageName(CMessage* self) { if (self->parent_field_descriptor != NULL) { return self->parent_field_descriptor->full_name(); } else { @@ -1823,7 +1824,7 @@ static PyObject* ToStr(CMessage* self) { // Passes ownership printer.SetDefaultFieldValuePrinter(new PythonFieldValuePrinter()); printer.SetHideUnknownFields(true); - string output; + std::string output; if (!printer.PrintToString(*self->message, &output)) { PyErr_SetString(PyExc_ValueError, "Unable to convert message to str"); return NULL; @@ -2024,7 +2025,7 @@ static PyObject* WhichOneof(CMessage* self, PyObject* arg) { char *name_data; if (PyString_AsStringAndSize(arg, &name_data, &name_size) < 0) return NULL; - string oneof_name = string(name_data, name_size); + std::string oneof_name = std::string(name_data, name_size); const OneofDescriptor* oneof_desc = self->message->GetDescriptor()->FindOneofByName(oneof_name); if (oneof_desc == NULL) { @@ -2039,7 +2040,7 @@ static PyObject* WhichOneof(CMessage* self, PyObject* arg) { if (field_in_oneof == NULL) { Py_RETURN_NONE; } else { - const string& name = field_in_oneof->name(); + const std::string& name = field_in_oneof->name(); return PyString_FromStringAndSize(name.c_str(), name.size()); } } @@ -2131,7 +2132,7 @@ static PyObject* DiscardUnknownFields(CMessage* self) { PyObject* FindInitializationErrors(CMessage* self) { Message* message = self->message; - std::vector errors; + std::vector errors; message->FindInitializationErrors(&errors); PyObject* error_list = PyList_New(errors.size()); @@ -2139,7 +2140,7 @@ PyObject* FindInitializationErrors(CMessage* self) { return NULL; } for (size_t i = 0; i < errors.size(); ++i) { - const string& error = errors[i]; + const std::string& error = errors[i]; PyObject* error_string = PyString_FromStringAndSize( error.c_str(), error.length()); if (error_string == NULL) { @@ -2229,8 +2230,8 @@ PyObject* InternalGetScalar(const Message* message, break; } case FieldDescriptor::CPPTYPE_STRING: { - string scratch; - const string& value = + std::string scratch; + const std::string& value = reflection->GetStringReference(*message, field_descriptor, &scratch); result = ToStringObject(field_descriptor, value); break; diff --git a/python/google/protobuf/pyext/message.h b/python/google/protobuf/pyext/message.h index 2f20e2a04f..c5a635da02 100644 --- a/python/google/protobuf/pyext/message.h +++ b/python/google/protobuf/pyext/message.h @@ -344,7 +344,7 @@ bool CheckAndSetString( bool append, int index); PyObject* ToStringObject(const FieldDescriptor* descriptor, - const string& value); + const std::string& value); // Check if the passed field descriptor belongs to the given message. // If not, return false and set a Python exception (a KeyError) diff --git a/python/google/protobuf/pyext/repeated_scalar_container.cc b/python/google/protobuf/pyext/repeated_scalar_container.cc index 07ac602832..712182b374 100644 --- a/python/google/protobuf/pyext/repeated_scalar_container.cc +++ b/python/google/protobuf/pyext/repeated_scalar_container.cc @@ -259,8 +259,8 @@ static PyObject* Item(PyObject* pself, Py_ssize_t index) { break; } case FieldDescriptor::CPPTYPE_STRING: { - string scratch; - const string& value = reflection->GetRepeatedStringReference( + std::string scratch; + const std::string& value = reflection->GetRepeatedStringReference( *message, field_descriptor, index, &scratch); result = ToStringObject(field_descriptor, value); break; diff --git a/python/google/protobuf/text_format.py b/python/google/protobuf/text_format.py index 977c70db84..ecdfd5a714 100755 --- a/python/google/protobuf/text_format.py +++ b/python/google/protobuf/text_format.py @@ -578,45 +578,18 @@ class _Printer(object): else: out.write(str(value)) elif field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_STRING: - embedded_unknown_message = None - if self.print_unknown_fields: - try: - # If this field is parseable as a Message, it is probably - # an embedded message. - # pylint: disable=protected-access - (embedded_unknown_message, pos) = decoder._DecodeUnknownFieldSet( - memoryview(value), 0, len(value)) - if pos != len(value): - embedded_unknown_message = None - except Exception: # pylint: disable=broad-except - pass - if embedded_unknown_message: - if self.as_one_line: - out.write(' { ') - else: - out.write(' {\n') - self.indent += 2 - - self._PrintUnknownFields(embedded_unknown_message) - - if self.as_one_line: - out.write('} ') - else: - self.indent -= 2 - out.write(' ' * self.indent + '}') + out.write('\"') + if isinstance(value, six.text_type) and (six.PY2 or not self.as_utf8): + out_value = value.encode('utf-8') else: - out.write('\"') - if isinstance(value, six.text_type) and (six.PY2 or not self.as_utf8): - out_value = value.encode('utf-8') - else: - out_value = value - if field.type == descriptor.FieldDescriptor.TYPE_BYTES: - # We always need to escape all binary data in TYPE_BYTES fields. - out_as_utf8 = False - else: - out_as_utf8 = self.as_utf8 - out.write(text_encoding.CEscape(out_value, out_as_utf8)) - out.write('\"') + out_value = value + if field.type == descriptor.FieldDescriptor.TYPE_BYTES: + # We always need to escape all binary data in TYPE_BYTES fields. + out_as_utf8 = False + else: + out_as_utf8 = self.as_utf8 + out.write(text_encoding.CEscape(out_value, out_as_utf8)) + out.write('\"') elif field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_BOOL: if value: out.write('true') diff --git a/src/google/protobuf/any.h b/src/google/protobuf/any.h index d7d43966e4..59dd50cb21 100644 --- a/src/google/protobuf/any.h +++ b/src/google/protobuf/any.h @@ -73,7 +73,7 @@ class PROTOBUF_EXPORT AnyMetadata { // Packs a message using the given type URL prefix. The type URL will be // constructed by concatenating the message type's full name to the prefix - // with an optional "/" separator if the prefix doesn't already end up "/". + // with an optional "/" separator if the prefix doesn't already end with "/". // For example, both PackFrom(message, "type.googleapis.com") and // PackFrom(message, "type.googleapis.com/") yield the same result type // URL: "type.googleapis.com/". @@ -96,7 +96,7 @@ class PROTOBUF_EXPORT AnyMetadata { bool UnpackTo(Message* message) const; // Checks whether the type specified in the type URL matches the given type. - // A type is consdiered matching if its full name matches the full name after + // A type is considered matching if its full name matches the full name after // the last "/" in the type URL. template bool Is() const { diff --git a/src/google/protobuf/any.pb.cc b/src/google/protobuf/any.pb.cc index 898987bdb7..3ccd92167b 100644 --- a/src/google/protobuf/any.pb.cc +++ b/src/google/protobuf/any.pb.cc @@ -85,18 +85,6 @@ PROTOBUF_NAMESPACE_OPEN void Any::InitAsDefaultInstance() { } -void Any::PackFrom(const ::PROTOBUF_NAMESPACE_ID::Message& message) { - _any_metadata_.PackFrom(message); -} - -void Any::PackFrom(const ::PROTOBUF_NAMESPACE_ID::Message& message, - const std::string& type_url_prefix) { - _any_metadata_.PackFrom(message, type_url_prefix); -} - -bool Any::UnpackTo(::PROTOBUF_NAMESPACE_ID::Message* message) const { - return _any_metadata_.UnpackTo(message); -} bool Any::GetAnyFieldDescriptors( const ::PROTOBUF_NAMESPACE_ID::Message& message, const ::PROTOBUF_NAMESPACE_ID::FieldDescriptor** type_url_field, diff --git a/src/google/protobuf/any.pb.h b/src/google/protobuf/any.pb.h index d85bb6c436..b858c92cfe 100644 --- a/src/google/protobuf/any.pb.h +++ b/src/google/protobuf/any.pb.h @@ -113,14 +113,32 @@ class PROTOBUF_EXPORT Any : // implements Any ----------------------------------------------- - void PackFrom(const ::PROTOBUF_NAMESPACE_ID::Message& message); + void PackFrom(const ::PROTOBUF_NAMESPACE_ID::Message& message) { + _any_metadata_.PackFrom(message); + } void PackFrom(const ::PROTOBUF_NAMESPACE_ID::Message& message, - const std::string& type_url_prefix); - bool UnpackTo(::PROTOBUF_NAMESPACE_ID::Message* message) const; + const std::string& type_url_prefix) { + _any_metadata_.PackFrom(message, type_url_prefix); + } + bool UnpackTo(::PROTOBUF_NAMESPACE_ID::Message* message) const { + return _any_metadata_.UnpackTo(message); + } static bool GetAnyFieldDescriptors( const ::PROTOBUF_NAMESPACE_ID::Message& message, const ::PROTOBUF_NAMESPACE_ID::FieldDescriptor** type_url_field, const ::PROTOBUF_NAMESPACE_ID::FieldDescriptor** value_field); + template ::value>::type> + void PackFrom(const T& message) { + _any_metadata_.PackFrom(message); + } + template ::value>::type> + void PackFrom(const T& message, + const std::string& type_url_prefix) { + _any_metadata_.PackFrom(message, type_url_prefix);} + template ::value>::type> + bool UnpackTo(T* message) const { + return _any_metadata_.UnpackTo(message); + } template bool Is() const { return _any_metadata_.Is(); } diff --git a/src/google/protobuf/arena.h b/src/google/protobuf/arena.h index 4a3b2dc99c..736b4e4081 100644 --- a/src/google/protobuf/arena.h +++ b/src/google/protobuf/arena.h @@ -69,7 +69,6 @@ struct ArenaOptions; // defined below } // namespace protobuf } // namespace google - namespace google { namespace protobuf { diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index d1e42517d3..1fbc34e235 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -263,8 +263,8 @@ void AddDefaultProtoPaths( } } -string PluginName(const std::string& plugin_prefix, - const std::string& directive) { +std::string PluginName(const std::string& plugin_prefix, + const std::string& directive) { // Assuming the directive starts with "--" and ends with "_out" or "_opt", // strip the "--" and "_out/_opt" and add the plugin prefix. return plugin_prefix + "gen-" + directive.substr(2, directive.size() - 6); diff --git a/src/google/protobuf/compiler/command_line_interface.h b/src/google/protobuf/compiler/command_line_interface.h index 49d7259436..1a9b66ad2f 100644 --- a/src/google/protobuf/compiler/command_line_interface.h +++ b/src/google/protobuf/compiler/command_line_interface.h @@ -153,7 +153,7 @@ class PROTOC_EXPORT CommandLineInterface { // The compiler determines the executable name to search for by concatenating // exe_name_prefix with the unrecognized flag name, removing "_out". So, for // example, if exe_name_prefix is "protoc-" and you pass the flag --foo_out, - // the compiler will try to run the program "protoc-foo". + // the compiler will try to run the program "protoc-gen-foo". // // The plugin program should implement the following usage: // plugin [--out=OUTDIR] [--parameter=PARAMETER] PROTO_FILES < DESCRIPTORS diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index 6a9dec4f1c..b3c014ccf3 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -52,6 +52,7 @@ #include #include #include +#include #include #include #include @@ -659,6 +660,73 @@ TEST_F(CommandLineInterfaceTest, MultipleInputs_DescriptorSetIn) { "bar.proto", "Bar"); } +TEST_F(CommandLineInterfaceTest, MultipleInputs_UnusedImport_DescriptorSetIn) { + // Test unused import warning is not raised when descriptor_set_in is called + // and custom options are in unknown field instead of uninterpreted_options. + FileDescriptorSet file_descriptor_set; + + const FileDescriptor* descriptor_file = + FileDescriptorProto::descriptor()->file(); + descriptor_file->CopyTo(file_descriptor_set.add_file()); + + const FileDescriptor* custom_file = + protobuf_unittest::AggregateMessage::descriptor()->file(); + FileDescriptorProto* file_descriptor_proto = file_descriptor_set.add_file(); + custom_file->CopyTo(file_descriptor_proto); + file_descriptor_proto->set_name("custom_options.proto"); + // Add a custom message option. + FieldDescriptorProto* extension_option = + file_descriptor_proto->add_extension(); + extension_option->set_name("unknown_option"); + extension_option->set_extendee(".google.protobuf.MessageOptions"); + extension_option->set_number(1111); + extension_option->set_label(FieldDescriptorProto::LABEL_OPTIONAL); + extension_option->set_type(FieldDescriptorProto::TYPE_INT64); + + file_descriptor_proto = file_descriptor_set.add_file(); + file_descriptor_proto->set_name("import_custom_unknown_options.proto"); + file_descriptor_proto->add_dependency("custom_options.proto"); + // Add custom message option to unknown field. This custom option is + // not known in generated pool, thus option will be in unknown fields. + file_descriptor_proto->add_message_type()->set_name("Bar"); + file_descriptor_proto->mutable_message_type(0) + ->mutable_options() + ->mutable_unknown_fields() + ->AddVarint(1111, 2222); + + WriteDescriptorSet("foo.bin", &file_descriptor_set); + + Run("protocol_compiler --test_out=$tmpdir --plug_out=$tmpdir " + "--descriptor_set_in=$tmpdir/foo.bin " + "import_custom_unknown_options.proto"); + + // TODO(jieluo): Fix this test. This test case only happens when + // CommandLineInterface::Run() is used instead of invoke protoc combined + // with descriptor_set_in, and same custom options are defined in both + // generated pool and descriptor_set_in. There's no such uages for now but + // still need to be fixed. + /* + file_descriptor_proto = file_descriptor_set.add_file(); + file_descriptor_proto->set_name("import_custom_extension_options.proto"); + file_descriptor_proto->add_dependency("custom_options.proto"); + // Add custom message option to unknown field. This custom option is + // also defined in generated pool, thus option will be in extensions. + file_descriptor_proto->add_message_type()->set_name("Foo"); + file_descriptor_proto->mutable_message_type(0) + ->mutable_options() + ->mutable_unknown_fields() + ->AddVarint(protobuf_unittest::message_opt1.number(), 2222); + + WriteDescriptorSet("foo.bin", &file_descriptor_set); + + Run("protocol_compiler --test_out=$tmpdir --plug_out=$tmpdir " + "--descriptor_set_in=$tmpdir/foo.bin import_custom_unknown_options.proto " + "import_custom_extension_options.proto"); + */ + + ExpectNoErrors(); +} + TEST_F(CommandLineInterfaceTest, MultipleInputsWithImport) { // Test parsing multiple input files with an import of a separate file. diff --git a/src/google/protobuf/compiler/cpp/cpp_bootstrap_unittest.cc b/src/google/protobuf/compiler/cpp/cpp_bootstrap_unittest.cc index f14074d91c..dbef855927 100755 --- a/src/google/protobuf/compiler/cpp/cpp_bootstrap_unittest.cc +++ b/src/google/protobuf/compiler/cpp/cpp_bootstrap_unittest.cc @@ -157,7 +157,8 @@ TEST(BootstrapTest, GeneratedFilesMatch) { const FileDescriptor* file = importer.Import(file_parameter[0] + std::string(".proto")); ASSERT_TRUE(file != nullptr) - << "Can't import file " << file_parameter[0] + string(".proto") << "\n"; + << "Can't import file " << file_parameter[0] + std::string(".proto") + << "\n"; EXPECT_EQ("", error_collector.text_); CppGenerator generator; MockGeneratorContext context; @@ -183,7 +184,7 @@ TEST(BootstrapTest, OptionNotExist) { DescriptorPool pool; GeneratorContext* generator_context = nullptr; std::string parameter = "aaa"; - string error; + std::string error; ASSERT_FALSE(generator.Generate( pool.FindFileByName("google/protobuf/descriptor.proto"), parameter, generator_context, &error)); diff --git a/src/google/protobuf/compiler/cpp/cpp_helpers.cc b/src/google/protobuf/compiler/cpp/cpp_helpers.cc index ca8bcba6d1..9f0693bed2 100644 --- a/src/google/protobuf/compiler/cpp/cpp_helpers.cc +++ b/src/google/protobuf/compiler/cpp/cpp_helpers.cc @@ -406,7 +406,7 @@ std::string SuperClassName(const Descriptor* descriptor, : "::MessageLite"); } -std::string ResolveKeyword(const string& name) { +std::string ResolveKeyword(const std::string& name) { if (kKeywords.count(name) > 0) { return name + "_"; } @@ -665,7 +665,7 @@ std::string DefaultValue(const Options& options, const FieldDescriptor* field) { // If floating point value contains a period (.) or an exponent // (either E or e), then append suffix 'f' to make it a float // literal. - if (float_value.find_first_of(".eE") != string::npos) { + if (float_value.find_first_of(".eE") != std::string::npos) { float_value.push_back('f'); } return float_value; @@ -709,8 +709,8 @@ std::string FilenameIdentifier(const std::string& filename) { return result; } -string UniqueName(const std::string& name, const std::string& filename, - const Options& options) { +std::string UniqueName(const std::string& name, const std::string& filename, + const Options& options) { return name + "_" + FilenameIdentifier(filename); } @@ -1428,7 +1428,7 @@ class ParseLoopGenerator { if (HasFieldPresence(field->file())) { format_("_Internal::set_has_$1$(&$has_bits$);\n", FieldName(field)); } - string default_string = + std::string default_string = field->default_value_string().empty() ? "::" + ProtobufNamespace(options_) + "::internal::GetEmptyStringAlreadyInited()" @@ -1635,7 +1635,7 @@ class ParseLoopGenerator { zigzag = StrCat("ZigZag", size); } if (field->is_repeated() || field->containing_oneof()) { - string prefix = field->is_repeated() ? "add" : "set"; + std::string prefix = field->is_repeated() ? "add" : "set"; format_( "_internal_$1$_$2$($pi_ns$::ReadVarint$3$(&ptr));\n" "CHK_(ptr);\n", @@ -1657,7 +1657,7 @@ class ParseLoopGenerator { case WireFormatLite::WIRETYPE_FIXED64: { std::string type = PrimitiveTypeName(options_, field->cpp_type()); if (field->is_repeated() || field->containing_oneof()) { - string prefix = field->is_repeated() ? "add" : "set"; + std::string prefix = field->is_repeated() ? "add" : "set"; format_( "_internal_$1$_$2$($pi_ns$::UnalignedLoad<$3$>(ptr));\n" "ptr += sizeof($3$);\n", @@ -1730,21 +1730,8 @@ class ParseLoopGenerator { format_.Indent(); for (const auto* field : ordered_fields) { - // Print the field's (or oneof's) proto-syntax definition as a comment. - // We don't want to print group bodies so we cut off after the first - // line. - std::string def; - { - DebugStringOptions options; - options.elide_group_body = true; - options.elide_oneof_body = true; - def = field->DebugStringWithOptions(options); - def = def.substr(0, def.find_first_of('\n')); - } - format_( - "// $1$\n" - "case $2$:\n", - def, field->number()); + PrintFieldComment(format_, field); + format_("case $1$:\n", field->number()); format_.Indent(); uint32 fallback_tag = 0; uint32 expected_tag = ExpectedTag(field, &fallback_tag); diff --git a/src/google/protobuf/compiler/cpp/cpp_helpers.h b/src/google/protobuf/compiler/cpp/cpp_helpers.h index 3537bd1fc2..2f11b50a07 100644 --- a/src/google/protobuf/compiler/cpp/cpp_helpers.h +++ b/src/google/protobuf/compiler/cpp/cpp_helpers.h @@ -159,7 +159,7 @@ std::string SuperClassName(const Descriptor* descriptor, const Options& options); // Adds an underscore if necessary to prevent conflicting with a keyword. -std::string ResolveKeyword(const string& name); +std::string ResolveKeyword(const std::string& name); // Get the (unqualified) name that should be used for this field in C++ code. // The name is coerced to lower-case to emulate proto1 behavior. People @@ -703,6 +703,18 @@ class PROTOC_EXPORT Formatter { } }; +template +void PrintFieldComment(const Formatter& format, const T* field) { + // Print the field's (or oneof's) proto-syntax definition as a comment. + // We don't want to print group bodies so we cut off after the first + // line. + DebugStringOptions options; + options.elide_group_body = true; + options.elide_oneof_body = true; + std::string def = field->DebugStringWithOptions(options); + format("// $1$\n", def.substr(0, def.find_first_of('\n'))); +} + class PROTOC_EXPORT NamespaceOpener { public: explicit NamespaceOpener(const Formatter& format) diff --git a/src/google/protobuf/compiler/cpp/cpp_message.cc b/src/google/protobuf/compiler/cpp/cpp_message.cc index b83e71bbbd..ec367a08a3 100644 --- a/src/google/protobuf/compiler/cpp/cpp_message.cc +++ b/src/google/protobuf/compiler/cpp/cpp_message.cc @@ -68,18 +68,6 @@ using internal::WireFormatLite; namespace { -template -void PrintFieldComment(const Formatter& format, const T* field) { - // Print the field's (or oneof's) proto-syntax definition as a comment. - // We don't want to print group bodies so we cut off after the first - // line. - DebugStringOptions options; - options.elide_group_body = true; - options.elide_oneof_body = true; - std::string def = field->DebugStringWithOptions(options); - format("// $1$\n", def.substr(0, def.find_first_of('\n'))); -} - void PrintPresenceCheck(const Formatter& format, const FieldDescriptor* field, const std::vector& has_bit_indices, io::Printer* printer, int* cached_has_bit_index) { @@ -1231,14 +1219,39 @@ void MessageGenerator::GenerateClassDefinition(io::Printer* printer) { "\n"); if (HasDescriptorMethods(descriptor_->file(), options_)) { format( - "void PackFrom(const ::$proto_ns$::Message& message);\n" + "void PackFrom(const ::$proto_ns$::Message& message) {\n" + " _any_metadata_.PackFrom(message);\n" + "}\n" "void PackFrom(const ::$proto_ns$::Message& message,\n" - " const std::string& type_url_prefix);\n" - "bool UnpackTo(::$proto_ns$::Message* message) const;\n" + " const std::string& type_url_prefix) {\n" + " _any_metadata_.PackFrom(message, type_url_prefix);\n" + "}\n" + "bool UnpackTo(::$proto_ns$::Message* message) const {\n" + " return _any_metadata_.UnpackTo(message);\n" + "}\n" "static bool GetAnyFieldDescriptors(\n" " const ::$proto_ns$::Message& message,\n" " const ::$proto_ns$::FieldDescriptor** type_url_field,\n" - " const ::$proto_ns$::FieldDescriptor** value_field);\n"); + " const ::$proto_ns$::FieldDescriptor** value_field);\n" + "template " + "::value>::type>\n" + "void PackFrom(const T& message) {\n" + " _any_metadata_.PackFrom(message);\n" + "}\n" + "template " + "::value>::type>\n" + "void PackFrom(const T& message,\n" + " const std::string& type_url_prefix) {\n" + " _any_metadata_.PackFrom(message, type_url_prefix);" + "}\n" + "template " + "::value>::type>\n" + "bool UnpackTo(T* message) const {\n" + " return _any_metadata_.UnpackTo(message);\n" + "}\n"); } else { format( "template \n" @@ -1248,7 +1261,7 @@ void MessageGenerator::GenerateClassDefinition(io::Printer* printer) { "template \n" "void PackFrom(const T& message,\n" " const std::string& type_url_prefix) {\n" - " _any_metadata_.PackFrom(message, type_url_prefix);" + " _any_metadata_.PackFrom(message, type_url_prefix);\n" "}\n" "template \n" "bool UnpackTo(T* message) const {\n" @@ -2041,18 +2054,6 @@ void MessageGenerator::GenerateClassMethods(io::Printer* printer) { if (IsAnyMessage(descriptor_, options_)) { if (HasDescriptorMethods(descriptor_->file(), options_)) { format( - "void $classname$::PackFrom(const ::$proto_ns$::Message& message) {\n" - " _any_metadata_.PackFrom(message);\n" - "}\n" - "\n" - "void $classname$::PackFrom(const ::$proto_ns$::Message& message,\n" - " const std::string& type_url_prefix) {\n" - " _any_metadata_.PackFrom(message, type_url_prefix);\n" - "}\n" - "\n" - "bool $classname$::UnpackTo(::$proto_ns$::Message* message) const {\n" - " return _any_metadata_.UnpackTo(message);\n" - "}\n" "bool $classname$::GetAnyFieldDescriptors(\n" " const ::$proto_ns$::Message& message,\n" " const ::$proto_ns$::FieldDescriptor** type_url_field,\n" diff --git a/src/google/protobuf/compiler/cpp/cpp_message_field.cc b/src/google/protobuf/compiler/cpp/cpp_message_field.cc index 8ccb4d15df..6c5a017158 100644 --- a/src/google/protobuf/compiler/cpp/cpp_message_field.cc +++ b/src/google/protobuf/compiler/cpp/cpp_message_field.cc @@ -44,8 +44,9 @@ namespace compiler { namespace cpp { namespace { -string ReinterpretCast(const string& type, const string& expression, - bool implicit_weak_field) { +std::string ReinterpretCast(const std::string& type, + const std::string& expression, + bool implicit_weak_field) { if (implicit_weak_field) { return "reinterpret_cast< " + type + " >(" + expression + ")"; } else { diff --git a/src/google/protobuf/compiler/cpp/metadata_test.cc b/src/google/protobuf/compiler/cpp/metadata_test.cc index 045c6a3d29..a16c8b530f 100644 --- a/src/google/protobuf/compiler/cpp/metadata_test.cc +++ b/src/google/protobuf/compiler/cpp/metadata_test.cc @@ -132,7 +132,7 @@ TEST_F(CppMetadataTest, AddsPragma) { atu::AddFile("test.proto", kSmallTestFile); EXPECT_TRUE( CaptureMetadata("test.proto", &file, &pb_h, &info, NULL, NULL, NULL)); - EXPECT_TRUE(pb_h.find("#ifdef guard_name") != string::npos); + EXPECT_TRUE(pb_h.find("#ifdef guard_name") != std::string::npos); EXPECT_TRUE(pb_h.find("#pragma pragma_name \"test.pb.h.meta\"") != std::string::npos); } diff --git a/src/google/protobuf/compiler/csharp/csharp_field_base.cc b/src/google/protobuf/compiler/csharp/csharp_field_base.cc index 765bb0e317..d23372d4b0 100644 --- a/src/google/protobuf/compiler/csharp/csharp_field_base.cc +++ b/src/google/protobuf/compiler/csharp/csharp_field_base.cc @@ -306,7 +306,9 @@ std::string FieldGeneratorBase::GetStringDefaultValueInternal(const FieldDescrip if (descriptor->default_value_string().empty()) return "\"\""; else - return "global::System.Text.Encoding.UTF8.GetString(global::System.Convert.FromBase64String(\"" + StringToBase64(descriptor->default_value_string()) + "\"))"; + return "global::System.Text.Encoding.UTF8.GetString(global::System." + "Convert.FromBase64String(\"" + + StringToBase64(descriptor->default_value_string()) + "\"))"; } std::string FieldGeneratorBase::GetBytesDefaultValueInternal(const FieldDescriptor* descriptor) { diff --git a/src/google/protobuf/compiler/csharp/csharp_helpers.cc b/src/google/protobuf/compiler/csharp/csharp_helpers.cc index 514eb9b0ba..98aa246c28 100644 --- a/src/google/protobuf/compiler/csharp/csharp_helpers.cc +++ b/src/google/protobuf/compiler/csharp/csharp_helpers.cc @@ -288,14 +288,18 @@ uint GetGroupEndTag(const Descriptor* descriptor) { const FieldDescriptor* field; for (int i = 0; i < containing_type->field_count(); i++) { field = containing_type->field(i); - if (field->type() == FieldDescriptor::Type::TYPE_GROUP && field->message_type() == descriptor) { - return internal::WireFormatLite::MakeTag(field->number(), internal::WireFormatLite::WIRETYPE_END_GROUP); + if (field->type() == FieldDescriptor::Type::TYPE_GROUP && + field->message_type() == descriptor) { + return internal::WireFormatLite::MakeTag( + field->number(), internal::WireFormatLite::WIRETYPE_END_GROUP); } } for (int i = 0; i < containing_type->extension_count(); i++) { field = containing_type->extension(i); - if (field->type() == FieldDescriptor::Type::TYPE_GROUP && field->message_type() == descriptor) { - return internal::WireFormatLite::MakeTag(field->number(), internal::WireFormatLite::WIRETYPE_END_GROUP); + if (field->type() == FieldDescriptor::Type::TYPE_GROUP && + field->message_type() == descriptor) { + return internal::WireFormatLite::MakeTag( + field->number(), internal::WireFormatLite::WIRETYPE_END_GROUP); } } } else { @@ -304,8 +308,10 @@ uint GetGroupEndTag(const Descriptor* descriptor) { const FieldDescriptor* field; for (int i = 0; i < containing_file->extension_count(); i++) { field = containing_file->extension(i); - if (field->type() == FieldDescriptor::Type::TYPE_GROUP && field->message_type() == descriptor) { - return internal::WireFormatLite::MakeTag(field->number(), internal::WireFormatLite::WIRETYPE_END_GROUP); + if (field->type() == FieldDescriptor::Type::TYPE_GROUP && + field->message_type() == descriptor) { + return internal::WireFormatLite::MakeTag( + field->number(), internal::WireFormatLite::WIRETYPE_END_GROUP); } } } diff --git a/src/google/protobuf/compiler/csharp/csharp_message.cc b/src/google/protobuf/compiler/csharp/csharp_message.cc index fe2421a80e..4247b32d60 100644 --- a/src/google/protobuf/compiler/csharp/csharp_message.cc +++ b/src/google/protobuf/compiler/csharp/csharp_message.cc @@ -150,7 +150,10 @@ void MessageGenerator::Generate(io::Printer* printer) { printer->Print(vars, "private pb::ExtensionSet<$class_name$> _extensions;\n"); } - printer->Print(vars, "private pb::ExtensionSet<$class_name$> _Extensions => _extensions;\n"); // a read-only property for fast retrieval of the set in IsInitialized + printer->Print(vars, + "private pb::ExtensionSet<$class_name$> _Extensions => " + "_extensions;\n"); // a read-only property for fast + // retrieval of the set in IsInitialized } for (int i = 0; i < has_bit_field_count_; i++) { @@ -265,28 +268,39 @@ void MessageGenerator::Generate(io::Printer* printer) { if (has_extension_ranges_) { printer->Print( - vars, - "public TValue GetExtension(pb::Extension<$class_name$, TValue> extension) {\n" - " return pb::ExtensionSet.Get(ref _extensions, extension);\n" - "}\n" - "public pbc::RepeatedField GetExtension(pb::RepeatedExtension<$class_name$, TValue> extension) {\n" - " return pb::ExtensionSet.Get(ref _extensions, extension);\n" - "}\n" - "public pbc::RepeatedField GetOrInitializeExtension(pb::RepeatedExtension<$class_name$, TValue> extension) {\n" - " return pb::ExtensionSet.GetOrInitialize(ref _extensions, extension);\n" - "}\n" - "public void SetExtension(pb::Extension<$class_name$, TValue> extension, TValue value) {\n" - " pb::ExtensionSet.Set(ref _extensions, extension, value);\n" - "}\n" - "public bool HasExtension(pb::Extension<$class_name$, TValue> extension) {\n" - " return pb::ExtensionSet.Has(ref _extensions, extension);\n" - "}\n" - "public void ClearExtension(pb::Extension<$class_name$, TValue> extension) {\n" - " pb::ExtensionSet.Clear(ref _extensions, extension);\n" - "}\n" - "public void ClearExtension(pb::RepeatedExtension<$class_name$, TValue> extension) {\n" - " pb::ExtensionSet.Clear(ref _extensions, extension);\n" - "}\n\n"); + vars, + "public TValue GetExtension(pb::Extension<$class_name$, " + "TValue> extension) {\n" + " return pb::ExtensionSet.Get(ref _extensions, extension);\n" + "}\n" + "public pbc::RepeatedField " + "GetExtension(pb::RepeatedExtension<$class_name$, TValue> " + "extension) {\n" + " return pb::ExtensionSet.Get(ref _extensions, extension);\n" + "}\n" + "public pbc::RepeatedField " + "GetOrInitializeExtension(pb::RepeatedExtension<$class_name$, " + "TValue> extension) {\n" + " return pb::ExtensionSet.GetOrInitialize(ref _extensions, " + "extension);\n" + "}\n" + "public void SetExtension(pb::Extension<$class_name$, TValue> " + "extension, TValue value) {\n" + " pb::ExtensionSet.Set(ref _extensions, extension, value);\n" + "}\n" + "public bool HasExtension(pb::Extension<$class_name$, TValue> " + "extension) {\n" + " return pb::ExtensionSet.Has(ref _extensions, extension);\n" + "}\n" + "public void ClearExtension(pb::Extension<$class_name$, " + "TValue> extension) {\n" + " pb::ExtensionSet.Clear(ref _extensions, extension);\n" + "}\n" + "public void " + "ClearExtension(pb::RepeatedExtension<$class_name$, TValue> " + "extension) {\n" + " pb::ExtensionSet.Clear(ref _extensions, extension);\n" + "}\n\n"); } // Nested messages and enums @@ -627,9 +641,9 @@ void MessageGenerator::GenerateMergingMethods(io::Printer* printer) { printer->Indent(); if (end_tag_ != 0) { printer->Print( - "case $end_tag$:\n" - " return;\n", - "end_tag", StrCat(end_tag_)); + "case $end_tag$:\n" + " return;\n", + "end_tag", StrCat(end_tag_)); } if (has_extension_ranges_) { printer->Print( @@ -683,7 +697,8 @@ void MessageGenerator::GenerateMergingMethods(io::Printer* printer) { // it's a waste of space to track presence for all values, so we only track them if they're not nullable int MessageGenerator::GetPresenceIndex(const FieldDescriptor* descriptor) { - if (IsNullable(descriptor) || !IsProto2(descriptor->file()) || descriptor->is_extension()) { + if (IsNullable(descriptor) || !IsProto2(descriptor->file()) || + descriptor->is_extension()) { return -1; } diff --git a/src/google/protobuf/compiler/csharp/csharp_reflection_class.cc b/src/google/protobuf/compiler/csharp/csharp_reflection_class.cc index 08b856a110..37154e3ce8 100644 --- a/src/google/protobuf/compiler/csharp/csharp_reflection_class.cc +++ b/src/google/protobuf/compiler/csharp/csharp_reflection_class.cc @@ -73,10 +73,10 @@ void ReflectionClassGenerator::Generate(io::Printer* printer) { if (file_->extension_count() > 0) { printer->Print( - "///

Holder for extension identifiers generated from the top level of $file_name$\n" + "/// Holder for extension identifiers generated from the top " + "level of $file_name$\n" "$access_level$ static partial class $class_name$ {\n", - "access_level", class_access_level(), - "class_name", extensionClassname_, + "access_level", class_access_level(), "class_name", extensionClassname_, "file_name", file_->name()); printer->Indent(); for (int i = 0; i < file_->extension_count(); i++) { diff --git a/src/google/protobuf/compiler/importer.cc b/src/google/protobuf/compiler/importer.cc index 9308519470..400d5d07e8 100644 --- a/src/google/protobuf/compiler/importer.cc +++ b/src/google/protobuf/compiler/importer.cc @@ -311,7 +311,7 @@ static std::string CanonicalizePath(std::string path) { static inline bool ContainsParentReference(const std::string& path) { return path == ".." || HasPrefixString(path, "../") || - HasSuffixString(path, "/..") || path.find("/../") != string::npos; + HasSuffixString(path, "/..") || path.find("/../") != std::string::npos; } // Maps a file from an old location to a new one. Typically, old_prefix is diff --git a/src/google/protobuf/compiler/java/java_enum_field_lite.cc b/src/google/protobuf/compiler/java/java_enum_field_lite.cc index e4b97e6349..5c137ca50c 100644 --- a/src/google/protobuf/compiler/java/java_enum_field_lite.cc +++ b/src/google/protobuf/compiler/java/java_enum_field_lite.cc @@ -116,6 +116,10 @@ void SetEnumVariables(const FieldDescriptor* descriptor, int messageBitIndex, } else { (*variables)["unknown"] = (*variables)["default"]; } + + // We use `x.getClass()` as a null check because it generates less bytecode + // than an `if (x == null) { throw ... }` statement. + (*variables)["null_check"] = "value.getClass();\n"; } } // namespace @@ -200,11 +204,8 @@ void ImmutableEnumFieldLiteGenerator::GenerateMembers( WriteFieldAccessorDocComment(printer, descriptor_, SETTER); printer->Print(variables_, "private void set$capitalized_name$($type$ value) {\n" - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n" - " $set_has_field_bit_message$\n" " $name$_ = value.getNumber();\n" + " $set_has_field_bit_message$\n" "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER); printer->Print(variables_, @@ -366,11 +367,8 @@ void ImmutableEnumOneofFieldLiteGenerator::GenerateMembers( WriteFieldAccessorDocComment(printer, descriptor_, SETTER); printer->Print(variables_, "private void set$capitalized_name$($type$ value) {\n" - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n" - " $set_oneof_case_message$;\n" " $oneof_name$_ = value.getNumber();\n" + " $set_oneof_case_message$;\n" "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER); printer->Print(variables_, @@ -583,18 +581,14 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateMembers( printer->Print(variables_, "private void set$capitalized_name$(\n" " int index, $type$ value) {\n" - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n" + " $null_check$" " ensure$capitalized_name$IsMutable();\n" " $name$_.setInt(index, value.getNumber());\n" "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER); printer->Print(variables_, "private void add$capitalized_name$($type$ value) {\n" - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n" + " $null_check$" " ensure$capitalized_name$IsMutable();\n" " $name$_.addInt(value.getNumber());\n" "}\n"); diff --git a/src/google/protobuf/compiler/java/java_file.cc b/src/google/protobuf/compiler/java/java_file.cc index a9180c9af4..4a96a365d4 100644 --- a/src/google/protobuf/compiler/java/java_file.cc +++ b/src/google/protobuf/compiler/java/java_file.cc @@ -267,6 +267,7 @@ void FileGenerator::Generate(io::Printer* printer) { } PrintGeneratedAnnotation( printer, '$', options_.annotate_code ? classname_ + ".java.pb.meta" : ""); + printer->Print( "$deprecation$public final class $classname$ {\n" " private $ctor$() {}\n", diff --git a/src/google/protobuf/compiler/java/java_map_field_lite.cc b/src/google/protobuf/compiler/java/java_map_field_lite.cc index 9055774844..8236f3a45c 100644 --- a/src/google/protobuf/compiler/java/java_map_field_lite.cc +++ b/src/google/protobuf/compiler/java/java_map_field_lite.cc @@ -92,14 +92,12 @@ void SetMessageVariables(const FieldDescriptor* descriptor, int messageBitIndex, (*variables)["boxed_key_type"] = TypeName(key, name_resolver, true); (*variables)["key_wire_type"] = WireType(key); (*variables)["key_default_value"] = DefaultValue(key, true, name_resolver); + // We use `x.getClass()` as a null check because it generates less bytecode + // than an `if (x == null) { throw ... }` statement. (*variables)["key_null_check"] = - IsReferenceType(keyJavaType) - ? "if (key == null) { throw new java.lang.NullPointerException(); }" - : ""; + IsReferenceType(keyJavaType) ? "key.getClass();" : ""; (*variables)["value_null_check"] = - IsReferenceType(valueJavaType) - ? "if (value == null) { throw new java.lang.NullPointerException(); }" - : ""; + IsReferenceType(valueJavaType) ? "value.getClass();" : ""; if (GetJavaType(value) == JAVATYPE_ENUM) { // We store enums as Integers internally. diff --git a/src/google/protobuf/compiler/java/java_message.cc b/src/google/protobuf/compiler/java/java_message.cc index 16d03e91de..dbf62e58ff 100644 --- a/src/google/protobuf/compiler/java/java_message.cc +++ b/src/google/protobuf/compiler/java/java_message.cc @@ -999,7 +999,8 @@ void ImmutableMessageGenerator::GenerateEqualsAndHashCode( io::Printer* printer) { printer->Print( "@java.lang.Override\n" - "public boolean equals(final java.lang.Object obj) {\n"); + "public boolean equals("); + printer->Print("final java.lang.Object obj) {\n"); printer->Indent(); printer->Print( "if (obj == this) {\n" diff --git a/src/google/protobuf/compiler/java/java_message_field_lite.cc b/src/google/protobuf/compiler/java/java_message_field_lite.cc index b111099152..cb5f3c0e09 100644 --- a/src/google/protobuf/compiler/java/java_message_field_lite.cc +++ b/src/google/protobuf/compiler/java/java_message_field_lite.cc @@ -96,6 +96,10 @@ void SetMessageVariables(const FieldDescriptor* descriptor, int messageBitIndex, GenerateGetBitFromLocal(builderBitIndex); (*variables)["set_has_field_bit_to_local"] = GenerateSetBitToLocal(messageBitIndex); + + // We use `x.getClass()` as a null check because it generates less bytecode + // than an `if (x == null) { throw ... }` statement. + (*variables)["null_check"] = "value.getClass();\n"; } } // namespace @@ -172,31 +176,18 @@ void ImmutableMessageFieldLiteGenerator::GenerateMembers( WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, "private void set$capitalized_name$($type$ value) {\n" - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n" + " $null_check$" " $name$_ = value;\n" " $set_has_field_bit_message$\n" " }\n"); - // Field.Builder setField(Field.Builder builderForValue) - WriteFieldDocComment(printer, descriptor_); - printer->Print(variables_, - "private void set$capitalized_name$(\n" - " $type$.Builder builderForValue) {\n" - " $name$_ = builderForValue.build();\n" - " $set_has_field_bit_message$\n" - "}\n"); - // Field.Builder mergeField(Field value) WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, "@java.lang.SuppressWarnings({\"ReferenceEquality\"})\n" "private void merge$capitalized_name$($type$ value) {\n" - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n" + " $null_check$" " if ($name$_ != null &&\n" " $name$_ != $type$.getDefaultInstance()) {\n" " $name$_ =\n" @@ -256,7 +247,7 @@ void ImmutableMessageFieldLiteGenerator::GenerateBuilderMembers( "$deprecation$public Builder ${$set$capitalized_name$$}$(\n" " $type$.Builder builderForValue) {\n" " copyOnWrite();\n" - " instance.set$capitalized_name$(builderForValue);\n" + " instance.set$capitalized_name$(builderForValue.build());\n" " return this;\n" "}\n"); printer->Annotate("{", "}", descriptor_); @@ -283,7 +274,6 @@ void ImmutableMessageFieldLiteGenerator::GenerateBuilderMembers( printer->Annotate("{", "}", descriptor_); } - void ImmutableMessageFieldLiteGenerator::GenerateFieldInfo( io::Printer* printer, std::vector* output) const { WriteIntToUtf16CharSequence(descriptor_->number(), output); @@ -342,30 +332,17 @@ void ImmutableMessageOneofFieldLiteGenerator::GenerateMembers( WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, "private void set$capitalized_name$($type$ value) {\n" - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n" + " $null_check$" " $oneof_name$_ = value;\n" " $set_oneof_case_message$;\n" "}\n"); - // Field.Builder setField(Field.Builder builderForValue) - WriteFieldDocComment(printer, descriptor_); - printer->Print(variables_, - "private void set$capitalized_name$(\n" - " $type$.Builder builderForValue) {\n" - " $oneof_name$_ = builderForValue.build();\n" - " $set_oneof_case_message$;\n" - "}\n"); - // Field.Builder mergeField(Field value) WriteFieldDocComment(printer, descriptor_); printer->Print( variables_, "private void merge$capitalized_name$($type$ value) {\n" - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n" + " $null_check$" " if ($has_oneof_case_message$ &&\n" " $oneof_name$_ != $type$.getDefaultInstance()) {\n" " $oneof_name$_ = $type$.newBuilder(($type$) $oneof_name$_)\n" @@ -436,7 +413,7 @@ void ImmutableMessageOneofFieldLiteGenerator::GenerateBuilderMembers( "$deprecation$public Builder ${$set$capitalized_name$$}$(\n" " $type$.Builder builderForValue) {\n" " copyOnWrite();\n" - " instance.set$capitalized_name$(builderForValue);\n" + " instance.set$capitalized_name$(builderForValue.build());\n" " return this;\n" "}\n"); printer->Annotate("{", "}", descriptor_); @@ -563,29 +540,16 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateMembers( printer->Print(variables_, "private void set$capitalized_name$(\n" " int index, $type$ value) {\n" - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n" + " $null_check$" " ensure$capitalized_name$IsMutable();\n" " $name$_.set(index, value);\n" "}\n"); - // Builder setRepeatedField(int index, Field.Builder builderForValue) - WriteFieldDocComment(printer, descriptor_); - printer->Print(variables_, - "private void set$capitalized_name$(\n" - " int index, $type$.Builder builderForValue) {\n" - " ensure$capitalized_name$IsMutable();\n" - " $name$_.set(index, builderForValue.build());\n" - "}\n"); - // Builder addRepeatedField(Field value) WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, "private void add$capitalized_name$($type$ value) {\n" - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n" + " $null_check$" " ensure$capitalized_name$IsMutable();\n" " $name$_.add(value);\n" "}\n"); @@ -595,29 +559,10 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateMembers( printer->Print(variables_, "private void add$capitalized_name$(\n" " int index, $type$ value) {\n" - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n" + " $null_check$" " ensure$capitalized_name$IsMutable();\n" " $name$_.add(index, value);\n" "}\n"); - // Builder addRepeatedField(Field.Builder builderForValue) - WriteFieldDocComment(printer, descriptor_); - printer->Print(variables_, - "private void add$capitalized_name$(\n" - " $type$.Builder builderForValue) {\n" - " ensure$capitalized_name$IsMutable();\n" - " $name$_.add(builderForValue.build());\n" - "}\n"); - - // Builder addRepeatedField(int index, Field.Builder builderForValue) - WriteFieldDocComment(printer, descriptor_); - printer->Print(variables_, - "private void add$capitalized_name$(\n" - " int index, $type$.Builder builderForValue) {\n" - " ensure$capitalized_name$IsMutable();\n" - " $name$_.add(index, builderForValue.build());\n" - "}\n"); // Builder addAllRepeatedField(Iterable values) WriteFieldDocComment(printer, descriptor_); @@ -698,7 +643,8 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateBuilderMembers( "$deprecation$public Builder ${$set$capitalized_name$$}$(\n" " int index, $type$.Builder builderForValue) {\n" " copyOnWrite();\n" - " instance.set$capitalized_name$(index, builderForValue);\n" + " instance.set$capitalized_name$(index,\n" + " builderForValue.build());\n" " return this;\n" "}\n"); printer->Annotate("{", "}", descriptor_); @@ -730,7 +676,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateBuilderMembers( "$deprecation$public Builder ${$add$capitalized_name$$}$(\n" " $type$.Builder builderForValue) {\n" " copyOnWrite();\n" - " instance.add$capitalized_name$(builderForValue);\n" + " instance.add$capitalized_name$(builderForValue.build());\n" " return this;\n" "}\n"); printer->Annotate("{", "}", descriptor_); @@ -741,7 +687,8 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateBuilderMembers( "$deprecation$public Builder ${$add$capitalized_name$$}$(\n" " int index, $type$.Builder builderForValue) {\n" " copyOnWrite();\n" - " instance.add$capitalized_name$(index, builderForValue);\n" + " instance.add$capitalized_name$(index,\n" + " builderForValue.build());\n" " return this;\n" "}\n"); printer->Annotate("{", "}", descriptor_); @@ -780,7 +727,6 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateBuilderMembers( printer->Annotate("{", "}", descriptor_); } - void RepeatedImmutableMessageFieldLiteGenerator::GenerateFieldInfo( io::Printer* printer, std::vector* output) const { WriteIntToUtf16CharSequence(descriptor_->number(), output); diff --git a/src/google/protobuf/compiler/java/java_primitive_field_lite.cc b/src/google/protobuf/compiler/java/java_primitive_field_lite.cc index 918bdb95c9..f038412bda 100644 --- a/src/google/protobuf/compiler/java/java_primitive_field_lite.cc +++ b/src/google/protobuf/compiler/java/java_primitive_field_lite.cc @@ -124,10 +124,9 @@ void SetPrimitiveVariables(const FieldDescriptor* descriptor, } if (IsReferenceType(javaType)) { - (*variables)["null_check"] = - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n"; + // We use `x.getClass()` as a null check because it generates less bytecode + // than an `if (x == null) { throw ... }` statement. + (*variables)["null_check"] = " value.getClass();\n"; } else { (*variables)["null_check"] = ""; } diff --git a/src/google/protobuf/compiler/java/java_string_field_lite.cc b/src/google/protobuf/compiler/java/java_string_field_lite.cc index 77cea92360..b2f22a5ff7 100644 --- a/src/google/protobuf/compiler/java/java_string_field_lite.cc +++ b/src/google/protobuf/compiler/java/java_string_field_lite.cc @@ -75,10 +75,9 @@ void SetPrimitiveVariables(const FieldDescriptor* descriptor, StrCat(static_cast(WireFormat::MakeTag(descriptor))); (*variables)["tag_size"] = StrCat( WireFormat::TagSize(descriptor->number(), GetType(descriptor))); - (*variables)["null_check"] = - " if (value == null) {\n" - " throw new NullPointerException();\n" - " }\n"; + // We use `x.getClass()` as a null check because it generates less bytecode + // than an `if (x == null) { throw ... }` statement. + (*variables)["null_check"] = " value.getClass();\n"; // TODO(birdo): Add @deprecated javadoc when generating javadoc is supported // by the proto compiler @@ -230,14 +229,13 @@ void ImmutableStringFieldLiteGenerator::GenerateMembers( WriteFieldStringBytesAccessorDocComment(printer, descriptor_, SETTER); printer->Print(variables_, "private void set$capitalized_name$Bytes(\n" - " com.google.protobuf.ByteString value) {\n" - "$null_check$"); + " com.google.protobuf.ByteString value) {\n"); if (CheckUtf8(descriptor_)) { printer->Print(variables_, " checkByteStringIsUtf8(value);\n"); } printer->Print(variables_, - " $set_has_field_bit_message$\n" " $name$_ = value.toStringUtf8();\n" + " $set_has_field_bit_message$\n" "}\n"); } @@ -402,15 +400,14 @@ void ImmutableStringOneofFieldLiteGenerator::GenerateMembers( WriteFieldStringBytesAccessorDocComment(printer, descriptor_, SETTER); printer->Print(variables_, "private void ${$set$capitalized_name$Bytes$}$(\n" - " com.google.protobuf.ByteString value) {\n" - "$null_check$"); + " com.google.protobuf.ByteString value) {\n"); printer->Annotate("{", "}", descriptor_); if (CheckUtf8(descriptor_)) { printer->Print(variables_, " checkByteStringIsUtf8(value);\n"); } printer->Print(variables_, - " $set_oneof_case_message$;\n" " $oneof_name$_ = value.toStringUtf8();\n" + " $set_oneof_case_message$;\n" "}\n"); } @@ -609,8 +606,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateMembers( WriteFieldStringBytesAccessorDocComment(printer, descriptor_, LIST_ADDER); printer->Print(variables_, "private void add$capitalized_name$Bytes(\n" - " com.google.protobuf.ByteString value) {\n" - "$null_check$"); + " com.google.protobuf.ByteString value) {\n"); if (CheckUtf8(descriptor_)) { printer->Print(variables_, " checkByteStringIsUtf8(value);\n"); } diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc index d06fbcf5c1..c1216f8ce9 100644 --- a/src/google/protobuf/compiler/js/js_generator.cc +++ b/src/google/protobuf/compiler/js/js_generator.cc @@ -123,8 +123,8 @@ std::string GetJSFilename(const GeneratorOptions& options, // Given a filename like foo/bar/baz.proto, returns the root directory // path ../../ -string GetRootPath(const std::string& from_filename, - const std::string& to_filename) { +std::string GetRootPath(const std::string& from_filename, + const std::string& to_filename) { if (to_filename.find("google/protobuf") == 0) { // Well-known types (.proto files in the google/protobuf directory) are // assumed to come from the 'google-protobuf' npm package. We may want to @@ -772,7 +772,7 @@ std::string PostProcessFloat(std::string result) { exponent = exponent.substr(1); } - return mantissa + "E" + string(exp_neg ? "-" : "") + exponent; + return mantissa + "E" + std::string(exp_neg ? "-" : "") + exponent; } // Otherwise, this is an ordinary decimal number. Append ".0" if result has no @@ -2292,11 +2292,11 @@ void Generator::GenerateFieldValueExpression(io::Printer* printer, field->cpp_type() == FieldDescriptor::CPPTYPE_DOUBLE; const bool is_boolean = field->cpp_type() == FieldDescriptor::CPPTYPE_BOOL; - const string with_default = use_default ? "WithDefault" : ""; - const string default_arg = + const std::string with_default = use_default ? "WithDefault" : ""; + const std::string default_arg = use_default ? StrCat(", ", JSFieldDefault(field)) : ""; - const string cardinality = field->is_repeated() ? "Repeated" : ""; - string type = ""; + const std::string cardinality = field->is_repeated() ? "Repeated" : ""; + std::string type = ""; if (is_float_or_double) { type = "FloatingPoint"; } @@ -3362,7 +3362,7 @@ void Generator::GenerateEnum(const GeneratorOptions& options, enumdesc->name()); printer->Annotate("name", enumdesc); - std::set used_name; + std::set used_name; std::vector valid_index; for (int i = 0; i < enumdesc->value_count(); i++) { if (enumdesc->options().allow_alias() && diff --git a/src/google/protobuf/compiler/mock_code_generator.cc b/src/google/protobuf/compiler/mock_code_generator.cc index bd75869845..e88b18bea7 100644 --- a/src/google/protobuf/compiler/mock_code_generator.cc +++ b/src/google/protobuf/compiler/mock_code_generator.cc @@ -149,7 +149,7 @@ void CheckSingleAnnotation(const std::string& expected_file, } // anonymous namespace void MockCodeGenerator::CheckGeneratedAnnotations( - const string& name, const std::string& file, + const std::string& name, const std::string& file, const std::string& output_directory) { std::string file_content; GOOGLE_CHECK_OK( diff --git a/src/google/protobuf/compiler/parser.cc b/src/google/protobuf/compiler/parser.cc index 8cbc95b52b..c5bd0a4548 100644 --- a/src/google/protobuf/compiler/parser.cc +++ b/src/google/protobuf/compiler/parser.cc @@ -119,7 +119,7 @@ bool IsLowercase(char c) { return c >= 'a' && c <= 'z'; } bool IsNumber(char c) { return c >= '0' && c <= '9'; } -bool IsUpperCamelCase(const string& name) { +bool IsUpperCamelCase(const std::string& name) { if (name.empty()) { return true; } @@ -136,7 +136,7 @@ bool IsUpperCamelCase(const string& name) { return true; } -bool IsUpperUnderscore(const string& name) { +bool IsUpperUnderscore(const std::string& name) { for (int i = 0; i < name.length(); i++) { const char c = name[i]; if (!IsUppercase(c) && c != '_' && !IsNumber(c)) { @@ -146,7 +146,7 @@ bool IsUpperUnderscore(const string& name) { return true; } -bool IsLowerUnderscore(const string& name) { +bool IsLowerUnderscore(const std::string& name) { for (int i = 0; i < name.length(); i++) { const char c = name[i]; if (!IsLowercase(c) && c != '_' && !IsNumber(c)) { @@ -156,7 +156,7 @@ bool IsLowerUnderscore(const string& name) { return true; } -bool IsNumberFollowUnderscore(const string& name) { +bool IsNumberFollowUnderscore(const std::string& name) { for (int i = 1; i < name.length(); i++) { const char c = name[i]; if (IsNumber(c) && name[i - 1] == '_') { @@ -223,7 +223,7 @@ bool Parser::Consume(const char* text) { if (TryConsume(text)) { return true; } else { - AddError("Expected \"" + string(text) + "\"."); + AddError("Expected \"" + std::string(text) + "\"."); return false; } } @@ -369,7 +369,7 @@ bool Parser::ConsumeEndOfDeclaration(const char* text, if (TryConsumeEndOfDeclaration(text, location)) { return true; } else { - AddError("Expected \"" + string(text) + "\"."); + AddError("Expected \"" + std::string(text) + "\"."); return false; } } @@ -387,7 +387,7 @@ void Parser::AddError(const std::string& error) { AddError(input_->current().line, input_->current().column, error); } -void Parser::AddWarning(const string& warning) { +void Parser::AddWarning(const std::string& warning) { if (error_collector_ != nullptr) { error_collector_->AddWarning(input_->current().line, input_->current().column, warning); @@ -477,7 +477,7 @@ void Parser::LocationRecorder::RecordLegacyLocation( } void Parser::LocationRecorder::RecordLegacyImportLocation( - const Message* descriptor, const string& name) { + const Message* descriptor, const std::string& name) { if (parser_->source_location_table_ != nullptr) { parser_->source_location_table_->AddImport( descriptor, name, location_->span(0), location_->span(1)); @@ -2327,7 +2327,7 @@ bool Parser::ParseImport(RepeatedPtrField* dependency, *weak_dependency->Add() = dependency->size(); } - string import_file; + std::string import_file; DO(ConsumeString(&import_file, "Expected a string naming the file to import.")); *dependency->Add() = import_file; @@ -2361,7 +2361,7 @@ bool SourceLocationTable::Find( } bool SourceLocationTable::FindImport(const Message* descriptor, - const string& name, int* line, + const std::string& name, int* line, int* column) const { const std::pair* result = FindOrNull(import_location_map_, std::make_pair(descriptor, name)); @@ -2385,7 +2385,8 @@ void SourceLocationTable::Add( } void SourceLocationTable::AddImport(const Message* descriptor, - const string& name, int line, int column) { + const std::string& name, int line, + int column) { import_location_map_[std::make_pair(descriptor, name)] = std::make_pair(line, column); } diff --git a/src/google/protobuf/compiler/parser.h b/src/google/protobuf/compiler/parser.h index 45d433dc2b..e8afdbc4d8 100644 --- a/src/google/protobuf/compiler/parser.h +++ b/src/google/protobuf/compiler/parser.h @@ -208,7 +208,7 @@ class PROTOBUF_EXPORT Parser { // Invokes error_collector_->AddWarning() with the line and column number // of the current token. - void AddWarning(const string& warning); + void AddWarning(const std::string& warning); // Records a location in the SourceCodeInfo.location table (see // descriptor.proto). We use RAII to ensure that the start and end locations @@ -262,7 +262,7 @@ class PROTOBUF_EXPORT Parser { const Message* descriptor, DescriptorPool::ErrorCollector::ErrorLocation location); void RecordLegacyImportLocation(const Message* descriptor, - const string& name); + const std::string& name); // Returns the number of path components in the recorder's current location. int CurrentPathSize() const; @@ -568,14 +568,14 @@ class PROTOBUF_EXPORT SourceLocationTable { bool Find(const Message* descriptor, DescriptorPool::ErrorCollector::ErrorLocation location, int* line, int* column) const; - bool FindImport(const Message* descriptor, const string& name, int* line, + bool FindImport(const Message* descriptor, const std::string& name, int* line, int* column) const; // Adds a location to the table. void Add(const Message* descriptor, DescriptorPool::ErrorCollector::ErrorLocation location, int line, int column); - void AddImport(const Message* descriptor, const string& name, int line, + void AddImport(const Message* descriptor, const std::string& name, int line, int column); // Clears the contents of the table. @@ -587,7 +587,7 @@ class PROTOBUF_EXPORT SourceLocationTable { std::pair > LocationMap; LocationMap location_map_; - std::map, std::pair > + std::map, std::pair > import_location_map_; }; diff --git a/src/google/protobuf/compiler/parser_unittest.cc b/src/google/protobuf/compiler/parser_unittest.cc index 66abb13097..8994cc5a5e 100644 --- a/src/google/protobuf/compiler/parser_unittest.cc +++ b/src/google/protobuf/compiler/parser_unittest.cc @@ -224,7 +224,7 @@ TEST_F(ParserTest, WarnIfSyntaxIdentifierOmmitted) { CaptureTestStderr(); EXPECT_TRUE(parser_->Parse(input_.get(), &file)); EXPECT_TRUE(GetCapturedTestStderr().find("No syntax specified") != - string::npos); + std::string::npos); } TEST_F(ParserTest, WarnIfFieldNameIsNotUpperCamel) { @@ -235,7 +235,7 @@ TEST_F(ParserTest, WarnIfFieldNameIsNotUpperCamel) { EXPECT_TRUE(parser_->Parse(input_.get(), &file)); EXPECT_TRUE(error_collector_.warning_.find( "Message name should be in UpperCamelCase. Found: abc.") != - string::npos); + std::string::npos); } TEST_F(ParserTest, WarnIfFieldNameIsNotLowerUnderscore) { @@ -248,7 +248,7 @@ TEST_F(ParserTest, WarnIfFieldNameIsNotLowerUnderscore) { EXPECT_TRUE(parser_->Parse(input_.get(), &file)); EXPECT_TRUE(error_collector_.warning_.find( "Field name should be lowercase. Found: SongName") != - string::npos); + std::string::npos); } TEST_F(ParserTest, WarnIfFieldNameContainsNumberImmediatelyFollowUnderscore) { @@ -261,7 +261,7 @@ TEST_F(ParserTest, WarnIfFieldNameContainsNumberImmediatelyFollowUnderscore) { EXPECT_TRUE(parser_->Parse(input_.get(), &file)); EXPECT_TRUE(error_collector_.warning_.find( "Number should not come right after an underscore. Found: " - "song_name_1.") != string::npos); + "song_name_1.") != std::string::npos); } // =================================================================== @@ -2403,9 +2403,9 @@ TEST_F(ParseDescriptorDebugTest, TestMaps) { // Make sure the debug string uses map syntax and does not have the auto // generated entry. std::string debug_string = file->DebugString(); - EXPECT_TRUE(debug_string.find("map<") != string::npos); - EXPECT_TRUE(debug_string.find("option map_entry") == string::npos); - EXPECT_TRUE(debug_string.find("MapEntry") == string::npos); + EXPECT_TRUE(debug_string.find("map<") != std::string::npos); + EXPECT_TRUE(debug_string.find("option map_entry") == std::string::npos); + EXPECT_TRUE(debug_string.find("MapEntry") == std::string::npos); // Make sure the descriptor debug string is parsable. FileDescriptorProto parsed; diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 13373cd199..786721c437 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -1496,6 +1496,23 @@ const FieldDescriptor* DescriptorPool::FindExtensionByNumber( return nullptr; } +const FieldDescriptor* DescriptorPool::InternalFindExtensionByNumberNoLock( + const Descriptor* extendee, int number) const { + if (extendee->extension_range_count() == 0) return nullptr; + + const FieldDescriptor* result = tables_->FindExtension(extendee, number); + if (result != nullptr) { + return result; + } + + if (underlay_ != nullptr) { + result = underlay_->InternalFindExtensionByNumberNoLock(extendee, number); + if (result != nullptr) return result; + } + + return nullptr; +} + const FieldDescriptor* DescriptorPool::FindExtensionByPrintableName( const Descriptor* extendee, const std::string& printable_name) const { if (extendee->extension_range_count() == 0) return nullptr; @@ -3252,7 +3269,8 @@ class DescriptorBuilder { // descriptor.proto. template void AllocateOptions(const typename DescriptorT::OptionsType& orig_options, - DescriptorT* descriptor, int options_field_tag); + DescriptorT* descriptor, int options_field_tag, + const std::string& option_name); // Specialization for FileOptions. void AllocateOptions(const FileOptions& orig_options, FileDescriptor* descriptor); @@ -3262,7 +3280,8 @@ class DescriptorBuilder { void AllocateOptionsImpl( const std::string& name_scope, const std::string& element_name, const typename DescriptorT::OptionsType& orig_options, - DescriptorT* descriptor, const std::vector& options_path); + DescriptorT* descriptor, const std::vector& options_path, + const std::string& option_name); // Allocate string on the string pool and initialize it to full proto name. // Full proto name is "scope.proto_name" if scope is non-empty and @@ -4075,12 +4094,13 @@ void DescriptorBuilder::ValidateSymbolName(const std::string& name, template void DescriptorBuilder::AllocateOptions( const typename DescriptorT::OptionsType& orig_options, - DescriptorT* descriptor, int options_field_tag) { + DescriptorT* descriptor, int options_field_tag, + const std::string& option_name) { std::vector options_path; descriptor->GetLocationPath(&options_path); options_path.push_back(options_field_tag); AllocateOptionsImpl(descriptor->full_name(), descriptor->full_name(), - orig_options, descriptor, options_path); + orig_options, descriptor, options_path, option_name); } // We specialize for FileDescriptor. @@ -4090,14 +4110,16 @@ void DescriptorBuilder::AllocateOptions(const FileOptions& orig_options, options_path.push_back(FileDescriptorProto::kOptionsFieldNumber); // We add the dummy token so that LookupSymbol does the right thing. AllocateOptionsImpl(descriptor->package() + ".dummy", descriptor->name(), - orig_options, descriptor, options_path); + orig_options, descriptor, options_path, + "google.protobuf.FileOptions"); } template void DescriptorBuilder::AllocateOptionsImpl( const std::string& name_scope, const std::string& element_name, const typename DescriptorT::OptionsType& orig_options, - DescriptorT* descriptor, const std::vector& options_path) { + DescriptorT* descriptor, const std::vector& options_path, + const std::string& option_name) { // We need to use a dummy pointer to work around a bug in older versions of // GCC. Otherwise, the following two lines could be replaced with: // typename DescriptorT::OptionsType* options = @@ -4130,6 +4152,25 @@ void DescriptorBuilder::AllocateOptionsImpl( options_to_interpret_.push_back(OptionsToInterpret( name_scope, element_name, options_path, &orig_options, options)); } + + // If the custom option is in unknown fields, no need to interpret it. + // Remove the dependency file from unused_dependency. + const UnknownFieldSet& unknown_fields = orig_options.unknown_fields(); + if (!unknown_fields.empty()) { + // Can not use options->GetDescriptor() which may case deadlock. + Symbol msg_symbol = tables_->FindSymbol(option_name); + if (msg_symbol.type == Symbol::MESSAGE) { + for (int i = 0; i < unknown_fields.field_count(); ++i) { + assert_mutex_held(pool_); + const FieldDescriptor* field = + pool_->InternalFindExtensionByNumberNoLock( + msg_symbol.descriptor, unknown_fields.field(i).number()); + if (field) { + unused_dependency_.erase(field->file()); + } + } + } + } } // A common pattern: We want to convert a repeated field in the descriptor @@ -4555,7 +4596,8 @@ void DescriptorBuilder::BuildMessage(const DescriptorProto& proto, result->options_ = nullptr; // Will set to default_instance later. } else { AllocateOptions(proto.options(), result, - DescriptorProto::kOptionsFieldNumber); + DescriptorProto::kOptionsFieldNumber, + "google.protobuf.MessageOptions"); } AddSymbol(result->full_name(), parent, result->name(), proto, Symbol(result)); @@ -4928,7 +4970,8 @@ void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto, result->options_ = nullptr; // Will set to default_instance later. } else { AllocateOptions(proto.options(), result, - FieldDescriptorProto::kOptionsFieldNumber); + FieldDescriptorProto::kOptionsFieldNumber, + "google.protobuf.FieldOptions"); } @@ -4968,7 +5011,8 @@ void DescriptorBuilder::BuildExtensionRange( options_path.push_back(index); options_path.push_back(DescriptorProto_ExtensionRange::kOptionsFieldNumber); AllocateOptionsImpl(parent->full_name(), parent->full_name(), - proto.options(), result, options_path); + proto.options(), result, options_path, + "google.protobuf.ExtensionRangeOptions"); } } @@ -5015,7 +5059,8 @@ void DescriptorBuilder::BuildOneof(const OneofDescriptorProto& proto, // Copy options. if (proto.has_options()) { AllocateOptions(proto.options(), result, - OneofDescriptorProto::kOptionsFieldNumber); + OneofDescriptorProto::kOptionsFieldNumber, + "google.protobuf.OneofOptions"); } AddSymbol(result->full_name(), parent, result->name(), proto, Symbol(result)); @@ -5129,7 +5174,8 @@ void DescriptorBuilder::BuildEnum(const EnumDescriptorProto& proto, result->options_ = nullptr; // Will set to default_instance later. } else { AllocateOptions(proto.options(), result, - EnumDescriptorProto::kOptionsFieldNumber); + EnumDescriptorProto::kOptionsFieldNumber, + "google.protobuf.EnumOptions"); } AddSymbol(result->full_name(), parent, result->name(), proto, Symbol(result)); @@ -5207,7 +5253,8 @@ void DescriptorBuilder::BuildEnumValue(const EnumValueDescriptorProto& proto, result->options_ = nullptr; // Will set to default_instance later. } else { AllocateOptions(proto.options(), result, - EnumValueDescriptorProto::kOptionsFieldNumber); + EnumValueDescriptorProto::kOptionsFieldNumber, + "google.protobuf.EnumValueOptions"); } // Again, enum values are weird because we makes them appear as siblings @@ -5272,7 +5319,8 @@ void DescriptorBuilder::BuildService(const ServiceDescriptorProto& proto, result->options_ = nullptr; // Will set to default_instance later. } else { AllocateOptions(proto.options(), result, - ServiceDescriptorProto::kOptionsFieldNumber); + ServiceDescriptorProto::kOptionsFieldNumber, + "google.protobuf.ServiceOptions"); } AddSymbol(result->full_name(), nullptr, result->name(), proto, @@ -5300,7 +5348,8 @@ void DescriptorBuilder::BuildMethod(const MethodDescriptorProto& proto, result->options_ = nullptr; // Will set to default_instance later. } else { AllocateOptions(proto.options(), result, - MethodDescriptorProto::kOptionsFieldNumber); + MethodDescriptorProto::kOptionsFieldNumber, + "google.protobuf.MethodOptions"); } result->client_streaming_ = proto.client_streaming(); diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index a51d93bfe4..422e444e27 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -1813,6 +1813,12 @@ class PROTOBUF_EXPORT DescriptorPool { bool TryFindExtensionInFallbackDatabase(const Descriptor* containing_type, int field_number) const; + // This internal find extension method only check with its table and underlay + // descriptor_pool's table. It does not check with fallback DB and no + // additional proto file will be build in this method. + const FieldDescriptor* InternalFindExtensionByNumberNoLock( + const Descriptor* extendee, int number) const; + // Like BuildFile() but called internally when the file has been loaded from // fallback_database_. Declared const because it is called by (semantically) // const methods. diff --git a/src/google/protobuf/map_entry_lite.h b/src/google/protobuf/map_entry_lite.h index 4668c5b42e..3fc3efab14 100644 --- a/src/google/protobuf/map_entry_lite.h +++ b/src/google/protobuf/map_entry_lite.h @@ -255,7 +255,8 @@ class MapEntryImpl : public Base { ctx->SetLastTag(tag); return ptr; } - ptr = UnknownFieldParse(tag, static_cast(nullptr), ptr, ctx); + ptr = UnknownFieldParse(tag, static_cast(nullptr), ptr, + ctx); } GOOGLE_PROTOBUF_PARSER_ASSERT(ptr); } diff --git a/src/google/protobuf/map_test.cc b/src/google/protobuf/map_test.cc index 0727ab4272..a2d81dbb44 100644 --- a/src/google/protobuf/map_test.cc +++ b/src/google/protobuf/map_test.cc @@ -3010,6 +3010,58 @@ TEST(WireFormatForMapFieldTest, SerializeMapDynamicMessage) { EXPECT_TRUE(dynamic_data.size() == generated_data.size()); } +TEST(WireFormatForMapFieldTest, MapByteSizeDynamicMessage) { + DynamicMessageFactory factory; + std::unique_ptr dynamic_message; + dynamic_message.reset( + factory.GetPrototype(unittest::TestMap::descriptor())->New()); + MapReflectionTester reflection_tester(unittest::TestMap::descriptor()); + reflection_tester.SetMapFieldsViaReflection(dynamic_message.get()); + reflection_tester.ExpectMapFieldsSetViaReflection(*dynamic_message); + std::string expected_serialized_data; + dynamic_message->SerializeToString(&expected_serialized_data); + int expected_size = expected_serialized_data.size(); + EXPECT_EQ(dynamic_message->ByteSize(), expected_size); + + std::unique_ptr message2; + message2.reset(factory.GetPrototype(unittest::TestMap::descriptor())->New()); + reflection_tester.SetMapFieldsViaMapReflection(message2.get()); + + const FieldDescriptor* field = + unittest::TestMap::descriptor()->FindFieldByName("map_int32_int32"); + const Reflection* reflection = dynamic_message->GetReflection(); + + // Force the map field to mark with STATE_MODIFIED_REPEATED + reflection->RemoveLast(dynamic_message.get(), field); + dynamic_message->MergeFrom(*message2); + dynamic_message->MergeFrom(*message2); + // The map field is marked as STATE_MODIFIED_REPEATED, ByteSize() will use + // repeated field which have duplicate keys to calculate. + int duplicate_size = dynamic_message->ByteSize(); + EXPECT_TRUE(duplicate_size > expected_size); + std::string duplicate_serialized_data; + dynamic_message->SerializeToString(&duplicate_serialized_data); + EXPECT_EQ(dynamic_message->ByteSize(), duplicate_serialized_data.size()); + + // Force the map field to mark with map CLEAN + EXPECT_EQ(reflection_tester.MapSize(*dynamic_message, "map_int32_int32"), 2); + // The map field is marked as CLEAN, ByteSize() will use map which do not + // have duplicate keys to calculate. + int size = dynamic_message->ByteSize(); + EXPECT_EQ(expected_size, size); + + // Protobuf used to have a bug for serialize when map it marked CLEAN. It used + // repeated field to calulate ByteSize but use map to serialize the real data, + // thus the ByteSize may bigger than real serialized size. A crash might be + // happen at SerializeToString(). Or an "unexpect end group" warning was + // raised at parse back if user use SerializeWithCachedSizes() which avoids + // size check at serialize. + std::string serialized_data; + dynamic_message->SerializeToString(&serialized_data); + EXPECT_EQ(serialized_data, expected_serialized_data); + dynamic_message->ParseFromString(serialized_data); +} + TEST(WireFormatForMapFieldTest, MapParseHelpers) { std::string data; diff --git a/src/google/protobuf/map_test_util.h b/src/google/protobuf/map_test_util.h index 54ccfbb8d0..9787d3d1e6 100644 --- a/src/google/protobuf/map_test_util.h +++ b/src/google/protobuf/map_test_util.h @@ -73,6 +73,7 @@ class MapReflectionTester { const std::string& field_name, int index); MapIterator MapBegin(Message* message, const std::string& field_name); MapIterator MapEnd(Message* message, const std::string& field_name); + int MapSize(const Message& message, const std::string& field_name); private: const FieldDescriptor* F(const std::string& name); @@ -658,6 +659,12 @@ inline MapIterator MapReflectionTester::MapEnd(Message* message, return reflection->MapEnd(message, F(field_name)); } +inline int MapReflectionTester::MapSize(const Message& message, + const std::string& field_name) { + const Reflection* reflection = message.GetReflection(); + return reflection->MapSize(message, F(field_name)); +} + inline void MapReflectionTester::ClearMapFieldsViaReflection(Message* message) { const Reflection* reflection = message->GetReflection(); diff --git a/src/google/protobuf/message.h b/src/google/protobuf/message.h index b5b5392f7e..92aa1f15b7 100644 --- a/src/google/protobuf/message.h +++ b/src/google/protobuf/message.h @@ -945,6 +945,7 @@ class PROTOBUF_EXPORT Reflection final { // Help method for MapIterator. friend class MapIterator; + friend class WireFormatForMapFieldTest; internal::MapFieldBase* MutableMapData(Message* message, const FieldDescriptor* field) const; diff --git a/src/google/protobuf/parse_context.h b/src/google/protobuf/parse_context.h index f15cb35f88..5a0df06d89 100644 --- a/src/google/protobuf/parse_context.h +++ b/src/google/protobuf/parse_context.h @@ -218,12 +218,12 @@ class PROTOBUF_EXPORT EpsCopyInputStream { overall_limit_ = 0; if (flat.size() > kSlopBytes) { limit_ = kSlopBytes; - limit_end_ = buffer_end_ = flat.end() - kSlopBytes; + limit_end_ = buffer_end_ = flat.data() + flat.size() - kSlopBytes; next_chunk_ = buffer_; if (aliasing_ == kOnPatch) aliasing_ = kNoDelta; - return flat.begin(); + return flat.data(); } else { - std::memcpy(buffer_, flat.begin(), flat.size()); + std::memcpy(buffer_, flat.data(), flat.size()); limit_ = 0; limit_end_ = buffer_end_ = buffer_ + flat.size(); next_chunk_ = nullptr; diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc index 24a41bbfae..a5b6fe5571 100644 --- a/src/google/protobuf/port_def.inc +++ b/src/google/protobuf/port_def.inc @@ -529,6 +529,10 @@ PROTOBUF_EXPORT_TEMPLATE_TEST(DEFAULT, __declspec(dllimport)); #undef IGNORE #pragma push_macro("IN") #undef IN +#pragma push_macro("min") +#undef min +#pragma push_macro("max") +#undef max #endif // _MSC_VER #if defined(__clang__) diff --git a/src/google/protobuf/port_undef.inc b/src/google/protobuf/port_undef.inc index 3a62932944..f6725658d0 100644 --- a/src/google/protobuf/port_undef.inc +++ b/src/google/protobuf/port_undef.inc @@ -87,6 +87,8 @@ #pragma pop_macro("GetMessage") #pragma pop_macro("IGNORE") #pragma pop_macro("IN") +#pragma pop_macro("min") +#pragma pop_macro("max") #endif #if defined(__clang__) diff --git a/src/google/protobuf/proto3_arena_lite_unittest.cc b/src/google/protobuf/proto3_arena_lite_unittest.cc index e2693708a2..06b8d08c8f 100644 --- a/src/google/protobuf/proto3_arena_lite_unittest.cc +++ b/src/google/protobuf/proto3_arena_lite_unittest.cc @@ -50,8 +50,7 @@ void SetAllFields(TestAllTypes* m) { m->set_optional_bytes("jkl;"); m->mutable_optional_nested_message()->set_bb(42); m->mutable_optional_foreign_message()->set_c(43); - m->set_optional_nested_enum( - proto3_arena_unittest::TestAllTypes_NestedEnum_BAZ); + m->set_optional_nested_enum(proto3_arena_unittest::TestAllTypes::BAZ); m->set_optional_foreign_enum(proto3_arena_unittest::FOREIGN_BAZ); m->mutable_optional_lazy_message()->set_bb(45); m->add_repeated_int32(100); @@ -59,8 +58,7 @@ void SetAllFields(TestAllTypes* m) { m->add_repeated_bytes("jkl;"); m->add_repeated_nested_message()->set_bb(46); m->add_repeated_foreign_message()->set_c(47); - m->add_repeated_nested_enum( - proto3_arena_unittest::TestAllTypes_NestedEnum_BAZ); + m->add_repeated_nested_enum(proto3_arena_unittest::TestAllTypes::BAZ); m->add_repeated_foreign_enum(proto3_arena_unittest::FOREIGN_BAZ); m->add_repeated_lazy_message()->set_bb(49); @@ -77,8 +75,7 @@ void ExpectAllFieldsSet(const TestAllTypes& m) { EXPECT_EQ(42, m.optional_nested_message().bb()); EXPECT_EQ(true, m.has_optional_foreign_message()); EXPECT_EQ(43, m.optional_foreign_message().c()); - EXPECT_EQ(proto3_arena_unittest::TestAllTypes_NestedEnum_BAZ, - m.optional_nested_enum()); + EXPECT_EQ(proto3_arena_unittest::TestAllTypes::BAZ, m.optional_nested_enum()); EXPECT_EQ(proto3_arena_unittest::FOREIGN_BAZ, m.optional_foreign_enum()); EXPECT_EQ(true, m.has_optional_lazy_message()); EXPECT_EQ(45, m.optional_lazy_message().bb()); @@ -94,7 +91,7 @@ void ExpectAllFieldsSet(const TestAllTypes& m) { EXPECT_EQ(1, m.repeated_foreign_message_size()); EXPECT_EQ(47, m.repeated_foreign_message(0).c()); EXPECT_EQ(1, m.repeated_nested_enum_size()); - EXPECT_EQ(proto3_arena_unittest::TestAllTypes_NestedEnum_BAZ, + EXPECT_EQ(proto3_arena_unittest::TestAllTypes::BAZ, m.repeated_nested_enum(0)); EXPECT_EQ(1, m.repeated_foreign_enum_size()); EXPECT_EQ(proto3_arena_unittest::FOREIGN_BAZ, m.repeated_foreign_enum(0)); diff --git a/src/google/protobuf/proto3_lite_unittest.inc b/src/google/protobuf/proto3_lite_unittest.inc index 636691b49c..4cf9cf8f1e 100644 --- a/src/google/protobuf/proto3_lite_unittest.inc +++ b/src/google/protobuf/proto3_lite_unittest.inc @@ -49,8 +49,7 @@ void SetAllFields(TestAllTypes* m) { m->set_optional_bytes("jkl;"); m->mutable_optional_nested_message()->set_bb(42); m->mutable_optional_foreign_message()->set_c(43); - m->set_optional_nested_enum( - UNITTEST::TestAllTypes_NestedEnum_BAZ); + m->set_optional_nested_enum(UNITTEST::TestAllTypes::BAZ); m->set_optional_foreign_enum( UNITTEST::FOREIGN_BAZ); m->mutable_optional_lazy_message()->set_bb(45); @@ -59,8 +58,7 @@ void SetAllFields(TestAllTypes* m) { m->add_repeated_bytes("jkl;"); m->add_repeated_nested_message()->set_bb(46); m->add_repeated_foreign_message()->set_c(47); - m->add_repeated_nested_enum( - UNITTEST::TestAllTypes_NestedEnum_BAZ); + m->add_repeated_nested_enum(UNITTEST::TestAllTypes::BAZ); m->add_repeated_foreign_enum( UNITTEST::FOREIGN_BAZ); m->add_repeated_lazy_message()->set_bb(49); @@ -78,8 +76,7 @@ void ExpectAllFieldsSet(const TestAllTypes& m) { EXPECT_EQ(42, m.optional_nested_message().bb()); EXPECT_EQ(true, m.has_optional_foreign_message()); EXPECT_EQ(43, m.optional_foreign_message().c()); - EXPECT_EQ(UNITTEST::TestAllTypes_NestedEnum_BAZ, - m.optional_nested_enum()); + EXPECT_EQ(UNITTEST::TestAllTypes::BAZ, m.optional_nested_enum()); EXPECT_EQ(UNITTEST::FOREIGN_BAZ, m.optional_foreign_enum()); EXPECT_EQ(true, m.has_optional_lazy_message()); @@ -96,8 +93,7 @@ void ExpectAllFieldsSet(const TestAllTypes& m) { EXPECT_EQ(1, m.repeated_foreign_message_size()); EXPECT_EQ(47, m.repeated_foreign_message(0).c()); EXPECT_EQ(1, m.repeated_nested_enum_size()); - EXPECT_EQ(UNITTEST::TestAllTypes_NestedEnum_BAZ, - m.repeated_nested_enum(0)); + EXPECT_EQ(UNITTEST::TestAllTypes::BAZ, m.repeated_nested_enum(0)); EXPECT_EQ(1, m.repeated_foreign_enum_size()); EXPECT_EQ(UNITTEST::FOREIGN_BAZ, m.repeated_foreign_enum(0)); diff --git a/src/google/protobuf/repeated_field.cc b/src/google/protobuf/repeated_field.cc index f0b85d39a1..1c43c56a9b 100644 --- a/src/google/protobuf/repeated_field.cc +++ b/src/google/protobuf/repeated_field.cc @@ -122,6 +122,7 @@ MessageLite* RepeatedPtrFieldBase::AddWeak(const MessageLite* prototype) { } // namespace internal + template class PROTOBUF_EXPORT_TEMPLATE_DEFINE(PROTOBUF_EXPORT) RepeatedField; template class PROTOBUF_EXPORT_TEMPLATE_DEFINE(PROTOBUF_EXPORT) diff --git a/src/google/protobuf/unknown_field_set.h b/src/google/protobuf/unknown_field_set.h index 5973c3e36f..b6134691b1 100644 --- a/src/google/protobuf/unknown_field_set.h +++ b/src/google/protobuf/unknown_field_set.h @@ -39,12 +39,15 @@ #define GOOGLE_PROTOBUF_UNKNOWN_FIELD_SET_H__ #include + #include #include + #include #include #include #include +#include #include #include @@ -164,6 +167,12 @@ class PROTOBUF_EXPORT UnknownFieldSet { return ParseFromArray(data.data(), static_cast(data.size())); } + // Merges this message's unknown field data (if any). This works whether + // the message is a lite or full proto (for legacy reasons, lite and full + // return different types for MessageType::unknown_fields()). + template + bool MergeFromMessage(const MessageType& message); + static const UnknownFieldSet* default_instance(); private: @@ -174,6 +183,27 @@ class PROTOBUF_EXPORT UnknownFieldSet { void InternalMergeFrom(const UnknownFieldSet& other); void ClearFallback(); + template ::value, int>::type = 0> + bool InternalMergeFromMessage(const MessageType& message) { + MergeFrom(message.GetReflection()->GetUnknownFields(message)); + return true; + } + + template ::value && + !std::is_base_of::value, + int>::type = 0> + bool InternalMergeFromMessage(const MessageType& message) { + const auto& unknown_fields = message.unknown_fields(); + io::ArrayInputStream array_stream(unknown_fields.data(), + unknown_fields.size()); + io::CodedInputStream coded_stream(&array_stream); + return MergeFromCodedStream(&coded_stream); + } + std::vector fields_; GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(UnknownFieldSet); }; @@ -373,6 +403,12 @@ inline UnknownFieldSet* UnknownField::mutable_group() { assert(type() == TYPE_GROUP); return data_.group_; } +template +bool UnknownFieldSet::MergeFromMessage(const MessageType& message) { + // SFINAE will route to the right version. + return InternalMergeFromMessage(message); +} + inline size_t UnknownField::GetLengthDelimitedSize() const { GOOGLE_DCHECK_EQ(TYPE_LENGTH_DELIMITED, type()); diff --git a/src/google/protobuf/unknown_field_set_unittest.cc b/src/google/protobuf/unknown_field_set_unittest.cc index b3af94b629..b1ccb5c5a1 100644 --- a/src/google/protobuf/unknown_field_set_unittest.cc +++ b/src/google/protobuf/unknown_field_set_unittest.cc @@ -36,17 +36,18 @@ // tests handling of unknown fields throughout the system. #include + +#include +#include +#include #include #include +#include #include #include #include #include #include - -#include -#include -#include #include #include #include @@ -306,6 +307,42 @@ TEST_F(UnknownFieldSetTest, MergeFrom) { destination.DebugString()); } +TEST_F(UnknownFieldSetTest, MergeFromMessage) { + unittest::TestEmptyMessage source, destination; + + destination.mutable_unknown_fields()->AddVarint(1, 1); + destination.mutable_unknown_fields()->AddVarint(3, 2); + source.mutable_unknown_fields()->AddVarint(2, 3); + source.mutable_unknown_fields()->AddVarint(3, 4); + + destination.mutable_unknown_fields()->MergeFromMessage(source); + + EXPECT_EQ( + // Note: The ordering of fields here depends on the ordering of adds + // and merging, above. + "1: 1\n" + "3: 2\n" + "2: 3\n" + "3: 4\n", + destination.DebugString()); +} + +TEST_F(UnknownFieldSetTest, MergeFromMessageLite) { + unittest::TestAllTypesLite source; + unittest::TestEmptyMessageLite destination; + + source.set_optional_fixed32(42); + destination.ParseFromString(source.SerializeAsString()); + + UnknownFieldSet unknown_field_set; + EXPECT_TRUE(unknown_field_set.MergeFromMessage(destination)); + EXPECT_EQ(unknown_field_set.field_count(), 1); + + const UnknownField& unknown_field = unknown_field_set.field(0); + EXPECT_EQ(unknown_field.number(), 7); + EXPECT_EQ(unknown_field.fixed32(), 42); +} + TEST_F(UnknownFieldSetTest, Clear) { // Clear the set. diff --git a/src/google/protobuf/util/internal/datapiece.cc b/src/google/protobuf/util/internal/datapiece.cc index 314d76ecd6..1ab0b86d2f 100644 --- a/src/google/protobuf/util/internal/datapiece.cc +++ b/src/google/protobuf/util/internal/datapiece.cc @@ -356,7 +356,7 @@ StatusOr DataPiece::StringToNumber(bool (*func)(StringPiece, } To result; if (func(str_, &result)) return result; - return InvalidArgument(StrCat("\"", string(str_), "\"")); + return InvalidArgument(StrCat("\"", std::string(str_), "\"")); } bool DataPiece::DecodeBase64(StringPiece src, std::string* dest) const { diff --git a/src/google/protobuf/util/internal/json_objectwriter_test.cc b/src/google/protobuf/util/internal/json_objectwriter_test.cc index 4d190b8708..e9f252b88a 100644 --- a/src/google/protobuf/util/internal/json_objectwriter_test.cc +++ b/src/google/protobuf/util/internal/json_objectwriter_test.cc @@ -152,7 +152,7 @@ TEST_F(JsonObjectWriterTest, RenderPrimitives) { ->RenderBytes("bytes", "abracadabra") ->RenderString("string", "string") ->RenderBytes("emptybytes", "") - ->RenderString("emptystring", string()) + ->RenderString("emptystring", std::string()) ->EndObject(); EXPECT_EQ( "{\"bool\":true," diff --git a/src/google/protobuf/util/internal/protostream_objectsource.cc b/src/google/protobuf/util/internal/protostream_objectsource.cc index fdb537f5fa..89f424b0cc 100644 --- a/src/google/protobuf/util/internal/protostream_objectsource.cc +++ b/src/google/protobuf/util/internal/protostream_objectsource.cc @@ -799,7 +799,6 @@ Status ProtoStreamObjectSource::RenderField( return util::Status(); } - Status ProtoStreamObjectSource::RenderNonMessageField( const google::protobuf::Field* field, StringPiece field_name, ObjectWriter* ow) const { diff --git a/src/google/protobuf/util/internal/protostream_objectsource_test.cc b/src/google/protobuf/util/internal/protostream_objectsource_test.cc index c67aeea859..22fc3f62df 100644 --- a/src/google/protobuf/util/internal/protostream_objectsource_test.cc +++ b/src/google/protobuf/util/internal/protostream_objectsource_test.cc @@ -56,6 +56,7 @@ namespace protobuf { namespace util { namespace converter { +using ::google::protobuf::Any; using io::ArrayInputStream; using io::CodedInputStream; using proto_util_converter::testing::AnyM; @@ -1156,6 +1157,7 @@ TEST_P(ProtostreamObjectSourceTimestampTest, TimestampDurationDefaultValue) { } + } // namespace converter } // namespace util } // namespace protobuf diff --git a/src/google/protobuf/util/internal/protostream_objectwriter.cc b/src/google/protobuf/util/internal/protostream_objectwriter.cc index 460ced3716..0b89ae2972 100644 --- a/src/google/protobuf/util/internal/protostream_objectwriter.cc +++ b/src/google/protobuf/util/internal/protostream_objectwriter.cc @@ -608,6 +608,12 @@ ProtoStreamObjectWriter* ProtoStreamObjectWriter::StartObject( return this; } + // Legacy JSON map is a list of key value pairs. Starts a map entry object. + if (options_.use_legacy_json_map_format && name.empty()) { + Push(name, IsAny(*field) ? Item::ANY : Item::MESSAGE, false, false); + return this; + } + if (IsMap(*field)) { // Begin a map. A map is triggered by a StartObject() call if the current // field has a map type. @@ -843,6 +849,10 @@ ProtoStreamObjectWriter* ProtoStreamObjectWriter::StartList( } if (IsMap(*field)) { + if (options_.use_legacy_json_map_format) { + Push(name, Item::MESSAGE, false, true); + return this; + } InvalidValue("Map", StrCat("Cannot bind a list to map for field '", name, "'.")); IncrementInvalidDepth(); diff --git a/src/google/protobuf/util/internal/protostream_objectwriter.h b/src/google/protobuf/util/internal/protostream_objectwriter.h index acbf5cc269..fc015c63f5 100644 --- a/src/google/protobuf/util/internal/protostream_objectwriter.h +++ b/src/google/protobuf/util/internal/protostream_objectwriter.h @@ -97,13 +97,17 @@ class PROTOBUF_EXPORT ProtoStreamObjectWriter : public ProtoWriter { // value type is google.protobuf.NullType. bool ignore_null_value_map_entry; + // If true, accepts repeated key/value pair for a map proto field. + bool use_legacy_json_map_format; + Options() : struct_integers_as_strings(false), ignore_unknown_fields(false), ignore_unknown_enum_values(false), use_lower_camel_for_enums(false), case_insensitive_enum_parsing(false), - ignore_null_value_map_entry(false) {} + ignore_null_value_map_entry(false), + use_legacy_json_map_format(false) {} // Default instance of Options with all options set to defaults. static const Options& Defaults() { diff --git a/src/google/protobuf/util/time_util.h b/src/google/protobuf/util/time_util.h index 9ab94cbd50..95063fc49c 100644 --- a/src/google/protobuf/util/time_util.h +++ b/src/google/protobuf/util/time_util.h @@ -257,7 +257,7 @@ inline std::ostream& operator<<(std::ostream& out, const Duration& d) { // Overloaded operators for Timestamp // -// Assignement operators. +// Assignment operators. PROTOBUF_EXPORT Timestamp& operator+=(Timestamp& t, const Duration& d); // NOLINT PROTOBUF_EXPORT Timestamp& operator-=(Timestamp& t, diff --git a/src/google/protobuf/wire_format.cc b/src/google/protobuf/wire_format.cc index a19a6f2c44..c94881b036 100644 --- a/src/google/protobuf/wire_format.cc +++ b/src/google/protobuf/wire_format.cc @@ -1111,7 +1111,17 @@ size_t WireFormat::FieldByteSize(const FieldDescriptor* field, size_t count = 0; if (field->is_repeated()) { - count = FromIntSize(message_reflection->FieldSize(message, field)); + if (field->is_map()) { + const MapFieldBase* map_field = + message_reflection->GetMapData(message, field); + if (map_field->IsMapValid()) { + count = FromIntSize(map_field->size()); + } else { + count = FromIntSize(message_reflection->FieldSize(message, field)); + } + } else { + count = FromIntSize(message_reflection->FieldSize(message, field)); + } } else if (field->containing_type()->options().map_entry()) { // Map entry fields always need to be serialized. count = 1; From c1ba7c643cfdb43345ba1924e441077377bcfef6 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Fri, 11 Oct 2019 20:31:10 -0400 Subject: [PATCH 04/27] Remove unused argument to avoid UB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `OneOfDescriptor_each` is registered as a Ruby method which takes zero parameters, which means it should take one argument. When Ruby invokes `OneOfDescriptor_each`, it calls it with one parameter only, which is one less than what `OneOfDescriptor_each` takes before this commit. Calling a function with the wrong number of argument is technically undefined behavior. See also: §6.5.2.2, N1256 --- ruby/ext/google/protobuf_c/defs.c | 2 +- ruby/ext/google/protobuf_c/protobuf.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ruby/ext/google/protobuf_c/defs.c b/ruby/ext/google/protobuf_c/defs.c index dd1c2d8317..d092a5bf09 100644 --- a/ruby/ext/google/protobuf_c/defs.c +++ b/ruby/ext/google/protobuf_c/defs.c @@ -1232,7 +1232,7 @@ VALUE OneofDescriptor_name(VALUE _self) { * * Iterates through fields in this oneof, yielding to the block on each one. */ -VALUE OneofDescriptor_each(VALUE _self, VALUE field) { +VALUE OneofDescriptor_each(VALUE _self) { DEFINE_SELF(OneofDescriptor, self, _self); upb_oneof_iter it; for (upb_oneof_begin(&it, self->oneofdef); diff --git a/ruby/ext/google/protobuf_c/protobuf.h b/ruby/ext/google/protobuf_c/protobuf.h index 6314b78810..7cab5641d7 100644 --- a/ruby/ext/google/protobuf_c/protobuf.h +++ b/ruby/ext/google/protobuf_c/protobuf.h @@ -259,7 +259,7 @@ OneofDescriptor* ruby_to_OneofDescriptor(VALUE value); VALUE OneofDescriptor_initialize(VALUE _self, VALUE cookie, VALUE descriptor_pool, VALUE ptr); VALUE OneofDescriptor_name(VALUE _self); -VALUE OneofDescriptor_each(VALUE _self, VALUE field); +VALUE OneofDescriptor_each(VALUE _self); void EnumDescriptor_mark(void* _self); void EnumDescriptor_free(void* _self); From f50bc62febd05a2e0a8b91886258323c3fd7fa04 Mon Sep 17 00:00:00 2001 From: Sydney Acksman Date: Fri, 11 Oct 2019 20:23:07 -0500 Subject: [PATCH 05/27] Add length checks to ExtensionCollection --- .../Reflection/ExtensionCollection.cs | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/csharp/src/Google.Protobuf/Reflection/ExtensionCollection.cs b/csharp/src/Google.Protobuf/Reflection/ExtensionCollection.cs index 38a97fc4b0..1b09bdde7f 100644 --- a/csharp/src/Google.Protobuf/Reflection/ExtensionCollection.cs +++ b/csharp/src/Google.Protobuf/Reflection/ExtensionCollection.cs @@ -48,14 +48,32 @@ namespace Google.Protobuf.Reflection { UnorderedExtensions = DescriptorUtil.ConvertAndMakeReadOnly( file.Proto.Extension, - (extension, i) => new FieldDescriptor(extension, file, null, i, null, extensions?[i])); + (extension, i) => { + if (extensions?.Length != 0) + { + return new FieldDescriptor(extension, file, null, i, null, extensions?[i]); + } + else + { + return new FieldDescriptor(extension, file, null, i, null, null); // return null if there's no extensions in this array for old code-gen + } + }); } internal ExtensionCollection(MessageDescriptor message, Extension[] extensions) { UnorderedExtensions = DescriptorUtil.ConvertAndMakeReadOnly( message.Proto.Extension, - (extension, i) => new FieldDescriptor(extension, message.File, message, i, null, extensions?[i])); + (extension, i) => { + if (extensions?.Length != 0) + { + return new FieldDescriptor(extension, message.File, message, i, null, extensions?[i]); + } + else + { + return new FieldDescriptor(extension, message.File, message, i, null, null); + } + }); } /// From e93dea77460c02fe0c7bfa42621817838f9d1342 Mon Sep 17 00:00:00 2001 From: Daniel Johansen Date: Thu, 17 Oct 2019 14:27:00 +0200 Subject: [PATCH 06/27] update Cmake example to build Visual Studio to newest 2019 version. --- cmake/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmake/README.md b/cmake/README.md index 55b40c526e..89d00c1b6f 100644 --- a/cmake/README.md +++ b/cmake/README.md @@ -13,13 +13,13 @@ Environment Setup Open the appropriate *Command Prompt* from the *Start* menu. -For example *VS2013 x64 Native Tools Command Prompt*: +For example *x86 Native Tools Command Prompt for VS 2019*: - C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\bin\amd64> + C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional> Change to your working directory: - C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\bin\amd64>cd C:\Path\to + C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional>cd C:\Path\to C:\Path\to> Where *C:\Path\to* is path to your real working directory. @@ -121,7 +121,7 @@ It will generate *nmake* *Makefile* in current directory. To create *Visual Studio* solution file: C:\Path\to\protobuf\cmake\build>mkdir solution & cd solution - C:\Path\to\protobuf\cmake\build\solution>cmake -G "Visual Studio 14 2015 Win64" ^ + C:\Path\to\protobuf\cmake\build\solution>cmake -G "Visual Studio 16 2019" ^ -DCMAKE_INSTALL_PREFIX=../../../../install ^ ../.. @@ -130,7 +130,7 @@ It will generate *Visual Studio* solution file *protobuf.sln* in current directo If the *gmock* directory does not exist, and you do not want to build protobuf unit tests, you need to add *cmake* command argument `-Dprotobuf_BUILD_TESTS=OFF` to disable testing. -To make a *Visual Studio* file for Visual Studio 15 2017, create the *Visual Studio* +To make a *Visual Studio* file for Visual Studio 16 2019, create the *Visual Studio* solution file above and edit the CMakeCache file. C:Path\to\protobuf\cmake\build\solution\CMakeCache From bba24bcb6bbdc4267e87859a91389c99536c40dd Mon Sep 17 00:00:00 2001 From: woegster Date: Wed, 16 Oct 2019 02:54:27 +0200 Subject: [PATCH 07/27] avoid c++ macro redifitnion on windows --- src/google/protobuf/stubs/common.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/google/protobuf/stubs/common.cc b/src/google/protobuf/stubs/common.cc index ff9a32d503..db60393218 100644 --- a/src/google/protobuf/stubs/common.cc +++ b/src/google/protobuf/stubs/common.cc @@ -39,7 +39,9 @@ #include #ifdef _WIN32 +#ifndef WIN32_LEAN_AND_MEAN #define WIN32_LEAN_AND_MEAN // We only need minimal includes +#endif #include #define snprintf _snprintf // see comment in strutil.cc #elif defined(HAVE_PTHREAD) From b467aa16143acd06b9fb180eb2a9f1cb6fc26526 Mon Sep 17 00:00:00 2001 From: woegster Date: Wed, 16 Oct 2019 03:13:33 +0200 Subject: [PATCH 08/27] more potential windows macro redefinition --- src/google/protobuf/compiler/subprocess.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/google/protobuf/compiler/subprocess.h b/src/google/protobuf/compiler/subprocess.h index 9c4f8b7690..c1ddaae531 100644 --- a/src/google/protobuf/compiler/subprocess.h +++ b/src/google/protobuf/compiler/subprocess.h @@ -34,7 +34,9 @@ #define GOOGLE_PROTOBUF_COMPILER_SUBPROCESS_H__ #ifdef _WIN32 +#ifndef WIN32_LEAN_AND_MEAN #define WIN32_LEAN_AND_MEAN // right... +#endif #include #else // _WIN32 #include From 6f0f382d5641b8632a05c3d27174e475f76f3f31 Mon Sep 17 00:00:00 2001 From: Christian Maurer Date: Tue, 15 Oct 2019 13:16:49 +0200 Subject: [PATCH 09/27] c++ remove conversion warning in MapEntryFuncs::ByteSizeLong --- src/google/protobuf/map_entry_lite.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/google/protobuf/map_entry_lite.h b/src/google/protobuf/map_entry_lite.h index 4668c5b42e..b3f482ed19 100644 --- a/src/google/protobuf/map_entry_lite.h +++ b/src/google/protobuf/map_entry_lite.h @@ -128,7 +128,7 @@ struct MapEntryFuncs { // Tags for key and value will both be one byte (field numbers 1 and 2). size_t inner_length = 2 + KeyTypeHandler::ByteSize(key) + ValueTypeHandler::ByteSize(value); - return inner_length + io::CodedOutputStream::VarintSize32(inner_length); + return inner_length + io::CodedOutputStream::VarintSize32(static_cast(inner_length)); } static int GetCachedSize(const Key& key, const Value& value) { From 912d8195dad66d158fed2c9dc38a039448f3f43e Mon Sep 17 00:00:00 2001 From: Christian Maurer Date: Tue, 15 Oct 2019 12:58:58 +0200 Subject: [PATCH 10/27] c++ remove unused parameters --- src/google/protobuf/extension_set.h | 2 +- src/google/protobuf/message_lite.h | 4 ++-- src/google/protobuf/parse_context.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/google/protobuf/extension_set.h b/src/google/protobuf/extension_set.h index baf54c3182..b704147183 100644 --- a/src/google/protobuf/extension_set.h +++ b/src/google/protobuf/extension_set.h @@ -760,7 +760,7 @@ class PROTOBUF_EXPORT ExtensionSet { bool FindExtension(int wire_type, uint32 field, const MessageLite* containing_type, - const internal::ParseContext* ctx, + const internal::ParseContext* /*ctx*/, ExtensionInfo* extension, bool* was_packed_on_wire) { GeneratedExtensionFinder finder(containing_type); return FindExtensionInfoFromFieldNumber(wire_type, field, &finder, diff --git a/src/google/protobuf/message_lite.h b/src/google/protobuf/message_lite.h index 1ef5d23143..0b718a0a2d 100644 --- a/src/google/protobuf/message_lite.h +++ b/src/google/protobuf/message_lite.h @@ -434,8 +434,8 @@ class PROTOBUF_EXPORT MessageLite { // method.) virtual int GetCachedSize() const = 0; - virtual const char* _InternalParse(const char* ptr, - internal::ParseContext* ctx) { + virtual const char* _InternalParse(const char* /*ptr*/, + internal::ParseContext* /*ctx*/) { return nullptr; } diff --git a/src/google/protobuf/parse_context.h b/src/google/protobuf/parse_context.h index f15cb35f88..fa2a1cdffa 100644 --- a/src/google/protobuf/parse_context.h +++ b/src/google/protobuf/parse_context.h @@ -491,7 +491,7 @@ PROTOBUF_EXPORT std::pair ReadTagFallback(const char* p, uint32 res); // Same as ParseVarint but only accept 5 bytes at most. -inline const char* ReadTag(const char* p, uint32* out, uint32 max_tag = 0) { +inline const char* ReadTag(const char* p, uint32* out, uint32 /*max_tag*/ = 0) { uint32 res = static_cast(p[0]); if (res < 128) { *out = res; From ca25badcbec5ef2d3bae5308012f915c04889fc2 Mon Sep 17 00:00:00 2001 From: Tomo Suzuki Date: Tue, 22 Oct 2019 13:42:35 -0400 Subject: [PATCH 11/27] Installing BOM before running Java test --- tests.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests.sh b/tests.sh index a5919c11ca..771575978c 100755 --- a/tests.sh +++ b/tests.sh @@ -213,6 +213,9 @@ build_java() { build_java_with_conformance_tests() { # Java build needs `protoc`. internal_build_cpp + # This local installation avoids the problem caused by a new version not yet in Maven Central + cd java/bom && $MVN install + cd ../.. cd java && $MVN test && $MVN install cd util && $MVN package assembly:single cd ../.. From 175ddfc1751f99843fe30d333a77868f91a8509c Mon Sep 17 00:00:00 2001 From: Donald Chai Date: Tue, 22 Oct 2019 20:03:51 -0700 Subject: [PATCH 12/27] Add com.google.truth as test dependencies This goes hand-in-hand with other dependencies like junit, but seems to only have been used from util/pom.xml. --- java/core/pom.xml | 5 +++++ java/lite/pom.xml | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/java/core/pom.xml b/java/core/pom.xml index 2bb1ba4307..8c20b1f65e 100644 --- a/java/core/pom.xml +++ b/java/core/pom.xml @@ -37,6 +37,11 @@ guava test + + com.google.truth + truth + test + diff --git a/java/lite/pom.xml b/java/lite/pom.xml index 3f45a17dbe..3343eb91e1 100644 --- a/java/lite/pom.xml +++ b/java/lite/pom.xml @@ -37,6 +37,11 @@ guava test + + com.google.truth + truth + test + From 2fc88527e0a1db37fca3e949326cdc2d3e0fbc43 Mon Sep 17 00:00:00 2001 From: Christian Maurer Date: Tue, 15 Oct 2019 13:36:54 +0200 Subject: [PATCH 13/27] c++ remove warning switch contains default but no case --- src/google/protobuf/compiler/cpp/cpp_helpers.cc | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/google/protobuf/compiler/cpp/cpp_helpers.cc b/src/google/protobuf/compiler/cpp/cpp_helpers.cc index ca8bcba6d1..e064a61a6e 100644 --- a/src/google/protobuf/compiler/cpp/cpp_helpers.cc +++ b/src/google/protobuf/compiler/cpp/cpp_helpers.cc @@ -1723,8 +1723,8 @@ class ParseLoopGenerator { "while (!ctx->Done(&ptr)) {\n" " $uint32$ tag;\n" " ptr = $pi_ns$::ReadTag(ptr, &tag);\n" - " CHK_(ptr);\n" - " switch (tag >> 3) {\n"); + " CHK_(ptr);\n"); + if (!ordered_fields.empty()) format_(" switch (tag >> 3) {\n"); format_.Indent(); format_.Indent(); @@ -1787,7 +1787,7 @@ class ParseLoopGenerator { } // for loop over ordered fields // Default case - format_("default: {\n"); + if (!ordered_fields.empty()) format_("default: {\n"); if (!ordered_fields.empty()) format_("handle_unusual:\n"); format_( " if ((tag & 7) == 4 || tag == 0) {\n" @@ -1828,12 +1828,11 @@ class ParseLoopGenerator { " CHK_(ptr != nullptr);\n" " continue;\n"); } - format_("}\n"); // default case + if (!ordered_fields.empty()) format_("}\n"); // default case format_.Outdent(); format_.Outdent(); - format_( - " } // switch\n" - "} // while\n"); + if (!ordered_fields.empty()) format_(" } // switch\n"); + format_("} // while\n"); } }; From 80734b2e1afe75602101bdc936173b3ae3a58ee8 Mon Sep 17 00:00:00 2001 From: Rafi Kamal Date: Wed, 23 Oct 2019 17:45:23 -0700 Subject: [PATCH 14/27] Reverted changes in conformance/failure_list_js.txt --- conformance/failure_list_js.txt | 114 ++++++++++++++++++++++++++++---- 1 file changed, 101 insertions(+), 13 deletions(-) diff --git a/conformance/failure_list_js.txt b/conformance/failure_list_js.txt index dd123dd6fe..4cb0a052c5 100644 --- a/conformance/failure_list_js.txt +++ b/conformance/failure_list_js.txt @@ -1,16 +1,84 @@ -Required.Proto3.ProtobufInput.ValidDataRepeated.BOOL.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.DOUBLE.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.FIXED32.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.FIXED64.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.INT32.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.INT64.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.SFIXED32.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.SFIXED64.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.SINT32.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.SINT64.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.UINT32.ProtobufOutput -Required.Proto3.ProtobufInput.ValidDataRepeated.UINT64.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.BOOL.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.BOOL.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.BOOL.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.DOUBLE.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.DOUBLE.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.DOUBLE.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.ENUM.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.ENUM.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.ENUM.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.FIXED32.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.FIXED32.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.FIXED32.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.FIXED64.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.FIXED64.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.FIXED64.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.FLOAT.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.FLOAT.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.FLOAT.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.INT32.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.INT32.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.INT32.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.INT64.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.INT64.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.INT64.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.SFIXED32.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.SFIXED32.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.SFIXED32.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.SFIXED64.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.SFIXED64.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.SFIXED64.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.SINT32.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.SINT32.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.SINT32.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.SINT64.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.SINT64.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.SINT64.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.UINT32.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.UINT32.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.UINT32.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.UINT64.PackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.UINT64.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto2.ProtobufInput.ValidDataRepeated.UINT64.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.BOOL.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.BOOL.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.BOOL.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.DOUBLE.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.DOUBLE.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.DOUBLE.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.ENUM.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.ENUM.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.ENUM.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.FIXED32.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.FIXED32.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.FIXED32.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.FIXED64.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.FIXED64.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.FIXED64.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.INT32.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.INT32.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.INT32.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.INT64.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.INT64.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.INT64.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.SFIXED32.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.SFIXED32.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.SFIXED32.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.SFIXED64.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.SFIXED64.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.SFIXED64.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.SINT32.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.SINT32.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.SINT32.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.SINT64.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.SINT64.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.SINT64.UnpackedInput.PackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT32.PackedInput.UnpackedOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT32.UnpackedInput.DefaultOutput.ProtobufOutput +Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT32.UnpackedInput.PackedOutput.ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.PackedInput.UnpackedOutput.ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.UnpackedInput.DefaultOutput.ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.UnpackedInput.PackedOutput.ProtobufOutput @@ -19,8 +87,28 @@ Required.Proto3.ProtobufInput.ValidDataMap.STRING.MESSAGE.MergeValue.ProtobufOut Required.Proto2.ProtobufInput.ValidDataRepeated.BOOL.PackedInput.ProtobufOutput Required.Proto2.ProtobufInput.ValidDataRepeated.DOUBLE.PackedInput.ProtobufOutput Required.Proto2.ProtobufInput.ValidDataRepeated.ENUM.PackedInput.ProtobufOutput +Required.Proto2.ProtobufInput.ValidDataRepeated.FIXED32.PackedInput.ProtobufOutput +Required.Proto2.ProtobufInput.ValidDataRepeated.FIXED64.PackedInput.ProtobufOutput +Required.Proto2.ProtobufInput.ValidDataRepeated.FLOAT.PackedInput.ProtobufOutput +Required.Proto2.ProtobufInput.ValidDataRepeated.INT32.PackedInput.ProtobufOutput +Required.Proto2.ProtobufInput.ValidDataRepeated.INT64.PackedInput.ProtobufOutput +Required.Proto2.ProtobufInput.ValidDataRepeated.SFIXED32.PackedInput.ProtobufOutput +Required.Proto2.ProtobufInput.ValidDataRepeated.SFIXED64.PackedInput.ProtobufOutput +Required.Proto2.ProtobufInput.ValidDataRepeated.SINT32.PackedInput.ProtobufOutput Required.Proto2.ProtobufInput.ValidDataRepeated.SINT64.PackedInput.ProtobufOutput Required.Proto2.ProtobufInput.ValidDataRepeated.UINT32.PackedInput.ProtobufOutput Required.Proto2.ProtobufInput.ValidDataRepeated.UINT64.PackedInput.ProtobufOutput Required.Proto3.ProtobufInput.ValidDataRepeated.BOOL.UnpackedInput.ProtobufOutput Required.Proto3.ProtobufInput.ValidDataRepeated.DOUBLE.UnpackedInput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.ENUM.UnpackedInput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.FIXED32.UnpackedInput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.FIXED64.UnpackedInput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.FLOAT.UnpackedInput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.INT32.UnpackedInput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.INT64.UnpackedInput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.SFIXED32.UnpackedInput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.SFIXED64.UnpackedInput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.SINT32.UnpackedInput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.SINT64.UnpackedInput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.UINT32.UnpackedInput.ProtobufOutput +Required.Proto3.ProtobufInput.ValidDataRepeated.UINT64.UnpackedInput.ProtobufOutput From 82db4520a12e8f2a15c6fa8bccc4a883b55f966e Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Mon, 28 Oct 2019 06:57:39 -0400 Subject: [PATCH 15/27] Update to 3.10.0 in docs --- java/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/java/README.md b/java/README.md index ccd2c97ba9..a10949489e 100644 --- a/java/README.md +++ b/java/README.md @@ -23,7 +23,7 @@ If you are using Maven, use the following: com.google.protobuf protobuf-java - 3.9.2 + 3.10.0 ``` @@ -37,7 +37,7 @@ protobuf-java-util package: com.google.protobuf protobuf-java-util - 3.9.2 + 3.10.0 ``` @@ -45,7 +45,7 @@ protobuf-java-util package: If you are using Gradle, add the following to your `build.gradle` file's dependencies: ``` - compile 'com.google.protobuf:protobuf-java:3.9.2' + compile 'com.google.protobuf:protobuf-java:3.10.0' ``` Again, be sure to check that the version number maches (or is newer than) the version number of protoc that you are using. From ed69e88695bbbeadd7aa332b439396d14ee97f59 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Mon, 28 Oct 2019 07:09:46 -0400 Subject: [PATCH 16/27] update error_prone and gson --- java/util/pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/util/pom.xml b/java/util/pom.xml index f0777b99c1..54a35817c7 100644 --- a/java/util/pom.xml +++ b/java/util/pom.xml @@ -25,7 +25,7 @@ com.google.errorprone error_prone_annotations - 2.3.2 + 2.3.3 com.google.guava @@ -35,7 +35,7 @@ com.google.code.gson gson - 2.8.5 + 2.8.6 junit From 29d650aefbba4fcb47d44d9ef3399baefa60f993 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Thu, 24 Oct 2019 20:02:07 +0200 Subject: [PATCH 17/27] [bazel] Add `strip_prefix` to download of @six This way, we can avoid an unnecessary copy. --- protobuf_deps.bzl | 1 + third_party/six.BUILD | 15 +++++---------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/protobuf_deps.bzl b/protobuf_deps.bzl index 9209b1c5a3..b5971a5a37 100644 --- a/protobuf_deps.bzl +++ b/protobuf_deps.bzl @@ -27,6 +27,7 @@ def protobuf_deps(): name = "six", build_file = "@com_google_protobuf//:third_party/six.BUILD", sha256 = "d16a0141ec1a18405cd4ce8b4613101da75da0e9a7aec5bdd4fa804d0e0eba73", + strip_prefix = "six-1.12.0", urls = ["https://pypi.python.org/packages/source/s/six/six-1.12.0.tar.gz"], ) diff --git a/third_party/six.BUILD b/third_party/six.BUILD index 88c75520de..327049cd14 100644 --- a/third_party/six.BUILD +++ b/third_party/six.BUILD @@ -1,13 +1,8 @@ -genrule( - name = "copy_six", - srcs = ["six-1.12.0/six.py"], - outs = ["__init__.py"], - cmd = "cp $< $(@)", -) +load("@rules_python//python:defs.bzl", "py_library") py_library( - name = "six", - srcs = ["__init__.py"], - srcs_version = "PY2AND3", - visibility = ["//visibility:public"], + name = "six", + srcs = ["six.py"], + srcs_version = "PY2AND3", + visibility = ["//visibility:public"], ) From e907a8781d7fcec5f95a04fe19eedf23d7396f34 Mon Sep 17 00:00:00 2001 From: Yannic Date: Fri, 25 Oct 2019 14:12:16 +0200 Subject: [PATCH 18/27] Update protobuf_deps.bzl --- protobuf_deps.bzl | 1 - 1 file changed, 1 deletion(-) diff --git a/protobuf_deps.bzl b/protobuf_deps.bzl index b5971a5a37..9209b1c5a3 100644 --- a/protobuf_deps.bzl +++ b/protobuf_deps.bzl @@ -27,7 +27,6 @@ def protobuf_deps(): name = "six", build_file = "@com_google_protobuf//:third_party/six.BUILD", sha256 = "d16a0141ec1a18405cd4ce8b4613101da75da0e9a7aec5bdd4fa804d0e0eba73", - strip_prefix = "six-1.12.0", urls = ["https://pypi.python.org/packages/source/s/six/six-1.12.0.tar.gz"], ) From 3581ee24e98eee66aef442590f61ff4726de3bd8 Mon Sep 17 00:00:00 2001 From: Yannic Date: Fri, 25 Oct 2019 14:12:36 +0200 Subject: [PATCH 19/27] Update six.BUILD --- third_party/six.BUILD | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/third_party/six.BUILD b/third_party/six.BUILD index 327049cd14..5d66ba7d61 100644 --- a/third_party/six.BUILD +++ b/third_party/six.BUILD @@ -1,8 +1,19 @@ load("@rules_python//python:defs.bzl", "py_library") +# Consume `six.py` as `__init__.py` for compatibility +# with `--incompatible_default_to_explicit_init_py`. +# https://github.com/protocolbuffers/protobuf/pull/6795#issuecomment-546060749 +# https://github.com/bazelbuild/bazel/issues/10076 +genrule( + name = "copy_six", + srcs = ["six-1.12.0/six.py"], + outs = ["__init__.py"], + cmd = "cp $< $(@)", +) + py_library( - name = "six", - srcs = ["six.py"], - srcs_version = "PY2AND3", - visibility = ["//visibility:public"], + name = "six", + srcs = ["__init__.py"], + srcs_version = "PY2AND3", + visibility = ["//visibility:public"], ) From fc7c65a258172e5e6fdb7b914b5aee8fd808b643 Mon Sep 17 00:00:00 2001 From: Yannic Date: Fri, 25 Oct 2019 14:13:41 +0200 Subject: [PATCH 20/27] Update six.BUILD --- third_party/six.BUILD | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/third_party/six.BUILD b/third_party/six.BUILD index 5d66ba7d61..041c72c66e 100644 --- a/third_party/six.BUILD +++ b/third_party/six.BUILD @@ -5,15 +5,15 @@ load("@rules_python//python:defs.bzl", "py_library") # https://github.com/protocolbuffers/protobuf/pull/6795#issuecomment-546060749 # https://github.com/bazelbuild/bazel/issues/10076 genrule( - name = "copy_six", - srcs = ["six-1.12.0/six.py"], - outs = ["__init__.py"], - cmd = "cp $< $(@)", + name = "copy_six", + srcs = ["six-1.12.0/six.py"], + outs = ["__init__.py"], + cmd = "cp $< $(@)", ) py_library( - name = "six", - srcs = ["__init__.py"], - srcs_version = "PY2AND3", - visibility = ["//visibility:public"], + name = "six", + srcs = ["__init__.py"], + srcs_version = "PY2AND3", + visibility = ["//visibility:public"], ) From 4252b7aff3ac38d09b7ea6dba3b603d71561330c Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Fri, 25 Oct 2019 14:49:07 +0200 Subject: [PATCH 21/27] Fix broken javadoc generation for repeated enum values --- src/google/protobuf/compiler/java/java_enum_field.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/google/protobuf/compiler/java/java_enum_field.cc b/src/google/protobuf/compiler/java/java_enum_field.cc index 5f77f042c3..65dd35364e 100644 --- a/src/google/protobuf/compiler/java/java_enum_field.cc +++ b/src/google/protobuf/compiler/java/java_enum_field.cc @@ -656,7 +656,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateMembers( " java.lang.Integer, $type$>($name$_, $name$_converter_);\n" "}\n"); printer->Annotate("{", "}", descriptor_); - WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER); + WriteFieldAccessorDocComment(printer, descriptor_, LIST_COUNT); printer->Print( variables_, "$deprecation$public int ${$get$capitalized_name$Count$}$() {\n" From a971a4ef1f4e9a165c30a852d83c84fd46f8267f Mon Sep 17 00:00:00 2001 From: Tapasweni Pathak Date: Tue, 24 Sep 2019 21:44:32 +0530 Subject: [PATCH 22/27] Fix: Undefined behavior in UTF8GenericScanFastAscii --- src/google/protobuf/stubs/structurally_valid.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/google/protobuf/stubs/structurally_valid.cc b/src/google/protobuf/stubs/structurally_valid.cc index 0598427d20..afbeab481a 100644 --- a/src/google/protobuf/stubs/structurally_valid.cc +++ b/src/google/protobuf/stubs/structurally_valid.cc @@ -504,6 +504,8 @@ int UTF8GenericScanFastAscii(const UTF8ScanObj* st, const uint8* isrc = reinterpret_cast(str); const uint8* src = isrc; const uint8* srclimit = isrc + str_length; + if (str_length < 7) + throw std::runtime_error("You are implementing a segmentation fault"); const uint8* srclimit8 = srclimit - 7; int n; int rest_consumed; From fbbbc45ae5167e395f2132189c09e088dc7a6775 Mon Sep 17 00:00:00 2001 From: Tapasweni Pathak Date: Wed, 16 Oct 2019 23:53:48 +0530 Subject: [PATCH 23/27] address review --- src/google/protobuf/stubs/structurally_valid.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/google/protobuf/stubs/structurally_valid.cc b/src/google/protobuf/stubs/structurally_valid.cc index afbeab481a..5b2d5d052c 100644 --- a/src/google/protobuf/stubs/structurally_valid.cc +++ b/src/google/protobuf/stubs/structurally_valid.cc @@ -504,8 +504,9 @@ int UTF8GenericScanFastAscii(const UTF8ScanObj* st, const uint8* isrc = reinterpret_cast(str); const uint8* src = isrc; const uint8* srclimit = isrc + str_length; - if (str_length < 7) - throw std::runtime_error("You are implementing a segmentation fault"); + if (srclimit.length() < 7) + *srclimit += `\0\0\0\0\0\0\0`; + const uint8* srclimit8 = srclimit - 7; int n; int rest_consumed; From 50fb80cdd86c9e0a6b6ec560c1039e011c91f477 Mon Sep 17 00:00:00 2001 From: Tapasweni Pathak Date: Thu, 24 Oct 2019 12:32:24 +0530 Subject: [PATCH 24/27] fix check for undefined behavior --- src/google/protobuf/stubs/structurally_valid.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/google/protobuf/stubs/structurally_valid.cc b/src/google/protobuf/stubs/structurally_valid.cc index 5b2d5d052c..9cd0324c6e 100644 --- a/src/google/protobuf/stubs/structurally_valid.cc +++ b/src/google/protobuf/stubs/structurally_valid.cc @@ -395,7 +395,7 @@ int UTF8GenericScan(const UTF8ScanObj* st, const uint8* isrc = reinterpret_cast(str); const uint8* src = isrc; const uint8* srclimit = isrc + str_length; - const uint8* srclimit8 = srclimit - 7; + const uint8* srclimit8 = str_length < 7 ? isrc : srclimit - 7; const uint8* Tbl_0 = &st->state_table[st->state0]; DoAgain: From 7ce420298a2562f95c492e175e9f7f61e3bd9631 Mon Sep 17 00:00:00 2001 From: Tapasweni Pathak Date: Thu, 24 Oct 2019 12:34:50 +0530 Subject: [PATCH 25/27] fix one more line --- src/google/protobuf/stubs/structurally_valid.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/google/protobuf/stubs/structurally_valid.cc b/src/google/protobuf/stubs/structurally_valid.cc index 9cd0324c6e..788b224d22 100644 --- a/src/google/protobuf/stubs/structurally_valid.cc +++ b/src/google/protobuf/stubs/structurally_valid.cc @@ -504,10 +504,7 @@ int UTF8GenericScanFastAscii(const UTF8ScanObj* st, const uint8* isrc = reinterpret_cast(str); const uint8* src = isrc; const uint8* srclimit = isrc + str_length; - if (srclimit.length() < 7) - *srclimit += `\0\0\0\0\0\0\0`; - - const uint8* srclimit8 = srclimit - 7; + const uint8* srclimit8 = str_length < 7 ? isrc : srclimit - 7; int n; int rest_consumed; int exit_reason; From c53e5b8e11659528150737f285e5c171b0f159b2 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Tue, 29 Oct 2019 12:48:24 -0700 Subject: [PATCH 26/27] Avoid too much overhead in layout_init (#6716) * Avoid initializing primitive fields in layout_init * Avoid initializing string/bytes/message fields in layout_init * Lazily create map when needed * Lazily create repeated fields * Change layout_init to only do memcpy * Fix test for php-7.0 * Fix conformance test where default value of string/message map is not encoded * Fix test for zts * Clean up * Fix comments --- .gitignore | 2 + php/ext/google/protobuf/array.c | 13 ++ php/ext/google/protobuf/encode_decode.c | 222 +++++++++++++++--------- php/ext/google/protobuf/map.c | 12 ++ php/ext/google/protobuf/message.c | 9 +- php/ext/google/protobuf/protobuf.h | 15 +- php/ext/google/protobuf/storage.c | 162 +++++++++-------- 7 files changed, 271 insertions(+), 164 deletions(-) diff --git a/.gitignore b/.gitignore index dbbec53211..4e909aecea 100644 --- a/.gitignore +++ b/.gitignore @@ -139,6 +139,8 @@ composer.lock php/tests/generated/ php/tests/old_protoc php/tests/protobuf/ +php/tests/core +php/tests/vgcore* php/ext/google/protobuf/.libs/ php/ext/google/protobuf/Makefile.fragments php/ext/google/protobuf/Makefile.global diff --git a/php/ext/google/protobuf/array.c b/php/ext/google/protobuf/array.c index e69bef4299..b52bdf62ca 100644 --- a/php/ext/google/protobuf/array.c +++ b/php/ext/google/protobuf/array.c @@ -259,6 +259,19 @@ void repeated_field_push_native(RepeatedField *intern, void *value) { } } +void repeated_field_ensure_created( + const upb_fielddef *field, + CACHED_VALUE *repeated_field PHP_PROTO_TSRMLS_DC) { + if (ZVAL_IS_NULL(CACHED_PTR_TO_ZVAL_PTR(repeated_field))) { + zval_ptr_dtor(repeated_field); +#if PHP_MAJOR_VERSION < 7 + MAKE_STD_ZVAL(CACHED_PTR_TO_ZVAL_PTR(repeated_field)); +#endif + repeated_field_create_with_field(repeated_field_type, field, + repeated_field PHP_PROTO_TSRMLS_CC); + } +} + void repeated_field_create_with_field( zend_class_entry *ce, const upb_fielddef *field, CACHED_VALUE *repeated_field PHP_PROTO_TSRMLS_DC) { diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index 166fb91bab..39304a5711 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -139,6 +139,15 @@ static const void* newhandlerdata(upb_handlers* h, uint32_t ofs) { return hd_ofs; } +static const void* newhandlerfielddata( + upb_handlers* h, const upb_fielddef* field) { + const void** hd_field = malloc(sizeof(void*)); + PHP_PROTO_ASSERT(hd_field != NULL); + *hd_field = field; + upb_handlers_addcleanup(h, hd_field, free); + return hd_field; +} + typedef struct { void* closure; stringsink sink; @@ -163,16 +172,18 @@ static const void *newunknownfieldshandlerdata(upb_handlers* h) { } typedef struct { + const upb_fielddef *fd; size_t ofs; const upb_msgdef *md; } submsg_handlerdata_t; -// Creates a handlerdata that contains offset and submessage type information. +// Creates a handlerdata that contains field and submessage type information. static const void *newsubmsghandlerdata(upb_handlers* h, uint32_t ofs, const upb_fielddef* f) { submsg_handlerdata_t* hd = (submsg_handlerdata_t*)malloc(sizeof(submsg_handlerdata_t)); PHP_PROTO_ASSERT(hd != NULL); + hd->fd = f; hd->ofs = ofs; hd->md = upb_fielddef_msgsubdef(f); upb_handlers_addcleanup(h, hd, free); @@ -221,8 +232,11 @@ static const void *newoneofhandlerdata(upb_handlers *h, // this field (such an instance always exists even in an empty message). static void *startseq_handler(void* closure, const void* hd) { MessageHeader* msg = closure; - const size_t *ofs = hd; - return CACHED_PTR_TO_ZVAL_PTR(DEREF(message_data(msg), *ofs, CACHED_VALUE*)); + const upb_fielddef** field = (const upb_fielddef**) hd; + CACHED_VALUE* cache = find_zval_property(msg, *field); + TSRMLS_FETCH(); + repeated_field_ensure_created(*field, cache PHP_PROTO_TSRMLS_CC); + return CACHED_PTR_TO_ZVAL_PTR(cache); } // Handlers that append primitive values to a repeated field. @@ -322,15 +336,6 @@ static void *empty_php_string(zval* value_ptr) { } #endif #if PHP_MAJOR_VERSION < 7 -static void *empty_php_string2(zval** value_ptr) { - SEPARATE_ZVAL_IF_NOT_REF(value_ptr); - if (Z_TYPE_PP(value_ptr) == IS_STRING && - !IS_INTERNED(Z_STRVAL_PP(value_ptr))) { - FREE(Z_STRVAL_PP(value_ptr)); - } - ZVAL_EMPTY_STRING(*value_ptr); - return (void*)(*value_ptr); -} static void new_php_string(zval** value_ptr, const char* str, size_t len) { SEPARATE_ZVAL_IF_NOT_REF(value_ptr); if (Z_TYPE_PP(value_ptr) == IS_STRING && @@ -340,13 +345,6 @@ static void new_php_string(zval** value_ptr, const char* str, size_t len) { ZVAL_STRINGL(*value_ptr, str, len, 1); } #else -static void *empty_php_string2(zval* value_ptr) { - if (Z_TYPE_P(value_ptr) == IS_STRING) { - zend_string_release(Z_STR_P(value_ptr)); - } - ZVAL_EMPTY_STRING(value_ptr); - return value_ptr; -} static void new_php_string(zval* value_ptr, const char* str, size_t len) { if (Z_TYPE_P(value_ptr) == IS_STRING) { zend_string_release(Z_STR_P(value_ptr)); @@ -371,6 +369,21 @@ static void* str_handler(void *closure, } static bool str_end_handler(void *closure, const void *hd) { + stringfields_parseframe_t* frame = closure; + const upb_fielddef **field = (const upb_fielddef **) hd; + MessageHeader* msg = (MessageHeader*)frame->closure; + + CACHED_VALUE* cached = find_zval_property(msg, *field); + + new_php_string(cached, frame->sink.ptr, frame->sink.len); + + stringsink_uninit(&frame->sink); + free(frame); + + return true; +} + +static bool map_str_end_handler(void *closure, const void *hd) { stringfields_parseframe_t* frame = closure; const size_t *ofs = hd; MessageHeader* msg = (MessageHeader*)frame->closure; @@ -430,26 +443,60 @@ static void *submsg_handler(void *closure, const void *hd) { zval* submsg_php; MessageHeader* submsg; - if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(DEREF(message_data(msg), submsgdata->ofs, - CACHED_VALUE*))) == IS_NULL) { + CACHED_VALUE* cached = find_zval_property(msg, submsgdata->fd); + + if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(cached)) == IS_NULL) { #if PHP_MAJOR_VERSION < 7 zval val; ZVAL_OBJ(&val, subklass->create_object(subklass TSRMLS_CC)); MessageHeader* intern = UNBOX(MessageHeader, &val); custom_data_init(subklass, intern PHP_PROTO_TSRMLS_CC); - REPLACE_ZVAL_VALUE(DEREF(message_data(msg), submsgdata->ofs, zval**), - &val, 1); + REPLACE_ZVAL_VALUE(cached, &val, 1); zval_dtor(&val); #else zend_object* obj = subklass->create_object(subklass TSRMLS_CC); - ZVAL_OBJ(DEREF(message_data(msg), submsgdata->ofs, zval*), obj); + ZVAL_OBJ(cached, obj); MessageHeader* intern = UNBOX_HASHTABLE_VALUE(MessageHeader, obj); custom_data_init(subklass, intern PHP_PROTO_TSRMLS_CC); #endif } - submsg_php = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), submsgdata->ofs, CACHED_VALUE*)); + submsg_php = CACHED_PTR_TO_ZVAL_PTR(cached); + + submsg = UNBOX(MessageHeader, submsg_php); + return submsg; +} + +static void *map_submsg_handler(void *closure, const void *hd) { + MessageHeader* msg = closure; + const submsg_handlerdata_t* submsgdata = hd; + TSRMLS_FETCH(); + Descriptor* subdesc = + UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj((void*)submsgdata->md)); + zend_class_entry* subklass = subdesc->klass; + zval* submsg_php; + MessageHeader* submsg; + + CACHED_VALUE* cached = + DEREF(message_data(msg), submsgdata->ofs, CACHED_VALUE*); + + if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(cached)) == IS_NULL) { +#if PHP_MAJOR_VERSION < 7 + zval val; + ZVAL_OBJ(&val, subklass->create_object(subklass TSRMLS_CC)); + MessageHeader* intern = UNBOX(MessageHeader, &val); + custom_data_init(subklass, intern PHP_PROTO_TSRMLS_CC); + REPLACE_ZVAL_VALUE(cached, &val, 1); + zval_dtor(&val); +#else + zend_object* obj = subklass->create_object(subklass TSRMLS_CC); + ZVAL_OBJ(cached, obj); + MessageHeader* intern = UNBOX_HASHTABLE_VALUE(MessageHeader, obj); + custom_data_init(subklass, intern PHP_PROTO_TSRMLS_CC); +#endif + } + + submsg_php = CACHED_PTR_TO_ZVAL_PTR(cached); submsg = UNBOX(MessageHeader, submsg_php); return submsg; @@ -457,7 +504,7 @@ static void *submsg_handler(void *closure, const void *hd) { // Handler data for startmap/endmap handlers. typedef struct { - size_t ofs; + const upb_fielddef* fd; const upb_msgdef* value_md; upb_fieldtype_t key_field_type; upb_fieldtype_t value_field_type; @@ -612,9 +659,10 @@ static void map_slot_value(upb_fieldtype_t type, const void* from, static void *startmapentry_handler(void *closure, const void *hd) { MessageHeader* msg = closure; const map_handlerdata_t* mapdata = hd; + CACHED_VALUE* cache = find_zval_property(msg, mapdata->fd); TSRMLS_FETCH(); - zval* map = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), mapdata->ofs, CACHED_VALUE*)); + map_field_ensure_created(mapdata->fd, cache PHP_PROTO_TSRMLS_CC); + zval* map = CACHED_PTR_TO_ZVAL_PTR(cache); map_parse_frame_t* frame = ALLOC(map_parse_frame_t); frame->data = ALLOC(map_parse_frame_data_t); @@ -662,7 +710,7 @@ static bool endmap_handler(void* closure, const void* hd, upb_status* s) { // key/value and endmsg handlers. The reason is that there is no easy way to // pass the handlerdata down to the sub-message handler setup. static map_handlerdata_t* new_map_handlerdata( - size_t ofs, + const upb_fielddef* field, const upb_msgdef* mapentry_def, Descriptor* desc) { const upb_fielddef* key_field; @@ -671,7 +719,7 @@ static map_handlerdata_t* new_map_handlerdata( map_handlerdata_t* hd = (map_handlerdata_t*)malloc(sizeof(map_handlerdata_t)); PHP_PROTO_ASSERT(hd != NULL); - hd->ofs = ofs; + hd->fd = field; key_field = upb_msgdef_itof(mapentry_def, MAP_KEY_FIELD); PHP_PROTO_ASSERT(key_field != NULL); hd->key_field_type = upb_fielddef_type(key_field); @@ -844,7 +892,7 @@ static void add_handlers_for_repeated_field(upb_handlers *h, const upb_fielddef *f, size_t offset) { upb_handlerattr attr = UPB_HANDLERATTR_INIT; - attr.handler_data = newhandlerdata(h, offset); + attr.handler_data = newhandlerfielddata(h, f); upb_handlers_setstartseq(h, f, startseq_handler, &attr); switch (upb_fielddef_type(f)) { @@ -884,7 +932,7 @@ static void add_handlers_for_repeated_field(upb_handlers *h, // Set up handlers for a singular field. static void add_handlers_for_singular_field(upb_handlers *h, const upb_fielddef *f, - size_t offset) { + size_t offset, bool is_map) { switch (upb_fielddef_type(f)) { #define SET_HANDLER(utype, ltype) \ case utype: { \ @@ -908,16 +956,29 @@ static void add_handlers_for_singular_field(upb_handlers *h, case UPB_TYPE_STRING: case UPB_TYPE_BYTES: { upb_handlerattr attr = UPB_HANDLERATTR_INIT; - attr.handler_data = newhandlerdata(h, offset); + if (is_map) { + attr.handler_data = newhandlerdata(h, offset); + } else { + attr.handler_data = newhandlerfielddata(h, f); + } upb_handlers_setstartstr(h, f, str_handler, &attr); upb_handlers_setstring(h, f, stringdata_handler, &attr); - upb_handlers_setendstr(h, f, str_end_handler, &attr); + if (is_map) { + upb_handlers_setendstr(h, f, map_str_end_handler, &attr); + } else { + upb_handlers_setendstr(h, f, str_end_handler, &attr); + } break; } case UPB_TYPE_MESSAGE: { upb_handlerattr attr = UPB_HANDLERATTR_INIT; - attr.handler_data = newsubmsghandlerdata(h, offset, f); - upb_handlers_setstartsubmsg(h, f, submsg_handler, &attr); + if (is_map) { + attr.handler_data = newsubmsghandlerdata(h, offset, f); + upb_handlers_setstartsubmsg(h, f, map_submsg_handler, &attr); + } else { + attr.handler_data = newsubmsghandlerdata(h, 0, f); + upb_handlers_setstartsubmsg(h, f, submsg_handler, &attr); + } break; } } @@ -929,7 +990,7 @@ static void add_handlers_for_mapfield(upb_handlers* h, size_t offset, Descriptor* desc) { const upb_msgdef* map_msgdef = upb_fielddef_msgsubdef(fielddef); - map_handlerdata_t* hd = new_map_handlerdata(offset, map_msgdef, desc); + map_handlerdata_t* hd = new_map_handlerdata(fielddef, map_msgdef, desc); upb_handlerattr attr = UPB_HANDLERATTR_INIT; upb_handlers_addcleanup(h, hd, free); @@ -951,10 +1012,10 @@ static void add_handlers_for_mapentry(const upb_msgdef* msgdef, upb_handlers* h, add_handlers_for_singular_field(h, key_field, offsetof(map_parse_frame_data_t, - key_storage)); + key_storage), true); add_handlers_for_singular_field(h, value_field, offsetof(map_parse_frame_data_t, - value_storage)); + value_storage), true); } // Set up handlers for a oneof field. @@ -1063,7 +1124,7 @@ void add_handlers_for_message(const void* closure, upb_handlers* h) { } else if (upb_fielddef_isseq(f)) { add_handlers_for_repeated_field(h, f, offset); } else { - add_handlers_for_singular_field(h, f, offset); + add_handlers_for_singular_field(h, f, offset, false); } } } @@ -1259,16 +1320,13 @@ static void putjsonany(MessageHeader* msg, const Descriptor* desc, const upb_fielddef* type_field = upb_msgdef_itof(desc->msgdef, UPB_ANY_TYPE); const upb_fielddef* value_field = upb_msgdef_itof(desc->msgdef, UPB_ANY_VALUE); - uint32_t type_url_offset; zval* type_url_php_str; const upb_msgdef *payload_type = NULL; upb_sink_startmsg(sink); /* Handle type url */ - type_url_offset = desc->layout->fields[upb_fielddef_index(type_field)].offset; - type_url_php_str = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), type_url_offset, CACHED_VALUE*)); + type_url_php_str = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, type_field)); if (Z_STRLEN_P(type_url_php_str) > 0) { putstr(type_url_php_str, type_field, sink, false); } @@ -1294,14 +1352,11 @@ static void putjsonany(MessageHeader* msg, const Descriptor* desc, } { - uint32_t value_offset; zval* value_php_str; const char* value_str; size_t value_len; - value_offset = desc->layout->fields[upb_fielddef_index(value_field)].offset; - value_php_str = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), value_offset, CACHED_VALUE*)); + value_php_str = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, value_field)); value_str = Z_STRVAL_P(value_php_str); value_len = Z_STRLEN_P(value_php_str); @@ -1355,17 +1410,21 @@ static void putjsonlistvalue( upb_sink_startmsg(sink); - array = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); - intern = UNBOX(RepeatedField, array); - ht = PHP_PROTO_HASH_OF(intern->array); - size = zend_hash_num_elements(ht); - - if (size == 0) { + array = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); + if (ZVAL_IS_NULL(array)) { upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); } else { - putarray(array, f, sink, depth, true TSRMLS_CC); + intern = UNBOX(RepeatedField, array); + ht = PHP_PROTO_HASH_OF(intern->array); + size = zend_hash_num_elements(ht); + + if (size == 0) { + upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); + upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); + } else { + putarray(array, f, sink, depth, true TSRMLS_CC); + } } upb_sink_endmsg(sink, &status); @@ -1384,16 +1443,20 @@ static void putjsonstruct( upb_sink_startmsg(sink); - map = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); - intern = UNBOX(Map, map); - size = upb_strtable_count(&intern->table); - - if (size == 0) { + map = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); + if (ZVAL_IS_NULL(map)) { upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); } else { - putmap(map, f, sink, depth, true TSRMLS_CC); + intern = UNBOX(Map, map); + size = upb_strtable_count(&intern->table); + + if (size == 0) { + upb_sink_startseq(sink, getsel(f, UPB_HANDLER_STARTSEQ), &subsink); + upb_sink_endseq(sink, getsel(f, UPB_HANDLER_ENDSEQ)); + } else { + putmap(map, f, sink, depth, true TSRMLS_CC); + } } upb_sink_endmsg(sink, &status); @@ -1455,28 +1518,24 @@ static void putrawmsg(MessageHeader* msg, const Descriptor* desc, } if (is_map_field(f)) { - zval* map = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); - if (map != NULL) { + zval* map = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); + if (!ZVAL_IS_NULL(map)) { putmap(map, f, sink, depth, is_json TSRMLS_CC); } } else if (upb_fielddef_isseq(f)) { - zval* array = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); - if (array != NULL) { + zval* array = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); + if (!ZVAL_IS_NULL(array)) { putarray(array, f, sink, depth, is_json TSRMLS_CC); } } else if (upb_fielddef_isstring(f)) { - zval* str = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); + zval* str = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); if (containing_oneof || (is_json && is_wrapper_msg(desc->msgdef)) || Z_STRLEN_P(str) > 0) { putstr(str, f, sink, is_json && is_wrapper_msg(desc->msgdef)); } } else if (upb_fielddef_issubmsg(f)) { - putsubmsg(CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)), - f, sink, depth, is_json TSRMLS_CC); + zval* submsg = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); + putsubmsg(submsg, f, sink, depth, is_json TSRMLS_CC); } else { upb_selector_t sel = getsel(f, upb_handlers_getprimitivehandlertype(f)); @@ -1847,9 +1906,8 @@ static void discard_unknown_fields(MessageHeader* msg) { value_field = map_field_value(f); if (!upb_fielddef_issubmsg(value_field)) continue; - zval* map_php = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); - if (map_php == NULL) continue; + zval* map_php = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); + if (ZVAL_IS_NULL(map_php)) continue; Map* intern = UNBOX(Map, map_php); for (map_begin(map_php, &map_it TSRMLS_CC); @@ -1868,9 +1926,8 @@ static void discard_unknown_fields(MessageHeader* msg) { } else if (upb_fielddef_isseq(f)) { if (!upb_fielddef_issubmsg(f)) continue; - zval* array_php = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); - if (array_php == NULL) continue; + zval* array_php = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); + if (ZVAL_IS_NULL(array_php)) continue; int size, i; RepeatedField* intern = UNBOX(RepeatedField, array_php); @@ -1890,8 +1947,7 @@ static void discard_unknown_fields(MessageHeader* msg) { discard_unknown_fields(submsg); } } else if (upb_fielddef_issubmsg(f)) { - zval* submsg_php = CACHED_PTR_TO_ZVAL_PTR( - DEREF(message_data(msg), offset, CACHED_VALUE*)); + zval* submsg_php = CACHED_PTR_TO_ZVAL_PTR(find_zval_property(msg, f)); if (Z_TYPE_P(submsg_php) == IS_NULL) continue; MessageHeader* submsg = UNBOX(MessageHeader, submsg_php); discard_unknown_fields(submsg); diff --git a/php/ext/google/protobuf/map.c b/php/ext/google/protobuf/map.c index 0ce10190e6..2764788b46 100644 --- a/php/ext/google/protobuf/map.c +++ b/php/ext/google/protobuf/map.c @@ -243,6 +243,18 @@ map_field_handlers->write_dimension = map_field_write_dimension; map_field_handlers->get_gc = map_field_get_gc; PHP_PROTO_INIT_CLASS_END +void map_field_ensure_created(const upb_fielddef *field, + CACHED_VALUE *map_field PHP_PROTO_TSRMLS_DC) { + if (ZVAL_IS_NULL(CACHED_PTR_TO_ZVAL_PTR(map_field))) { + zval_ptr_dtor(map_field); +#if PHP_MAJOR_VERSION < 7 + MAKE_STD_ZVAL(CACHED_PTR_TO_ZVAL_PTR(map_field)); +#endif + map_field_create_with_field(map_field_type, field, + map_field PHP_PROTO_TSRMLS_CC); + } +} + void map_field_create_with_field(const zend_class_entry *ce, const upb_fielddef *field, CACHED_VALUE *map_field PHP_PROTO_TSRMLS_DC) { diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 303f5d4741..03dec75928 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -178,7 +178,7 @@ static zval* message_get_property_internal(zval* object, zend_get_property_info(Z_OBJCE_P(object), Z_STR_P(member), true); #endif return layout_get( - self->descriptor->layout, message_data(self), field, + self->descriptor->layout, self, field, OBJ_PROP(Z_OBJ_P(object), property_info->offset) TSRMLS_CC); } @@ -191,7 +191,7 @@ static void message_get_oneof_property_internal(zval* object, zval* member, return; } - layout_get(self->descriptor->layout, message_data(self), field, + layout_get(self->descriptor->layout, self, field, ZVAL_PTR_TO_CACHED_PTR(return_value) TSRMLS_CC); } @@ -255,7 +255,6 @@ void custom_data_init(const zend_class_entry* ce, MessageHeader* intern PHP_PROTO_TSRMLS_DC) { Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_ce_obj(ce)); intern->data = ALLOC_N(uint8_t, desc->layout->size); - memset(message_data(intern), 0, desc->layout->size); // We wrap first so that everything in the message object is GC-rooted in // case a collection happens during object creation in layout_init(). intern->descriptor = desc; @@ -575,9 +574,9 @@ PHP_METHOD(Message, readOneof) { const upb_fielddef* field = upb_msgdef_itof(msg->descriptor->msgdef, index); // Unlike singular fields, oneof fields share cached property. So we cannot - // let lay_get modify the cached property. Instead, we pass in the return + // let layout_get modify the cached property. Instead, we pass in the return // value directly. - layout_get(msg->descriptor->layout, message_data(msg), field, + layout_get(msg->descriptor->layout, msg, field, ZVAL_PTR_TO_CACHED_PTR(return_value) TSRMLS_CC); } diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 4abfecbdf5..86bc5b3317 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -200,7 +200,7 @@ #define CACHED_VALUE zval* #define CACHED_TO_ZVAL_PTR(VALUE) (VALUE) -#define CACHED_PTR_TO_ZVAL_PTR(VALUE) (*VALUE) +#define CACHED_PTR_TO_ZVAL_PTR(VALUE) (*(CACHED_VALUE*)(VALUE)) #define ZVAL_PTR_TO_CACHED_PTR(VALUE) (&VALUE) #define ZVAL_PTR_TO_CACHED_VALUE(VALUE) (VALUE) #define ZVAL_TO_CACHED_VALUE(VALUE) (&VALUE) @@ -475,7 +475,7 @@ static inline int php_proto_zend_hash_get_current_data_ex(HashTable* ht, #define CACHED_VALUE zval #define CACHED_TO_ZVAL_PTR(VALUE) (&VALUE) -#define CACHED_PTR_TO_ZVAL_PTR(VALUE) (VALUE) +#define CACHED_PTR_TO_ZVAL_PTR(VALUE) ((CACHED_VALUE*)(VALUE)) #define ZVAL_PTR_TO_CACHED_PTR(VALUE) (VALUE) #define ZVAL_PTR_TO_CACHED_VALUE(VALUE) (*VALUE) #define ZVAL_TO_CACHED_VALUE(VALUE) (VALUE) @@ -935,6 +935,7 @@ struct MessageField { struct MessageLayout { const upb_msgdef* msgdef; + void* empty_template; // Can memcpy() onto a layout to clear it. MessageField* fields; size_t size; }; @@ -948,7 +949,7 @@ PHP_PROTO_WRAP_OBJECT_END MessageLayout* create_layout(const upb_msgdef* msgdef); void layout_init(MessageLayout* layout, void* storage, zend_object* object PHP_PROTO_TSRMLS_DC); -zval* layout_get(MessageLayout* layout, const void* storage, +zval* layout_get(MessageLayout* layout, MessageHeader* header, const upb_fielddef* field, CACHED_VALUE* cache TSRMLS_DC); void layout_set(MessageLayout* layout, MessageHeader* header, const upb_fielddef* field, zval* val TSRMLS_DC); @@ -1089,6 +1090,8 @@ upb_value map_iter_value(MapIter* iter, int* len); const upb_fielddef* map_entry_key(const upb_msgdef* msgdef); const upb_fielddef* map_entry_value(const upb_msgdef* msgdef); +void map_field_ensure_created(const upb_fielddef *field, + CACHED_VALUE *map_field PHP_PROTO_TSRMLS_DC); void map_field_create_with_field(const zend_class_entry* ce, const upb_fielddef* field, CACHED_VALUE* map_field PHP_PROTO_TSRMLS_DC); @@ -1147,6 +1150,9 @@ PHP_PROTO_WRAP_OBJECT_START(RepeatedFieldIter) long position; PHP_PROTO_WRAP_OBJECT_END +void repeated_field_ensure_created( + const upb_fielddef *field, + CACHED_VALUE *repeated_field PHP_PROTO_TSRMLS_DC); void repeated_field_create_with_field( zend_class_entry* ce, const upb_fielddef* field, CACHED_VALUE* repeated_field PHP_PROTO_TSRMLS_DC); @@ -1489,6 +1495,9 @@ size_t stringsink_string(void *_sink, const void *hd, const char *ptr, #define FREE(object) efree(object) #define PEFREE(object) pefree(object, 1) +// Find corresponding zval property for the field. +CACHED_VALUE* find_zval_property(MessageHeader* msg, const upb_fielddef* field); + // String argument. #define STR(str) (str), strlen(str) diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index 1c28b1c185..e6050d0eb3 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -75,11 +75,9 @@ static bool native_slot_is_default(upb_fieldtype_t type, const void* memory) { #undef CASE_TYPE case UPB_TYPE_STRING: case UPB_TYPE_BYTES: - return Z_STRLEN_P(CACHED_PTR_TO_ZVAL_PTR(DEREF(memory, CACHED_VALUE*))) == - 0; + return Z_STRLEN_P(CACHED_PTR_TO_ZVAL_PTR(memory)) == 0; case UPB_TYPE_MESSAGE: - return Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(DEREF(memory, CACHED_VALUE*))) == - IS_NULL; + return Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(memory)) == IS_NULL; default: return false; } } @@ -599,6 +597,8 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { // Reserve space for unknown fields. off += sizeof(void*); + layout->empty_template = NULL; + TSRMLS_FETCH(); Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msgdef)); layout->fields = ALLOC_N(MessageField, nfields); @@ -744,64 +744,35 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { layout->size = off; layout->msgdef = msgdef; + // Create the empty message template. + layout->empty_template = ALLOC_N(char, layout->size); + memset(layout->empty_template, 0, layout->size); + return layout; } void free_layout(MessageLayout* layout) { + FREE(layout->empty_template); FREE(layout->fields); FREE(layout); } void layout_init(MessageLayout* layout, void* storage, zend_object* object PHP_PROTO_TSRMLS_DC) { - int i; - upb_msg_field_iter it; - - // Init unknown fields - memset(storage, 0, sizeof(void*)); - - for (upb_msg_field_begin(&it, layout->msgdef), i = 0; !upb_msg_field_done(&it); - upb_msg_field_next(&it), i++) { - const upb_fielddef* field = upb_msg_iter_field(&it); - void* memory = slot_memory(layout, storage, field); - uint32_t* oneof_case = slot_oneof_case(layout, storage, field); - int cache_index = slot_property_cache(layout, storage, field); - CACHED_VALUE* property_ptr = OBJ_PROP(object, cache_index); - - if (upb_fielddef_containingoneof(field)) { - memset(memory, 0, NATIVE_SLOT_MAX_SIZE); - *oneof_case = ONEOF_CASE_NONE; - } else if (is_map_field(field)) { - zval_ptr_dtor(property_ptr); -#if PHP_MAJOR_VERSION < 7 - MAKE_STD_ZVAL(*property_ptr); -#endif - map_field_create_with_field(map_field_type, field, - property_ptr PHP_PROTO_TSRMLS_CC); - DEREF(memory, CACHED_VALUE*) = property_ptr; - } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { - zval_ptr_dtor(property_ptr); -#if PHP_MAJOR_VERSION < 7 - MAKE_STD_ZVAL(*property_ptr); -#endif - repeated_field_create_with_field(repeated_field_type, field, - property_ptr PHP_PROTO_TSRMLS_CC); - DEREF(memory, CACHED_VALUE*) = property_ptr; - } else { - native_slot_init(upb_fielddef_type(field), memory, property_ptr); - } - } + memcpy(storage, layout->empty_template, layout->size); } -// For non-singular fields, the related memory needs to point to the actual -// zval in properties table first. -static void* value_memory(const upb_fielddef* field, void* memory) { - switch (upb_fielddef_type(field)) { +// Switch memory for processing for singular fields based on field type. +// * primitive fields: memory +// * others (string, bytes and message): cache (the correspond zval +// property) +static void* value_memory( + upb_fieldtype_t type, void* memory, CACHED_VALUE* cache) { + switch (type) { case UPB_TYPE_STRING: case UPB_TYPE_BYTES: case UPB_TYPE_MESSAGE: - memory = DEREF(memory, CACHED_VALUE*); - break; + return cache; default: // No operation break; @@ -809,8 +780,17 @@ static void* value_memory(const upb_fielddef* field, void* memory) { return memory; } -zval* layout_get(MessageLayout* layout, const void* storage, +CACHED_VALUE* find_zval_property( + MessageHeader* header, const upb_fielddef* field) { + int property_cache_index = + header->descriptor->layout->fields[upb_fielddef_index(field)] + .cache_index; + return OBJ_PROP(&header->std, property_cache_index); +} + +zval* layout_get(MessageLayout* layout, MessageHeader* header, const upb_fielddef* field, CACHED_VALUE* cache TSRMLS_DC) { + const void* storage = message_data(header); void* memory = slot_memory(layout, storage, field); uint32_t* oneof_case = slot_oneof_case(layout, storage, field); @@ -818,14 +798,21 @@ zval* layout_get(MessageLayout* layout, const void* storage, if (*oneof_case != upb_fielddef_number(field)) { native_slot_get_default(upb_fielddef_type(field), cache TSRMLS_CC); } else { - native_slot_get(upb_fielddef_type(field), value_memory(field, memory), - cache TSRMLS_CC); + upb_fieldtype_t type = upb_fielddef_type(field); + CACHED_VALUE* stored_cache = find_zval_property(header, field); + native_slot_get( + type, value_memory(type, memory, stored_cache), cache TSRMLS_CC); } return CACHED_PTR_TO_ZVAL_PTR(cache); + } else if (is_map_field(field)) { + map_field_ensure_created(field, cache PHP_PROTO_TSRMLS_CC); + return CACHED_PTR_TO_ZVAL_PTR(cache); } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { + repeated_field_ensure_created(field, cache PHP_PROTO_TSRMLS_CC); return CACHED_PTR_TO_ZVAL_PTR(cache); } else { - native_slot_get(upb_fielddef_type(field), value_memory(field, memory), + upb_fieldtype_t type = upb_fielddef_type(field); + native_slot_get(type, value_memory(type, memory, cache), cache TSRMLS_CC); return CACHED_PTR_TO_ZVAL_PTR(cache); } @@ -868,8 +855,8 @@ void layout_set(MessageLayout* layout, MessageHeader* header, *oneof_case = upb_fielddef_number(field); } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { // Works for both repeated and map fields - memory = DEREF(memory, void**); - zval* property_ptr = CACHED_PTR_TO_ZVAL_PTR((CACHED_VALUE*)memory); + CACHED_VALUE* cached = find_zval_property(header, field); + zval* property_ptr = CACHED_PTR_TO_ZVAL_PTR(cached); if (EXPECTED(property_ptr != val)) { zend_class_entry *subce = NULL; @@ -901,7 +888,7 @@ void layout_set(MessageLayout* layout, MessageHeader* header, &converted_value); } #if PHP_MAJOR_VERSION < 7 - REPLACE_ZVAL_VALUE((zval**)memory, &converted_value, 1); + REPLACE_ZVAL_VALUE((zval**)cached, &converted_value, 1); #else php_proto_zval_ptr_dtor(property_ptr); ZVAL_ZVAL(property_ptr, &converted_value, 1, 0); @@ -916,12 +903,16 @@ void layout_set(MessageLayout* layout, MessageHeader* header, Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msg)); ce = desc->klass; } - native_slot_set(type, ce, value_memory(field, memory), val TSRMLS_CC); + CACHED_VALUE* cache = find_zval_property(header, field); + native_slot_set( + type, ce, value_memory(upb_fielddef_type(field), memory, cache), + val TSRMLS_CC); } } -static void native_slot_merge(const upb_fielddef* field, const void* from_memory, - void* to_memory PHP_PROTO_TSRMLS_DC) { +static void native_slot_merge( + const upb_fielddef* field, const void* from_memory, + void* to_memory PHP_PROTO_TSRMLS_DC) { upb_fieldtype_t type = upb_fielddef_type(field); zend_class_entry* ce = NULL; if (!native_slot_is_default(type, from_memory)) { @@ -943,9 +934,8 @@ static void native_slot_merge(const upb_fielddef* field, const void* from_memory #undef CASE_TYPE case UPB_TYPE_STRING: case UPB_TYPE_BYTES: - native_slot_set(type, NULL, value_memory(field, to_memory), - CACHED_PTR_TO_ZVAL_PTR(DEREF( - from_memory, CACHED_VALUE*)) PHP_PROTO_TSRMLS_CC); + native_slot_set(type, NULL, to_memory, + CACHED_PTR_TO_ZVAL_PTR(from_memory) PHP_PROTO_TSRMLS_CC); break; case UPB_TYPE_MESSAGE: { const upb_msgdef* msg = upb_fielddef_msgsubdef(field); @@ -953,22 +943,21 @@ static void native_slot_merge(const upb_fielddef* field, const void* from_memory ce = desc->klass; if (native_slot_is_default(type, to_memory)) { #if PHP_MAJOR_VERSION < 7 - SEPARATE_ZVAL_IF_NOT_REF((zval**)value_memory(field, to_memory)); + SEPARATE_ZVAL_IF_NOT_REF((zval**)to_memory); #endif CREATE_OBJ_ON_ALLOCATED_ZVAL_PTR( - CACHED_PTR_TO_ZVAL_PTR(DEREF(to_memory, CACHED_VALUE*)), ce); + CACHED_PTR_TO_ZVAL_PTR(to_memory), ce); MessageHeader* submsg = - UNBOX(MessageHeader, - CACHED_PTR_TO_ZVAL_PTR(DEREF(to_memory, CACHED_VALUE*))); + UNBOX(MessageHeader, CACHED_PTR_TO_ZVAL_PTR(to_memory)); custom_data_init(ce, submsg PHP_PROTO_TSRMLS_CC); } MessageHeader* sub_from = UNBOX(MessageHeader, - CACHED_PTR_TO_ZVAL_PTR(DEREF(from_memory, CACHED_VALUE*))); + CACHED_PTR_TO_ZVAL_PTR(from_memory)); MessageHeader* sub_to = UNBOX(MessageHeader, - CACHED_PTR_TO_ZVAL_PTR(DEREF(to_memory, CACHED_VALUE*))); + CACHED_PTR_TO_ZVAL_PTR(to_memory)); layout_merge(desc->layout, sub_from, sub_to PHP_PROTO_TSRMLS_CC); break; @@ -1069,10 +1058,17 @@ void layout_merge(MessageLayout* layout, MessageHeader* from, int size, key_length, value_length; MapIter map_it; - zval* to_map_php = - CACHED_PTR_TO_ZVAL_PTR(DEREF(to_memory, CACHED_VALUE*)); - zval* from_map_php = - CACHED_PTR_TO_ZVAL_PTR(DEREF(from_memory, CACHED_VALUE*)); + CACHED_VALUE* from_cache = find_zval_property(from, field); + CACHED_VALUE* to_cache = find_zval_property(to, field); + + if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(from_cache)) == IS_NULL) { + continue; + } + map_field_ensure_created(field, to_cache PHP_PROTO_TSRMLS_CC); + + zval* to_map_php = CACHED_PTR_TO_ZVAL_PTR(to_cache); + zval* from_map_php = CACHED_PTR_TO_ZVAL_PTR(from_cache); + Map* to_map = UNBOX(Map, to_map_php); Map* from_map = UNBOX(Map, from_map_php); @@ -1098,8 +1094,16 @@ void layout_merge(MessageLayout* layout, MessageHeader* from, } } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { - zval* to_array_php = CACHED_PTR_TO_ZVAL_PTR(DEREF(to_memory, CACHED_VALUE*)); - zval* from_array_php = CACHED_PTR_TO_ZVAL_PTR(DEREF(from_memory, CACHED_VALUE*)); + CACHED_VALUE* from_cache = find_zval_property(from, field); + CACHED_VALUE* to_cache = find_zval_property(to, field); + + if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(from_cache)) == IS_NULL) { + continue; + } + repeated_field_ensure_created(field, to_cache PHP_PROTO_TSRMLS_CC); + + zval* to_array_php = CACHED_PTR_TO_ZVAL_PTR(to_cache); + zval* from_array_php = CACHED_PTR_TO_ZVAL_PTR(from_cache); RepeatedField* to_array = UNBOX(RepeatedField, to_array_php); RepeatedField* from_array = UNBOX(RepeatedField, from_array_php); @@ -1129,7 +1133,19 @@ void layout_merge(MessageLayout* layout, MessageHeader* from, } } } else { - native_slot_merge(field, from_memory, to_memory PHP_PROTO_TSRMLS_CC); + switch (upb_fielddef_type(field)) { + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: + case UPB_TYPE_MESSAGE: { + CACHED_VALUE* from_cache = find_zval_property(from, field); + CACHED_VALUE* to_cache = find_zval_property(to, field); + native_slot_merge(field, from_cache, to_cache PHP_PROTO_TSRMLS_CC); + break; + } + default: + native_slot_merge(field, from_memory, to_memory PHP_PROTO_TSRMLS_CC); + break; + } } } } From 403df1d047607b5194d938d3c9943a4813308eb3 Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Tue, 29 Oct 2019 14:05:40 -0700 Subject: [PATCH 27/27] Added a descriptor option number for ocaml-protoc-plugin --- docs/options.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/options.md b/docs/options.md index 7149b21f8c..22a8bc979e 100644 --- a/docs/options.md +++ b/docs/options.md @@ -212,3 +212,7 @@ with info about your project (name and website) so we can add an entry for you. 1. Dart port of protocol buffers * Website https://github.com/dart-lang/protobuf * Extensions: 1073 + +1. Ocaml-protoc-plugin + * Website: https://github.com/issuu/ocaml-protoc-plugin + * Extensions: 1074