diff --git a/Makefile.am b/Makefile.am index 9b0e68bc7..91a029b46 100644 --- a/Makefile.am +++ b/Makefile.am @@ -978,6 +978,7 @@ python_EXTRA_DIST= \ python/google/protobuf/internal/service_reflection_test.py \ python/google/protobuf/internal/symbol_database_test.py \ python/google/protobuf/internal/test_bad_identifiers.proto \ + python/google/protobuf/internal/test_proto3_optional.proto \ python/google/protobuf/internal/test_util.py \ python/google/protobuf/internal/testing_refleaks.py \ python/google/protobuf/internal/text_encoding_test.py \ diff --git a/python/google/protobuf/internal/json_format_test.py b/python/google/protobuf/internal/json_format_test.py index ea2bca944..1510228bf 100755 --- a/python/google/protobuf/internal/json_format_test.py +++ b/python/google/protobuf/internal/json_format_test.py @@ -53,6 +53,7 @@ from google.protobuf import wrappers_pb2 from google.protobuf import any_test_pb2 from google.protobuf import unittest_mset_pb2 from google.protobuf import unittest_pb2 +from google.protobuf.internal import test_proto3_optional_pb2 from google.protobuf import descriptor_pool from google.protobuf import json_format from google.protobuf.util import json_format_pb2 @@ -339,6 +340,20 @@ class JsonFormatTest(JsonFormatBase): parsed_message = json_format_proto3_pb2.TestMessage() self.CheckParseBack(message, parsed_message) + def testProto3Optional(self): + message = test_proto3_optional_pb2.TestProto3Optional() + self.assertEqual( + json.loads( + json_format.MessageToJson( + message, including_default_value_fields=True)), + json.loads('{}')) + message.optional_int32 = 0 + self.assertEqual( + json.loads( + json_format.MessageToJson( + message, including_default_value_fields=True)), + json.loads('{"optionalInt32": 0}')) + def testIntegersRepresentedAsFloat(self): message = json_format_proto3_pb2.TestMessage() json_format.Parse('{"int32Value": -2.147483648e9}', message) diff --git a/python/google/protobuf/internal/message_test.py b/python/google/protobuf/internal/message_test.py index 097c082c8..0508bb211 100755 --- a/python/google/protobuf/internal/message_test.py +++ b/python/google/protobuf/internal/message_test.py @@ -83,6 +83,7 @@ from google.protobuf.internal import encoder from google.protobuf.internal import more_extensions_pb2 from google.protobuf.internal import packed_field_test_pb2 from google.protobuf.internal import test_util +from google.protobuf.internal import test_proto3_optional_pb2 from google.protobuf.internal import testing_refleaks from google.protobuf import message from google.protobuf.internal import _parameterized @@ -1674,6 +1675,59 @@ class Proto3Test(unittest.TestCase): self.assertEqual(False, message.optional_bool) self.assertEqual(0, message.optional_nested_message.bb) + def testProto3Optional(self): + msg = test_proto3_optional_pb2.TestProto3Optional() + self.assertFalse(msg.HasField('optional_int32')) + self.assertFalse(msg.HasField('optional_float')) + self.assertFalse(msg.HasField('optional_string')) + self.assertFalse(msg.HasField('optional_nested_message')) + self.assertFalse(msg.optional_nested_message.HasField('bb')) + + # Set fields. + msg.optional_int32 = 1 + msg.optional_float = 1.0 + msg.optional_string = '123' + msg.optional_nested_message.bb = 1 + self.assertTrue(msg.HasField('optional_int32')) + self.assertTrue(msg.HasField('optional_float')) + self.assertTrue(msg.HasField('optional_string')) + self.assertTrue(msg.HasField('optional_nested_message')) + self.assertTrue(msg.optional_nested_message.HasField('bb')) + # Set to default value does not clear the fields + msg.optional_int32 = 0 + msg.optional_float = 0.0 + msg.optional_string = '' + msg.optional_nested_message.bb = 0 + self.assertTrue(msg.HasField('optional_int32')) + self.assertTrue(msg.HasField('optional_float')) + self.assertTrue(msg.HasField('optional_string')) + self.assertTrue(msg.HasField('optional_nested_message')) + self.assertTrue(msg.optional_nested_message.HasField('bb')) + + # Test serialize + msg2 = test_proto3_optional_pb2.TestProto3Optional() + msg2.ParseFromString(msg.SerializeToString()) + self.assertTrue(msg2.HasField('optional_int32')) + self.assertTrue(msg2.HasField('optional_float')) + self.assertTrue(msg2.HasField('optional_string')) + self.assertTrue(msg2.HasField('optional_nested_message')) + self.assertTrue(msg2.optional_nested_message.HasField('bb')) + + self.assertEqual(msg.WhichOneof('_optional_int32'), 'optional_int32') + + # Clear these fields. + msg.ClearField('optional_int32') + msg.ClearField('optional_float') + msg.ClearField('optional_string') + msg.ClearField('optional_nested_message') + self.assertFalse(msg.HasField('optional_int32')) + self.assertFalse(msg.HasField('optional_float')) + self.assertFalse(msg.HasField('optional_string')) + self.assertFalse(msg.HasField('optional_nested_message')) + self.assertFalse(msg.optional_nested_message.HasField('bb')) + + self.assertEqual(msg.WhichOneof('_optional_int32'), None) + def testAssignUnknownEnum(self): """Assigning an unknown enum value is allowed and preserves the value.""" m = unittest_proto3_arena_pb2.TestAllTypes() diff --git a/python/google/protobuf/internal/python_message.py b/python/google/protobuf/internal/python_message.py index 0691a4c94..beea89472 100644 --- a/python/google/protobuf/internal/python_message.py +++ b/python/google/protobuf/internal/python_message.py @@ -826,7 +826,8 @@ def _AddListFieldsMethod(message_descriptor, cls): cls.ListFields = ListFields _PROTO3_ERROR_TEMPLATE = \ - 'Protocol message %s has no non-repeated submessage field "%s"' + ('Protocol message %s has no non-repeated submessage field "%s" ' + 'nor marked as optional') _PROTO2_ERROR_TEMPLATE = 'Protocol message %s has no non-repeated field "%s"' def _AddHasFieldMethod(message_descriptor, cls): diff --git a/python/google/protobuf/internal/test_proto3_optional.proto b/python/google/protobuf/internal/test_proto3_optional.proto new file mode 100644 index 000000000..a5abd19d1 --- /dev/null +++ b/python/google/protobuf/internal/test_proto3_optional.proto @@ -0,0 +1,73 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +syntax = "proto3"; + +package protobuf_unittest; + +option java_multiple_files = true; +option java_package = "com.google.protobuf.testing.proto"; + +message TestProto3Optional { + message NestedMessage { + // The field name "b" fails to compile in proto1 because it conflicts with + // a local variable named "b" in one of the generated methods. Doh. + // This file needs to compile in proto1 to test backwards-compatibility. + optional int32 bb = 1; + } + + enum NestedEnum { + UNSPECIFIED = 0; + FOO = 1; + BAR = 2; + BAZ = 3; + NEG = -1; // Intentionally negative. + } + + // Singular + optional int32 optional_int32 = 1; + optional int64 optional_int64 = 2; + optional uint32 optional_uint32 = 3; + optional uint64 optional_uint64 = 4; + optional sint32 optional_sint32 = 5; + optional sint64 optional_sint64 = 6; + optional fixed32 optional_fixed32 = 7; + optional fixed64 optional_fixed64 = 8; + optional sfixed32 optional_sfixed32 = 9; + optional sfixed64 optional_sfixed64 = 10; + optional float optional_float = 11; + optional double optional_double = 12; + optional bool optional_bool = 13; + optional string optional_string = 14; + optional bytes optional_bytes = 15; + + optional NestedMessage optional_nested_message = 18; + optional NestedEnum optional_nested_enum = 21; +} diff --git a/python/google/protobuf/internal/text_format_test.py b/python/google/protobuf/internal/text_format_test.py index b7d2333b3..cdf035b28 100755 --- a/python/google/protobuf/internal/text_format_test.py +++ b/python/google/protobuf/internal/text_format_test.py @@ -125,6 +125,112 @@ class TextFormatMessageToStringTests(TextFormatBase): ' "\\000\\001\\007\\010\\014\\n\\r\\t\\013\\\\\\\'\\""\n' 'repeated_string: "\\303\\274\\352\\234\\237"\n') + def testPrintFloatPrecision(self, message_module): + message = message_module.TestAllTypes() + + message.repeated_float.append(0.0) + message.repeated_float.append(0.8) + message.repeated_float.append(1.0) + message.repeated_float.append(1.2) + message.repeated_float.append(1.23) + message.repeated_float.append(1.234) + message.repeated_float.append(1.2345) + message.repeated_float.append(1.23456) + message.repeated_float.append(1.2e10) + message.repeated_float.append(1.23e10) + message.repeated_float.append(1.234e10) + message.repeated_float.append(1.2345e10) + message.repeated_float.append(1.23456e10) + message.repeated_double.append(0.0) + message.repeated_double.append(0.8) + message.repeated_double.append(1.0) + message.repeated_double.append(1.2) + message.repeated_double.append(1.23) + message.repeated_double.append(1.234) + message.repeated_double.append(1.2345) + message.repeated_double.append(1.23456) + message.repeated_double.append(1.234567) + message.repeated_double.append(1.2345678) + message.repeated_double.append(1.23456789) + message.repeated_double.append(1.234567898) + message.repeated_double.append(1.2345678987) + message.repeated_double.append(1.23456789876) + message.repeated_double.append(1.234567898765) + message.repeated_double.append(1.2345678987654) + message.repeated_double.append(1.23456789876543) + message.repeated_double.append(1.2e100) + message.repeated_double.append(1.23e100) + message.repeated_double.append(1.234e100) + message.repeated_double.append(1.2345e100) + message.repeated_double.append(1.23456e100) + message.repeated_double.append(1.234567e100) + message.repeated_double.append(1.2345678e100) + message.repeated_double.append(1.23456789e100) + message.repeated_double.append(1.234567898e100) + message.repeated_double.append(1.2345678987e100) + message.repeated_double.append(1.23456789876e100) + message.repeated_double.append(1.234567898765e100) + message.repeated_double.append(1.2345678987654e100) + message.repeated_double.append(1.23456789876543e100) + # pylint: disable=g-long-ternary + self.CompareToGoldenText( + self.RemoveRedundantZeros(text_format.MessageToString(message)), + 'repeated_float: 0\n' + # This should be 0.8 + 'repeated_float: 0.80000001\n' + 'repeated_float: 1\n' + 'repeated_float: 1.2\n' + 'repeated_float: 1.23\n' + 'repeated_float: 1.234\n' + # This should be 1.2345 + 'repeated_float: 1.2345001\n' + 'repeated_float: 1.23456\n' + # Note that these don't use scientific notation. + 'repeated_float: 12000000000\n' + 'repeated_float: 12300000000\n' + 'repeated_float: 12340000000\n' + 'repeated_float: 12345000000\n' + 'repeated_float: 12345600000\n' + 'repeated_double: 0\n' + 'repeated_double: 0.8\n' + 'repeated_double: 1\n' + 'repeated_double: 1.2\n' + 'repeated_double: 1.23\n' + 'repeated_double: 1.234\n' + 'repeated_double: 1.2345\n' + 'repeated_double: 1.23456\n' + 'repeated_double: 1.234567\n' + 'repeated_double: 1.2345678\n' + 'repeated_double: 1.23456789\n' + 'repeated_double: 1.234567898\n' + 'repeated_double: 1.2345678987\n' + 'repeated_double: 1.23456789876\n' + + ('repeated_double: 1.23456789876\n' + 'repeated_double: 1.23456789877\n' + 'repeated_double: 1.23456789877\n' + if six.PY2 else + 'repeated_double: 1.234567898765\n' + 'repeated_double: 1.2345678987654\n' + 'repeated_double: 1.23456789876543\n') + + 'repeated_double: 1.2e+100\n' + 'repeated_double: 1.23e+100\n' + 'repeated_double: 1.234e+100\n' + 'repeated_double: 1.2345e+100\n' + 'repeated_double: 1.23456e+100\n' + 'repeated_double: 1.234567e+100\n' + 'repeated_double: 1.2345678e+100\n' + 'repeated_double: 1.23456789e+100\n' + 'repeated_double: 1.234567898e+100\n' + 'repeated_double: 1.2345678987e+100\n' + 'repeated_double: 1.23456789876e+100\n' + + ('repeated_double: 1.23456789877e+100\n' + 'repeated_double: 1.23456789877e+100\n' + 'repeated_double: 1.23456789877e+100\n' + if six.PY2 else + 'repeated_double: 1.234567898765e+100\n' + 'repeated_double: 1.2345678987654e+100\n' + 'repeated_double: 1.23456789876543e+100\n')) + def testPrintExoticUnicodeSubclass(self, message_module): class UnicodeSub(six.text_type): diff --git a/python/google/protobuf/pyext/message.cc b/python/google/protobuf/pyext/message.cc index e4d6c7cca..e39764d0e 100644 --- a/python/google/protobuf/pyext/message.cc +++ b/python/google/protobuf/pyext/message.cc @@ -1424,19 +1424,13 @@ bool CheckHasPresence(const FieldDescriptor* field_descriptor, bool in_oneof) { return false; } - if (field_descriptor->file()->syntax() == FileDescriptor::SYNTAX_PROTO3) { - // HasField() is supported for oneof fields. - if (field_descriptor->containing_oneof() != NULL) { - return true; - } - - if (field_descriptor->cpp_type() != FieldDescriptor::CPPTYPE_MESSAGE) { - PyErr_Format( - PyExc_ValueError, - "Can't test non-submessage field \"%s.%s\" for presence in proto3.", - message_name.c_str(), field_descriptor->name().c_str()); - return false; - } + if (field_descriptor->containing_oneof() == NULL && + !field_descriptor->is_singular_with_presence()) { + PyErr_Format(PyExc_ValueError, + "Can't test non-optional, non-submessage field \"%s.%s\" for " + "presence in proto3.", + message_name.c_str(), field_descriptor->name().c_str()); + return false; } return true; diff --git a/python/setup.py b/python/setup.py index 9aabbf7aa..7e462b181 100755 --- a/python/setup.py +++ b/python/setup.py @@ -110,6 +110,7 @@ def GenerateUnittestProtos(): generate_proto("google/protobuf/internal/no_package.proto", False) generate_proto("google/protobuf/internal/packed_field_test.proto", False) generate_proto("google/protobuf/internal/test_bad_identifiers.proto", False) + generate_proto("google/protobuf/internal/test_proto3_optional.proto", False) generate_proto("google/protobuf/pyext/python.proto", False) diff --git a/src/google/protobuf/any.pb.h b/src/google/protobuf/any.pb.h index 1b7dca0bd..187f7d81b 100644 --- a/src/google/protobuf/any.pb.h +++ b/src/google/protobuf/any.pb.h @@ -338,7 +338,6 @@ inline std::string* Any::_internal_mutable_type_url() { } inline std::string* Any::release_type_url() { // @@protoc_insertion_point(field_release:google.protobuf.Any.type_url) - return type_url_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Any::set_allocated_type_url(std::string* type_url) { @@ -420,7 +419,6 @@ inline std::string* Any::_internal_mutable_value() { } inline std::string* Any::release_value() { // @@protoc_insertion_point(field_release:google.protobuf.Any.value) - return value_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Any::set_allocated_value(std::string* value) { diff --git a/src/google/protobuf/api.pb.h b/src/google/protobuf/api.pb.h index 0d0ce4ab7..66ac61af8 100644 --- a/src/google/protobuf/api.pb.h +++ b/src/google/protobuf/api.pb.h @@ -845,7 +845,6 @@ inline std::string* Api::_internal_mutable_name() { } inline std::string* Api::release_name() { // @@protoc_insertion_point(field_release:google.protobuf.Api.name) - return name_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Api::set_allocated_name(std::string* name) { @@ -1002,7 +1001,6 @@ inline std::string* Api::_internal_mutable_version() { } inline std::string* Api::release_version() { // @@protoc_insertion_point(field_release:google.protobuf.Api.version) - return version_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Api::set_allocated_version(std::string* version) { @@ -1222,7 +1220,6 @@ inline std::string* Method::_internal_mutable_name() { } inline std::string* Method::release_name() { // @@protoc_insertion_point(field_release:google.protobuf.Method.name) - return name_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Method::set_allocated_name(std::string* name) { @@ -1304,7 +1301,6 @@ inline std::string* Method::_internal_mutable_request_type_url() { } inline std::string* Method::release_request_type_url() { // @@protoc_insertion_point(field_release:google.protobuf.Method.request_type_url) - return request_type_url_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Method::set_allocated_request_type_url(std::string* request_type_url) { @@ -1406,7 +1402,6 @@ inline std::string* Method::_internal_mutable_response_type_url() { } inline std::string* Method::release_response_type_url() { // @@protoc_insertion_point(field_release:google.protobuf.Method.response_type_url) - return response_type_url_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Method::set_allocated_response_type_url(std::string* response_type_url) { @@ -1568,7 +1563,6 @@ inline std::string* Mixin::_internal_mutable_name() { } inline std::string* Mixin::release_name() { // @@protoc_insertion_point(field_release:google.protobuf.Mixin.name) - return name_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Mixin::set_allocated_name(std::string* name) { @@ -1650,7 +1644,6 @@ inline std::string* Mixin::_internal_mutable_root() { } inline std::string* Mixin::release_root() { // @@protoc_insertion_point(field_release:google.protobuf.Mixin.root) - return root_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Mixin::set_allocated_root(std::string* root) { diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index ab7a7d4bb..ed64289bd 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -1105,7 +1105,12 @@ PopulateSingleSimpleDescriptorDatabase(const std::string& descriptor_set_name) { bool CommandLineInterface::AllowProto3Optional( const FileDescriptor& file) const { if (allow_proto3_optional_) return true; - if (file.name() == "google/protobuf/unittest_proto3_optional.proto") { + // Whitelist all ads protos. Ads is an early adopter of this feature. + if (file.name().find("google/ads/googleads") != std::string::npos) { + return true; + } + if (file.name() == "google/protobuf/unittest_proto3_optional.proto" || + file.name() == "google/protobuf/internal/test_proto3_optional.proto") { return true; } return false; diff --git a/src/google/protobuf/compiler/cpp/cpp_field.cc b/src/google/protobuf/compiler/cpp/cpp_field.cc index 5b2ca151d..6a73ab804 100644 --- a/src/google/protobuf/compiler/cpp/cpp_field.cc +++ b/src/google/protobuf/compiler/cpp/cpp_field.cc @@ -73,7 +73,7 @@ void SetCommonFieldVariables(const FieldDescriptor* descriptor, (*variables)["set_hasbit"] = ""; (*variables)["clear_hasbit"] = ""; - if (HasFieldPresence(descriptor->file())) { + if (HasHasbit(descriptor)) { (*variables)["set_hasbit_io"] = "_Internal::set_has_" + FieldName(descriptor) + "(&_has_bits_);"; } else { @@ -90,7 +90,8 @@ void SetCommonFieldVariables(const FieldDescriptor* descriptor, } void FieldGenerator::SetHasBitIndex(int32 has_bit_index) { - if (!HasFieldPresence(descriptor_->file()) || has_bit_index == -1) { + if (!HasHasbit(descriptor_)) { + GOOGLE_CHECK_EQ(has_bit_index, -1); return; } variables_["set_hasbit"] = StrCat( @@ -155,7 +156,7 @@ FieldGenerator* FieldGeneratorMap::MakeGenerator( default: return new RepeatedPrimitiveFieldGenerator(field, options); } - } else if (field->containing_oneof()) { + } else if (InRealOneof(field)) { switch (field->cpp_type()) { case FieldDescriptor::CPPTYPE_MESSAGE: return new MessageOneofFieldGenerator(field, options, scc_analyzer); diff --git a/src/google/protobuf/compiler/cpp/cpp_helpers.cc b/src/google/protobuf/compiler/cpp/cpp_helpers.cc index c3cd80829..7aeffb0f0 100644 --- a/src/google/protobuf/compiler/cpp/cpp_helpers.cc +++ b/src/google/protobuf/compiler/cpp/cpp_helpers.cc @@ -795,10 +795,10 @@ bool IsStringInlined(const FieldDescriptor* descriptor, if (IsAnyMessage(descriptor->containing_type(), options)) return false; if (descriptor->containing_type()->options().map_entry()) return false; - // Limit to proto2, as we rely on has bits to distinguish field presence for - // release_$name$. On proto3, we cannot use the address of the string - // instance when the field has been inlined. - if (!HasFieldPresence(descriptor->file())) return false; + // We rely on has bits to distinguish field presence for release_$name$. When + // there is no hasbit, we cannot use the address of the string instance when + // the field has been inlined. + if (!HasHasbit(descriptor)) return false; if (options.access_info_map) { if (descriptor->is_required()) return true; @@ -1151,7 +1151,7 @@ bool IsImplicitWeakField(const FieldDescriptor* field, const Options& options, return UsingImplicitWeakFields(field->file(), options) && field->type() == FieldDescriptor::TYPE_MESSAGE && !field->is_required() && !field->is_map() && !field->is_extension() && - field->containing_oneof() == nullptr && + !InRealOneof(field) && !IsWellKnownMessage(field->message_type()->file()) && field->message_type()->file()->name() != "net/proto2/proto/descriptor.proto" && @@ -1403,7 +1403,7 @@ class ParseLoopGenerator { "#define CHK_(x) if (PROTOBUF_PREDICT_FALSE(!(x))) goto failure\n"); format_.Indent(); int hasbits_size = 0; - if (HasFieldPresence(descriptor->file())) { + if (num_hasbits_ > 0) { hasbits_size = (num_hasbits_ + 31) / 32; } // For now only optimize small hasbits. @@ -1442,7 +1442,7 @@ class ParseLoopGenerator { using WireFormatLite = internal::WireFormatLite; void GenerateArenaString(const FieldDescriptor* field) { - if (HasFieldPresence(field->file())) { + if (HasHasbit(field)) { format_("_Internal::set_has_$1$(&$has_bits$);\n", FieldName(field)); } std::string default_string = @@ -1474,8 +1474,8 @@ class ParseLoopGenerator { GetOptimizeFor(field->file(), options_) != FileOptions::LITE_RUNTIME && // For now only use arena string for strings with empty defaults. field->default_value_string().empty() && - !IsStringInlined(field, options_) && - field->containing_oneof() == nullptr && ctype == FieldOptions::STRING) { + !IsStringInlined(field, options_) && !InRealOneof(field) && + ctype == FieldOptions::STRING) { GenerateArenaString(field); } else { std::string name; @@ -1565,8 +1565,8 @@ class ParseLoopGenerator { const FieldDescriptor* val = field->message_type()->FindFieldByName("value"); GOOGLE_CHECK(val); - if (HasFieldPresence(field->file()) && - val->type() == FieldDescriptor::TYPE_ENUM) { + if (val->type() == FieldDescriptor::TYPE_ENUM && + !HasPreservingUnknownEnumSemantics(field)) { format_( "auto object = " "::$proto_ns$::internal::InitEnumParseWrapper<$unknown_" @@ -1580,7 +1580,7 @@ class ParseLoopGenerator { FieldName(field)); } } else if (IsLazy(field, options_)) { - if (field->containing_oneof() != nullptr) { + if (InRealOneof(field)) { format_( "if (!_internal_has_$1$()) {\n" " clear_$2$();\n" @@ -1590,7 +1590,7 @@ class ParseLoopGenerator { "}\n" "ptr = ctx->ParseMessage($2$_.$1$_, ptr);\n", FieldName(field), field->containing_oneof()->name()); - } else if (HasFieldPresence(field->file())) { + } else if (HasHasbit(field)) { format_( "_Internal::set_has_$1$(&$has_bits$);\n" "ptr = ctx->ParseMessage(&$1$_, ptr);\n", @@ -1684,14 +1684,14 @@ class ParseLoopGenerator { field->type() == FieldDescriptor::TYPE_SINT64)) { zigzag = "ZigZag"; } - if (field->is_repeated() || field->containing_oneof()) { + if (field->is_repeated() || InRealOneof(field)) { std::string prefix = field->is_repeated() ? "add" : "set"; format_( "_internal_$1$_$2$($pi_ns$::ReadVarint$3$$4$(&ptr));\n" "CHK_(ptr);\n", prefix, FieldName(field), zigzag, size); } else { - if (HasFieldPresence(field->file())) { + if (HasHasbit(field)) { format_("_Internal::set_has_$1$(&$has_bits$);\n", FieldName(field)); } @@ -1706,14 +1706,14 @@ class ParseLoopGenerator { case WireFormatLite::WIRETYPE_FIXED32: case WireFormatLite::WIRETYPE_FIXED64: { std::string type = PrimitiveTypeName(options_, field->cpp_type()); - if (field->is_repeated() || field->containing_oneof()) { + if (field->is_repeated() || InRealOneof(field)) { std::string prefix = field->is_repeated() ? "add" : "set"; format_( "_internal_$1$_$2$($pi_ns$::UnalignedLoad<$3$>(ptr));\n" "ptr += sizeof($3$);\n", prefix, FieldName(field), type); } else { - if (HasFieldPresence(field->file())) { + if (HasHasbit(field)) { format_("_Internal::set_has_$1$(&$has_bits$);\n", FieldName(field)); } format_( diff --git a/src/google/protobuf/compiler/cpp/cpp_helpers.h b/src/google/protobuf/compiler/cpp/cpp_helpers.h index 267578e38..ec57cb4a5 100644 --- a/src/google/protobuf/compiler/cpp/cpp_helpers.h +++ b/src/google/protobuf/compiler/cpp/cpp_helpers.h @@ -429,9 +429,6 @@ inline bool HasFieldPresence(const FileDescriptor* file) { } inline bool HasHasbit(const FieldDescriptor* field) { - // TODO(haberman): remove, so proto3 optional fields have hasbits. - if (field->file()->syntax() == FileDescriptor::SYNTAX_PROTO3) return false; - // This predicate includes proto3 message fields only if they have "optional". // Foo submsg1 = 1; // HasHasbit() == false // optional Foo submsg2 = 2; // HasHasbit() == true @@ -447,6 +444,23 @@ inline bool HasHasbit(const FieldDescriptor* field) { !field->options().weak(); } +inline bool InRealOneof(const FieldDescriptor* field) { + return field->containing_oneof() && + !field->containing_oneof()->is_synthetic(); +} + +// In practice all synthetic oneofs should be at the end of the list, but we +// decline to depend on this for correctness of the function. +inline int RealOneofCount(const Descriptor* descriptor) { + int count = 0; + for (int i = 0; i < descriptor->oneof_decl_count(); i++) { + if (!descriptor->oneof_decl(i)->is_synthetic()) { + count++; + } + } + return count; +} + // Returns true if 'enum' semantics are such that unknown values are preserved // in the enum field itself, rather than going to the UnknownFieldSet. inline bool HasPreservingUnknownEnumSemantics(const FieldDescriptor* field) { @@ -872,6 +886,14 @@ struct OneOfRangeImpl { using value_type = const OneofDescriptor*; using difference_type = int; + explicit Iterator(const Descriptor* _descriptor) + : idx(-1), descriptor(_descriptor) { + Next(); + } + + Iterator(int _idx, const Descriptor* _descriptor) + : idx(_idx), descriptor(_descriptor) {} + value_type operator*() { return descriptor->oneof_decl(idx); } friend bool operator==(const Iterator& a, const Iterator& b) { @@ -883,15 +905,22 @@ struct OneOfRangeImpl { } Iterator& operator++() { - idx++; + Next(); return *this; } + void Next() { + do { + idx++; + } while (idx < descriptor->oneof_decl_count() && + descriptor->oneof_decl(idx)->is_synthetic()); + } + int idx; const Descriptor* descriptor; }; - Iterator begin() const { return {0, descriptor}; } + Iterator begin() const { return Iterator(descriptor); } Iterator end() const { return {descriptor->oneof_decl_count(), descriptor}; } const Descriptor* descriptor; diff --git a/src/google/protobuf/compiler/cpp/cpp_message.cc b/src/google/protobuf/compiler/cpp/cpp_message.cc index c8c7d6bd7..33be14694 100644 --- a/src/google/protobuf/compiler/cpp/cpp_message.cc +++ b/src/google/protobuf/compiler/cpp/cpp_message.cc @@ -201,10 +201,11 @@ RunMap FindRuns(const std::vector& fields, // Emits an if-statement with a condition that evaluates to true if |field| is // considered non-default (will be sent over the wire), for message types // without true field presence. Should only be called if -// !HasFieldPresence(message_descriptor). +// !HasHasbit(field). bool EmitFieldNonDefaultCondition(io::Printer* printer, const std::string& prefix, const FieldDescriptor* field) { + GOOGLE_CHECK(!HasHasbit(field)); Formatter format(printer); format.Set("prefix", prefix); format.Set("name", FieldName(field)); @@ -225,7 +226,7 @@ bool EmitFieldNonDefaultCondition(io::Printer* printer, } format.Indent(); return true; - } else if (field->containing_oneof()) { + } else if (InRealOneof(field)) { format("if (_internal_has_$name$()) {\n"); format.Indent(); return true; @@ -281,8 +282,7 @@ void CollectMapInfo(const Options& options, const Descriptor* descriptor, bool HasPrivateHasMethod(const FieldDescriptor* field) { // Only for oneofs in message types with no field presence. has_$name$(), // based on the oneof case, is still useful internally for generated code. - return (!HasFieldPresence(field->file()) && - field->containing_oneof() != NULL); + return (!HasFieldPresence(field->file()) && InRealOneof(field)); } // TODO(ckennelly): Cull these exclusions if/when these protos do not have @@ -597,7 +597,7 @@ MessageGenerator::MessageGenerator( if (IsWeak(field, options_)) { num_weak_fields_++; - } else if (!field->containing_oneof()) { + } else if (!InRealOneof(field)) { optimized_order_.push_back(field); } } @@ -677,7 +677,7 @@ void MessageGenerator::AddGenerators( void MessageGenerator::GenerateFieldAccessorDeclarations(io::Printer* printer) { Formatter format(printer, variables_); // optimized_fields_ does not contain fields where - // field->containing_oneof() != NULL + // InRealOneof(field) == true // so we need to iterate over those as well. // // We place the non-oneof fields in optimized_order_, as that controls the @@ -689,7 +689,7 @@ void MessageGenerator::GenerateFieldAccessorDeclarations(io::Printer* printer) { ordered_fields.insert(ordered_fields.begin(), optimized_order_.begin(), optimized_order_.end()); for (auto field : FieldRange(descriptor_)) { - if (field->containing_oneof() == nullptr && !field->options().weak() && + if (!InRealOneof(field) && !field->options().weak() && IsFieldUsed(field, options_)) { continue; } @@ -922,7 +922,7 @@ void MessageGenerator::GenerateFieldClear(const FieldDescriptor* field, format.Indent(); - if (field->containing_oneof()) { + if (InRealOneof(field)) { // Clear this field only if it is the active field in this oneof, // otherwise ignore format("if (_internal_has_$name$()) {\n"); @@ -983,7 +983,7 @@ void MessageGenerator::GenerateFieldAccessorDefinitions(io::Printer* printer) { ? ".weak" : ""); } - } else if (field->containing_oneof()) { + } else if (InRealOneof(field)) { format.Set("field_name", UnderscoresToCamelCase(field->name(), true)); format.Set("oneof_name", field->containing_oneof()->name()); format.Set("oneof_index", @@ -1485,7 +1485,7 @@ void MessageGenerator::GenerateClassDefinition(io::Printer* printer) { for (auto field : FieldRange(descriptor_)) { // set_has_***() generated in all oneofs. if (!field->is_repeated() && !field->options().weak() && - field->containing_oneof()) { + InRealOneof(field)) { format("void set_has_$1$();\n", FieldName(field)); } } @@ -1594,11 +1594,12 @@ void MessageGenerator::GenerateClassDefinition(io::Printer* printer) { } // Generate _oneof_case_. - if (descriptor_->oneof_decl_count() > 0) { + int count = RealOneofCount(descriptor_); + if (count > 0) { format( "$uint32$ _oneof_case_[$1$];\n" "\n", - descriptor_->oneof_decl_count()); + count); } if (num_weak_fields_) { @@ -1642,24 +1643,22 @@ void MessageGenerator::GenerateExtraDefaultFields(io::Printer* printer) { // Generate oneof default instance and weak field instances for reflection // usage. Formatter format(printer, variables_); - if (descriptor_->oneof_decl_count() > 0 || num_weak_fields_ > 0) { - for (auto oneof : OneOfRange(descriptor_)) { - for (auto field : FieldRange(oneof)) { - if (!IsFieldUsed(field, options_)) { - continue; - } - if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE || - (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && - EffectiveStringCType(field, options_) != FieldOptions::STRING)) { - format("const "); - } - field_generators_.get(field).GeneratePrivateMembers(printer); + for (auto oneof : OneOfRange(descriptor_)) { + for (auto field : FieldRange(oneof)) { + if (!IsFieldUsed(field, options_)) { + continue; } + if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE || + (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && + EffectiveStringCType(field, options_) != FieldOptions::STRING)) { + format("const "); + } + field_generators_.get(field).GeneratePrivateMembers(printer); } - for (auto field : FieldRange(descriptor_)) { - if (field->options().weak() && IsFieldUsed(field, options_)) { - format(" const ::$proto_ns$::Message* $1$_;\n", FieldName(field)); - } + } + for (auto field : FieldRange(descriptor_)) { + if (field->options().weak() && IsFieldUsed(field, options_)) { + format(" const ::$proto_ns$::Message* $1$_;\n", FieldName(field)); } } } @@ -1696,7 +1695,7 @@ bool MessageGenerator::GenerateParseTable(io::Printer* printer, size_t offset, format("PROTOBUF_FIELD_OFFSET($classtype$, _has_bits_),\n"); } - if (descriptor_->oneof_decl_count() > 0) { + if (RealOneofCount(descriptor_) > 0) { format("PROTOBUF_FIELD_OFFSET($classtype$, _oneof_case_),\n"); } else { format("-1, // no _oneof_case_\n"); @@ -1755,17 +1754,17 @@ uint32 CalcFieldNum(const FieldGenerator& generator, type = internal::FieldMetadata::kStringPieceType; } } - if (field->containing_oneof()) { + + if (InRealOneof(field)) { return internal::FieldMetadata::CalculateType( type, internal::FieldMetadata::kOneOf); - } - if (field->is_packed()) { + } else if (field->is_packed()) { return internal::FieldMetadata::CalculateType( type, internal::FieldMetadata::kPacked); } else if (field->is_repeated()) { return internal::FieldMetadata::CalculateType( type, internal::FieldMetadata::kRepeated); - } else if (HasHasbit(field) || field->containing_oneof() || is_a_map) { + } else if (HasHasbit(field) || InRealOneof(field) || is_a_map) { return internal::FieldMetadata::CalculateType( type, internal::FieldMetadata::kPresence); } else { @@ -1860,7 +1859,7 @@ int MessageGenerator::GenerateFieldMetadata(io::Printer* printer) { } std::string classfieldname = FieldName(field); - if (field->containing_oneof()) { + if (InRealOneof(field)) { classfieldname = field->containing_oneof()->name(); } format.Set("field_name", classfieldname); @@ -1896,7 +1895,7 @@ int MessageGenerator::GenerateFieldMetadata(io::Printer* printer) { type = internal::FieldMetadata::kSpecial; ptr = "reinterpret_cast(::" + variables_["proto_ns"] + "::internal::LazyFieldSerializer"; - if (field->containing_oneof()) { + if (InRealOneof(field)) { ptr += "OneOf"; } else if (!HasHasbit(field)) { ptr += "NoPresence"; @@ -1913,7 +1912,7 @@ int MessageGenerator::GenerateFieldMetadata(io::Printer* printer) { "reinterpret_cast(::$proto_ns$::internal::WeakFieldSerializer)},\n", tag); - } else if (field->containing_oneof()) { + } else if (InRealOneof(field)) { format.Set("oneofoffset", sizeof(uint32) * field->containing_oneof()->index()); format( @@ -1973,10 +1972,10 @@ void MessageGenerator::GenerateDefaultInstanceInitializer( if (!field->is_repeated() && !IsLazy(field, options_) && field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE && - (field->containing_oneof() == NULL || + (!InRealOneof(field) || HasDescriptorMethods(descriptor_->file(), options_))) { std::string name; - if (field->containing_oneof() || field->options().weak()) { + if (InRealOneof(field) || field->options().weak()) { name = "_" + classname_ + "_default_instance_."; } else { name = @@ -2008,7 +2007,7 @@ void MessageGenerator::GenerateDefaultInstanceInitializer( " $1$::internal_default_instance());\n", FieldMessageTypeName(field, options_)); } - } else if (field->containing_oneof() && + } else if (InRealOneof(field) && HasDescriptorMethods(descriptor_->file(), options_)) { field_generators_.get(field).GenerateConstructorCode(printer); } @@ -2083,6 +2082,7 @@ void MessageGenerator::GenerateClassMethods(io::Printer* printer) { } if (HasHasbit(field)) { int has_bit_index = HasBitIndex(field); + GOOGLE_CHECK_NE(has_bit_index, kNoHasbit) << field->full_name(); format( "static void set_has_$1$(HasBits* has_bits) {\n" " (*has_bits)[$2$] |= $3$u;\n" @@ -2118,7 +2118,7 @@ void MessageGenerator::GenerateClassMethods(io::Printer* printer) { Formatter::SaveState saver(&format); std::map vars; SetCommonFieldVariables(field, &vars, options_); - if (field->containing_oneof()) { + if (InRealOneof(field)) { SetCommonOneofFieldVariables(field, &vars); } format.AddMap(vars); @@ -2129,7 +2129,7 @@ void MessageGenerator::GenerateClassMethods(io::Printer* printer) { GenerateStructors(printer); format("\n"); - if (descriptor_->oneof_decl_count() > 0) { + if (RealOneofCount(descriptor_) > 0) { GenerateOneofClear(printer); format("\n"); } @@ -2258,8 +2258,8 @@ size_t MessageGenerator::GenerateParseOffsets(io::Printer* printer) { processing_type |= static_cast( field->is_repeated() ? internal::kRepeatedMask : 0); - processing_type |= static_cast( - field->containing_oneof() ? internal::kOneofMask : 0); + processing_type |= + static_cast(InRealOneof(field) ? internal::kOneofMask : 0); if (field->is_map()) { processing_type = internal::TYPE_MAP; @@ -2269,7 +2269,7 @@ size_t MessageGenerator::GenerateParseOffsets(io::Printer* printer) { WireFormat::TagSize(field->number(), field->type()); std::map vars; - if (field->containing_oneof() != NULL) { + if (InRealOneof(field)) { vars["name"] = field->containing_oneof()->name(); vars["presence"] = StrCat(field->containing_oneof()->index()); } else { @@ -2400,7 +2400,7 @@ std::pair MessageGenerator::GenerateOffsets( } else { format("~0u, // no _extensions_\n"); } - if (descriptor_->oneof_decl_count() > 0) { + if (RealOneofCount(descriptor_) > 0) { format("PROTOBUF_FIELD_OFFSET($classtype$, _oneof_case_[0]),\n"); } else { format("~0u, // no _oneof_case_\n"); @@ -2418,13 +2418,13 @@ std::pair MessageGenerator::GenerateOffsets( } const int kNumGenericOffsets = 5; // the number of fixed offsets above const size_t offsets = kNumGenericOffsets + descriptor_->field_count() + - descriptor_->oneof_decl_count() - num_stripped; + RealOneofCount(descriptor_) - num_stripped; size_t entries = offsets; for (auto field : FieldRange(descriptor_)) { if (!IsFieldUsed(field, options_)) { continue; } - if (field->containing_oneof() || field->options().weak()) { + if (InRealOneof(field) || field->options().weak()) { format("offsetof($classtype$DefaultTypeInternal, $1$_)", FieldName(field)); } else { @@ -2439,9 +2439,12 @@ std::pair MessageGenerator::GenerateOffsets( format(",\n"); } + int count = 0; for (auto oneof : OneOfRange(descriptor_)) { format("PROTOBUF_FIELD_OFFSET($classtype$, $1$_),\n", oneof->name()); + count++; } + GOOGLE_CHECK_EQ(count, RealOneofCount(descriptor_)); if (IsMapEntryMessage(descriptor_)) { entries += 2; @@ -2653,7 +2656,7 @@ void MessageGenerator::GenerateStructors(io::Printer* printer) { for (auto field : optimized_order_) { GOOGLE_DCHECK(IsFieldUsed(field, options_)); bool has_arena_constructor = field->is_repeated(); - if (field->containing_oneof() == NULL && + if (!InRealOneof(field) && (IsLazy(field, options_) || IsStringPiece(field, options_))) { has_arena_constructor = true; } @@ -3010,8 +3013,8 @@ void MessageGenerator::GenerateClear(io::Printer* printer) { void MessageGenerator::GenerateOneofClear(io::Printer* printer) { // Generated function clears the active field and union case (e.g. foo_case_). - for (int i = 0; i < descriptor_->oneof_decl_count(); i++) { - auto oneof = descriptor_->oneof_decl(i); + int i = 0; + for (auto oneof : OneOfRange(descriptor_)) { Formatter format(printer, variables_); format.Set("oneofname", oneof->name()); @@ -3048,6 +3051,7 @@ void MessageGenerator::GenerateOneofClear(io::Printer* printer) { format( "}\n" "\n"); + i++; } } @@ -3118,7 +3122,8 @@ void MessageGenerator::GenerateSwap(io::Printer* printer) { format("swap($1$_, other->$1$_);\n", oneof->name()); } - for (int i = 0; i < descriptor_->oneof_decl_count(); i++) { + int count = RealOneofCount(descriptor_); + for (int i = 0; i < count; i++) { format("swap(_oneof_case_[$1$], other->_oneof_case_[$1$]);\n", i); } @@ -3567,7 +3572,7 @@ void MessageGenerator::GenerateSerializeWithCachedSizesBody( if (eager_ || MustFlush(field)) { Flush(); } - if (field->containing_oneof() == NULL) { + if (!InRealOneof(field)) { // TODO(ckennelly): Defer non-oneof fields similarly to oneof fields. if (!field->options().weak() && !field->is_repeated() && !eager_) { @@ -4009,7 +4014,7 @@ void MessageGenerator::GenerateIsInitialized(io::Printer* printer) { } else if (field->options().weak()) { continue; } else { - GOOGLE_CHECK(!field->containing_oneof()); + GOOGLE_CHECK(!InRealOneof(field)); format( "if (_internal_has_$1$()) {\n" " if (!$1$_->IsInitialized()) return false;\n" @@ -4049,7 +4054,7 @@ void MessageGenerator::GenerateIsInitialized(io::Printer* printer) { field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE && !ShouldIgnoreRequiredFieldCheck(field, options_) && scc_analyzer_->HasRequiredFields(field->message_type())) { - GOOGLE_CHECK(!(field->options().weak() || !field->containing_oneof())); + GOOGLE_CHECK(!(field->options().weak() || !InRealOneof(field))); if (field->options().weak()) { // Just skip. } else { diff --git a/src/google/protobuf/compiler/cpp/cpp_string_field.cc b/src/google/protobuf/compiler/cpp/cpp_string_field.cc index 6560cef86..43fe86062 100644 --- a/src/google/protobuf/compiler/cpp/cpp_string_field.cc +++ b/src/google/protobuf/compiler/cpp/cpp_string_field.cc @@ -281,7 +281,7 @@ void StringFieldGenerator::GenerateInlineAccessorDefinitions( "$annotate_accessor$" " // @@protoc_insertion_point(field_release:$full_name$)\n"); - if (HasFieldPresence(descriptor_->file())) { + if (HasHasbit(descriptor_)) { format( " if (!_internal_has_$name$()) {\n" " return nullptr;\n" @@ -291,7 +291,6 @@ void StringFieldGenerator::GenerateInlineAccessorDefinitions( "$default_variable$, GetArena());\n"); } else { format( - " $clear_hasbit$\n" " return $name$_.Release($default_variable$, GetArena());\n"); } @@ -386,7 +385,7 @@ void StringFieldGenerator::GenerateInlineAccessorDefinitions( "$annotate_accessor$" " // @@protoc_insertion_point(field_release:$full_name$)\n"); - if (HasFieldPresence(descriptor_->file())) { + if (HasHasbit(descriptor_)) { format( " if (!_internal_has_$name$()) {\n" " return nullptr;\n" @@ -454,10 +453,10 @@ void StringFieldGenerator::GenerateMessageClearingCode( // the minimal number of branches / amount of extraneous code at runtime // (given that the below methods are inlined one-liners)! - // If we have field presence, then the Clear() method of the protocol buffer + // If we have a hasbit, then the Clear() method of the protocol buffer // will have checked that this field is set. If so, we can avoid redundant // checks against default_variable. - const bool must_be_present = HasFieldPresence(descriptor_->file()); + const bool must_be_present = HasHasbit(descriptor_); if (inlined_ && must_be_present) { // Calling mutable_$name$() gives us a string reference and sets the has bit @@ -502,7 +501,7 @@ void StringFieldGenerator::GenerateMessageClearingCode( void StringFieldGenerator::GenerateMergingCode(io::Printer* printer) const { Formatter format(printer, variables_); - if (SupportsArenas(descriptor_) || descriptor_->containing_oneof() != NULL) { + if (SupportsArenas(descriptor_) || InRealOneof(descriptor_)) { // TODO(gpike): improve this format("_internal_set_$name$(from._internal_$name$());\n"); } else { @@ -538,7 +537,7 @@ void StringFieldGenerator::GenerateCopyConstructorCode( Formatter format(printer, variables_); GenerateConstructorCode(printer); - if (HasFieldPresence(descriptor_->file())) { + if (HasHasbit(descriptor_)) { format("if (from._internal_has_$name$()) {\n"); } else { format("if (!from._internal_$name$().empty()) {\n"); @@ -546,7 +545,7 @@ void StringFieldGenerator::GenerateCopyConstructorCode( format.Indent(); - if (SupportsArenas(descriptor_) || descriptor_->containing_oneof() != NULL) { + if (SupportsArenas(descriptor_) || InRealOneof(descriptor_)) { // TODO(gpike): improve this format( "$name$_.Set$lite$($default_variable$, from._internal_$name$(),\n" diff --git a/src/google/protobuf/compiler/python/python_generator.cc b/src/google/protobuf/compiler/python/python_generator.cc index 1e7e923d8..8762818a0 100644 --- a/src/google/protobuf/compiler/python/python_generator.cc +++ b/src/google/protobuf/compiler/python/python_generator.cc @@ -301,6 +301,10 @@ Generator::Generator() : file_(nullptr) {} Generator::~Generator() {} +uint64 Generator::GetSupportedFeatures() const { + return CodeGenerator::Feature::FEATURE_PROTO3_OPTIONAL; +} + bool Generator::Generate(const FileDescriptor* file, const std::string& parameter, GeneratorContext* context, std::string* error) const { diff --git a/src/google/protobuf/compiler/python/python_generator.h b/src/google/protobuf/compiler/python/python_generator.h index 6f299e850..d379a4c97 100644 --- a/src/google/protobuf/compiler/python/python_generator.h +++ b/src/google/protobuf/compiler/python/python_generator.h @@ -74,6 +74,8 @@ class PROTOC_EXPORT Generator : public CodeGenerator { GeneratorContext* generator_context, std::string* error) const; + uint64 GetSupportedFeatures() const override; + private: void PrintImports() const; void PrintFileDescriptor() const; diff --git a/src/google/protobuf/dynamic_message.cc b/src/google/protobuf/dynamic_message.cc index c82be66bc..971d5b7ab 100644 --- a/src/google/protobuf/dynamic_message.cc +++ b/src/google/protobuf/dynamic_message.cc @@ -103,6 +103,28 @@ namespace { bool IsMapFieldInApi(const FieldDescriptor* field) { return field->is_map(); } +// Sync with helpers.h. +inline bool HasHasbit(const FieldDescriptor* field) { + // This predicate includes proto3 message fields only if they have "optional". + // Foo submsg1 = 1; // HasHasbit() == false + // optional Foo submsg2 = 2; // HasHasbit() == true + // This is slightly odd, as adding "optional" to a singular proto3 field does + // not change the semantics or API. However whenever any field in a message + // has a hasbit, it forces reflection to include hasbit offsets for *all* + // fields, even if almost all of them are set to -1 (no hasbit). So to avoid + // causing a sudden size regression for ~all proto3 messages, we give proto3 + // message fields a hasbit only if "optional" is present. If the user is + // explicitly writing "optional", it is likely they are writing it on + // primitive fields also. + return (field->has_optional_keyword() || field->is_required()) && + !field->options().weak(); +} + +inline bool InRealOneof(const FieldDescriptor* field) { + return field->containing_oneof() && + !field->containing_oneof()->is_synthetic(); +} + // Compute the byte size of the in-memory representation of the field. int FieldSpaceUsed(const FieldDescriptor* field) { typedef FieldDescriptor FD; // avoid line wrapping @@ -356,9 +378,11 @@ void DynamicMessage::SharedCtor(bool lock_factory) { const Descriptor* descriptor = type_info_->type; // Initialize oneof cases. + int oneof_count = 0; for (int i = 0; i < descriptor->oneof_decl_count(); ++i) { - new (OffsetToPointer(type_info_->oneof_case_offset + sizeof(uint32) * i)) - uint32(0); + if (descriptor->oneof_decl(i)->is_synthetic()) continue; + new (OffsetToPointer(type_info_->oneof_case_offset + + sizeof(uint32) * oneof_count++)) uint32(0); } if (type_info_->extensions_offset != -1) { @@ -367,7 +391,7 @@ void DynamicMessage::SharedCtor(bool lock_factory) { for (int i = 0; i < descriptor->field_count(); i++) { const FieldDescriptor* field = descriptor->field(i); void* field_ptr = OffsetToPointer(type_info_->offsets[i]); - if (field->containing_oneof()) { + if (InRealOneof(field)) { continue; } switch (field->cpp_type()) { @@ -480,7 +504,7 @@ DynamicMessage::~DynamicMessage() { // be touched. for (int i = 0; i < descriptor->field_count(); i++) { const FieldDescriptor* field = descriptor->field(i); - if (field->containing_oneof()) { + if (InRealOneof(field)) { void* field_ptr = OffsetToPointer(type_info_->oneof_case_offset + sizeof(uint32) * field->containing_oneof()->index()); @@ -684,9 +708,15 @@ const Message* DynamicMessageFactory::GetPrototypeNoLock( // this block. // - A big bitfield containing a bit for each field indicating whether // or not that field is set. + int real_oneof_count = 0; + for (int i = 0; i < type->oneof_decl_count(); i++) { + if (!type->oneof_decl(i)->is_synthetic()) { + real_oneof_count++; + } + } // Compute size and offsets. - uint32* offsets = new uint32[type->field_count() + type->oneof_decl_count()]; + uint32* offsets = new uint32[type->field_count() + real_oneof_count]; type_info->offsets.reset(offsets); // Decide all field offsets by packing in order. @@ -696,26 +726,35 @@ const Message* DynamicMessageFactory::GetPrototypeNoLock( size = AlignOffset(size); // Next the has_bits, which is an array of uint32s. - if (type->file()->syntax() == FileDescriptor::SYNTAX_PROTO3) { - type_info->has_bits_offset = -1; - } else { - type_info->has_bits_offset = size; - int has_bits_array_size = - DivideRoundingUp(type->field_count(), bitsizeof(uint32)); + type_info->has_bits_offset = -1; + int max_hasbit = 0; + for (int i = 0; i < type->field_count(); i++) { + if (HasHasbit(type->field(i))) { + if (type_info->has_bits_offset == -1) { + // At least one field in the message requires a hasbit, so allocate + // hasbits. + type_info->has_bits_offset = size; + uint32* has_bits_indices = new uint32[type->field_count()]; + for (int i = 0; i < type->field_count(); i++) { + // Initialize to -1, fields that need a hasbit will overwrite. + has_bits_indices[i] = -1; + } + type_info->has_bits_indices.reset(has_bits_indices); + } + type_info->has_bits_indices[i] = max_hasbit++; + } + } + + if (max_hasbit > 0) { + int has_bits_array_size = DivideRoundingUp(max_hasbit, bitsizeof(uint32)); size += has_bits_array_size * sizeof(uint32); size = AlignOffset(size); - - uint32* has_bits_indices = new uint32[type->field_count()]; - for (int i = 0; i < type->field_count(); i++) { - has_bits_indices[i] = i; - } - type_info->has_bits_indices.reset(has_bits_indices); } // The oneof_case, if any. It is an array of uint32s. - if (type->oneof_decl_count() > 0) { + if (real_oneof_count > 0) { type_info->oneof_case_offset = size; - size += type->oneof_decl_count() * sizeof(uint32); + size += real_oneof_count * sizeof(uint32); size = AlignOffset(size); } @@ -736,7 +775,7 @@ const Message* DynamicMessageFactory::GetPrototypeNoLock( for (int i = 0; i < type->field_count(); i++) { // Make sure field is aligned to avoid bus errors. // Oneof fields do not use any space. - if (!type->field(i)->containing_oneof()) { + if (!InRealOneof(type->field(i))) { int field_size = FieldSpaceUsed(type->field(i)); size = AlignTo(size, std::min(kSafeAlignment, field_size)); offsets[i] = size; @@ -746,9 +785,11 @@ const Message* DynamicMessageFactory::GetPrototypeNoLock( // The oneofs. for (int i = 0; i < type->oneof_decl_count(); i++) { - size = AlignTo(size, kSafeAlignment); - offsets[type->field_count() + i] = size; - size += kMaxOneofUnionSize; + if (!type->oneof_decl(i)->is_synthetic()) { + size = AlignTo(size, kSafeAlignment); + offsets[type->field_count() + i] = size; + size += kMaxOneofUnionSize; + } } type_info->weak_field_map_offset = -1; @@ -759,17 +800,16 @@ const Message* DynamicMessageFactory::GetPrototypeNoLock( // Construct the reflection object. - if (type->oneof_decl_count() > 0) { - // Compute the size of default oneof instance and offsets of default - // oneof fields. - for (int i = 0; i < type->oneof_decl_count(); i++) { - for (int j = 0; j < type->oneof_decl(i)->field_count(); j++) { - const FieldDescriptor* field = type->oneof_decl(i)->field(j); - int field_size = OneofFieldSpaceUsed(field); - size = AlignTo(size, std::min(kSafeAlignment, field_size)); - offsets[field->index()] = size; - size += field_size; - } + // Compute the size of default oneof instance and offsets of default + // oneof fields. + for (int i = 0; i < type->oneof_decl_count(); i++) { + if (type->oneof_decl(i)->is_synthetic()) continue; + for (int j = 0; j < type->oneof_decl(i)->field_count(); j++) { + const FieldDescriptor* field = type->oneof_decl(i)->field(j); + int field_size = OneofFieldSpaceUsed(field); + size = AlignTo(size, std::min(kSafeAlignment, field_size)); + offsets[field->index()] = size; + size += field_size; } } size = AlignOffset(size); @@ -781,7 +821,7 @@ const Message* DynamicMessageFactory::GetPrototypeNoLock( // of dynamic message to avoid dead lock. DynamicMessage* prototype = new (base) DynamicMessage(type_info, false); - if (type->oneof_decl_count() > 0 || num_weak_fields > 0) { + if (real_oneof_count > 0 || num_weak_fields > 0) { // Construct default oneof instance. ConstructDefaultOneofInstance(type_info->type, type_info->offsets.get(), prototype); @@ -811,6 +851,7 @@ void DynamicMessageFactory::ConstructDefaultOneofInstance( const Descriptor* type, const uint32 offsets[], void* default_oneof_or_weak_instance) { for (int i = 0; i < type->oneof_decl_count(); i++) { + if (type->oneof_decl(i)->is_synthetic()) continue; for (int j = 0; j < type->oneof_decl(i)->field_count(); j++) { const FieldDescriptor* field = type->oneof_decl(i)->field(j); void* field_ptr = @@ -857,6 +898,7 @@ void DynamicMessageFactory::DeleteDefaultOneofInstance( const Descriptor* type, const uint32 offsets[], const void* default_oneof_instance) { for (int i = 0; i < type->oneof_decl_count(); i++) { + if (type->oneof_decl(i)->is_synthetic()) continue; for (int j = 0; j < type->oneof_decl(i)->field_count(); j++) { const FieldDescriptor* field = type->oneof_decl(i)->field(j); if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) { diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index 373192bd2..6ee8c984e 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc @@ -287,7 +287,7 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { break; } } else { - if (field->containing_oneof() && !HasOneofField(message, field)) { + if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { continue; } switch (field->cpp_type()) { @@ -476,6 +476,7 @@ void Reflection::SwapField(Message* message1, Message* message2, void Reflection::SwapOneofField(Message* message1, Message* message2, const OneofDescriptor* oneof_descriptor) const { + GOOGLE_DCHECK(!oneof_descriptor->is_synthetic()); uint32 oneof_case1 = GetOneofCase(*message1, oneof_descriptor); uint32 oneof_case2 = GetOneofCase(*message2, oneof_descriptor); @@ -632,7 +633,7 @@ void Reflection::Swap(Message* message1, Message* message2) const { int fields_with_has_bits = 0; for (int i = 0; i < descriptor_->field_count(); i++) { const FieldDescriptor* field = descriptor_->field(i); - if (field->is_repeated() || field->containing_oneof()) { + if (field->is_repeated() || schema_.InRealOneof(field)) { continue; } fields_with_has_bits++; @@ -647,12 +648,15 @@ void Reflection::Swap(Message* message1, Message* message2) const { for (int i = 0; i <= last_non_weak_field_index_; i++) { const FieldDescriptor* field = descriptor_->field(i); - if (field->containing_oneof()) continue; + if (schema_.InRealOneof(field)) continue; SwapField(message1, message2, field); } const int oneof_decl_count = descriptor_->oneof_decl_count(); for (int i = 0; i < oneof_decl_count; i++) { - SwapOneofField(message1, message2, descriptor_->oneof_decl(i)); + const OneofDescriptor* oneof = descriptor_->oneof_decl(i); + if (!oneof->is_synthetic()) { + SwapOneofField(message1, message2, oneof); + } } if (schema_.HasExtensionSet()) { @@ -694,7 +698,7 @@ void Reflection::SwapFields( MutableExtensionSet(message1)->SwapExtension( MutableExtensionSet(message2), field->number()); } else { - if (field->containing_oneof()) { + if (schema_.InRealOneof(field)) { int oneof_index = field->containing_oneof()->index(); // Only swap the oneof field once. if (swapped_oneof.find(oneof_index) != swapped_oneof.end()) { @@ -725,7 +729,7 @@ bool Reflection::HasField(const Message& message, if (field->is_extension()) { return GetExtensionSet(message).Has(field->number()); } else { - if (field->containing_oneof()) { + if (schema_.InRealOneof(field)) { return HasOneofField(message, field); } else { return HasBit(message, field); @@ -785,7 +789,7 @@ void Reflection::ClearField(Message* message, if (field->is_extension()) { MutableExtensionSet(message)->ClearExtension(field->number()); } else if (!field->is_repeated()) { - if (field->containing_oneof()) { + if (schema_.InRealOneof(field)) { ClearOneofField(message, field); return; } @@ -1050,7 +1054,7 @@ void Reflection::ListFields(const Message& message, } } else { const OneofDescriptor* containing_oneof = field->containing_oneof(); - if (containing_oneof) { + if (schema_.InRealOneof(field)) { const uint32* const oneof_case_array = GetConstPointerAtOffset( &message, schema_.oneof_case_offset_); // Equivalent to: HasOneofField(message, field) @@ -1208,7 +1212,7 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, const std::string* default_ptr = &DefaultRaw(field).Get(); - if (field->containing_oneof() && !HasOneofField(*message, field)) { + if (schema_.InRealOneof(field) && !HasOneofField(*message, field)) { ClearOneof(message, field->containing_oneof()); MutableField(message, field) ->UnsafeSetDefault(default_ptr); @@ -1475,7 +1479,7 @@ Message* Reflection::MutableMessage(Message* message, Message** result_holder = MutableRaw(message, field); - if (field->containing_oneof()) { + if (schema_.InRealOneof(field)) { if (!HasOneofField(*message, field)) { ClearOneof(message, field->containing_oneof()); result_holder = MutableField(message, field); @@ -1504,7 +1508,7 @@ void Reflection::UnsafeArenaSetAllocatedMessage( MutableExtensionSet(message)->UnsafeArenaSetAllocatedMessage( field->number(), field->type(), field, sub_message); } else { - if (field->containing_oneof()) { + if (schema_.InRealOneof(field)) { if (sub_message == nullptr) { ClearOneof(message, field->containing_oneof()); return; @@ -1566,10 +1570,10 @@ Message* Reflection::UnsafeArenaReleaseMessage(Message* message, MutableExtensionSet(message)->UnsafeArenaReleaseMessage(field, factory)); } else { - if (!(field->is_repeated() || field->containing_oneof())) { + if (!(field->is_repeated() || schema_.InRealOneof(field))) { ClearBit(message, field); } - if (field->containing_oneof()) { + if (schema_.InRealOneof(field)) { if (HasOneofField(*message, field)) { *MutableOneofCase(message, field->containing_oneof()) = 0; } else { @@ -1754,6 +1758,10 @@ const void* Reflection::GetRawRepeatedField(const Message& message, const FieldDescriptor* Reflection::GetOneofFieldDescriptor( const Message& message, const OneofDescriptor* oneof_descriptor) const { + if (oneof_descriptor->is_synthetic()) { + const FieldDescriptor* field = oneof_descriptor->field(0); + return HasField(message, field) ? field : nullptr; + } uint32 field_number = GetOneofCase(message, oneof_descriptor); if (field_number == 0) { return nullptr; @@ -1850,7 +1858,7 @@ Type* Reflection::MutableRawNonOneof(Message* message, template const Type& Reflection::GetRaw(const Message& message, const FieldDescriptor* field) const { - if (field->containing_oneof() && !HasOneofField(message, field)) { + if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return DefaultRaw(field); } return GetConstRefAtOffset(message, schema_.GetFieldOffset(field)); @@ -1878,12 +1886,14 @@ uint32* Reflection::MutableHasBits(Message* message) const { uint32 Reflection::GetOneofCase(const Message& message, const OneofDescriptor* oneof_descriptor) const { + GOOGLE_DCHECK(!oneof_descriptor->is_synthetic()); return GetConstRefAtOffset( message, schema_.GetOneofCaseOffset(oneof_descriptor)); } uint32* Reflection::MutableOneofCase( Message* message, const OneofDescriptor* oneof_descriptor) const { + GOOGLE_DCHECK(!oneof_descriptor->is_synthetic()); return GetPointerAtOffset( message, schema_.GetOneofCaseOffset(oneof_descriptor)); } @@ -1917,7 +1927,7 @@ InternalMetadata* Reflection::MutableInternalMetadata(Message* message) const { bool Reflection::HasBit(const Message& message, const FieldDescriptor* field) const { GOOGLE_DCHECK(!field->options().weak()); - if (schema_.HasHasbits()) { + if (schema_.HasBitIndex(field) != -1) { return IsIndexInHasBitSet(GetHasBits(message), schema_.HasBitIndex(field)); } @@ -1977,10 +1987,8 @@ bool Reflection::HasBit(const Message& message, void Reflection::SetBit(Message* message, const FieldDescriptor* field) const { GOOGLE_DCHECK(!field->options().weak()); - if (!schema_.HasHasbits()) { - return; - } const uint32 index = schema_.HasBitIndex(field); + if (index == -1) return; MutableHasBits(message)[index / 32] |= (static_cast(1) << (index % 32)); } @@ -2017,6 +2025,9 @@ void Reflection::SwapBit(Message* message1, Message* message2, bool Reflection::HasOneof(const Message& message, const OneofDescriptor* oneof_descriptor) const { + if (oneof_descriptor->is_synthetic()) { + return HasField(message, oneof_descriptor->field(0)); + } return (GetOneofCase(message, oneof_descriptor) > 0); } @@ -2039,6 +2050,10 @@ void Reflection::ClearOneofField(Message* message, void Reflection::ClearOneof(Message* message, const OneofDescriptor* oneof_descriptor) const { + if (oneof_descriptor->is_synthetic()) { + ClearField(message, oneof_descriptor->field(0)); + return; + } // TODO(jieluo): Consider to cache the unused object instead of deleting // it. It will be much faster if an application switches a lot from // a few oneof fields. Time/space tradeoff @@ -2119,19 +2134,19 @@ const Type& Reflection::GetField(const Message& message, template void Reflection::SetField(Message* message, const FieldDescriptor* field, const Type& value) const { - if (field->containing_oneof() && !HasOneofField(*message, field)) { + bool real_oneof = schema_.InRealOneof(field); + if (real_oneof && !HasOneofField(*message, field)) { ClearOneof(message, field->containing_oneof()); } *MutableRaw(message, field) = value; - field->containing_oneof() ? SetOneofCase(message, field) - : SetBit(message, field); + real_oneof ? SetOneofCase(message, field) : SetBit(message, field); } template Type* Reflection::MutableField(Message* message, const FieldDescriptor* field) const { - field->containing_oneof() ? SetOneofCase(message, field) - : SetBit(message, field); + schema_.InRealOneof(field) ? SetOneofCase(message, field) + : SetBit(message, field); return MutableRaw(message, field); } diff --git a/src/google/protobuf/generated_message_reflection.h b/src/google/protobuf/generated_message_reflection.h index 2c2e40ca9..7aab17f8a 100644 --- a/src/google/protobuf/generated_message_reflection.h +++ b/src/google/protobuf/generated_message_reflection.h @@ -124,16 +124,21 @@ struct ReflectionSchema { // Size of a google::protobuf::Message object of this type. uint32 GetObjectSize() const { return static_cast(object_size_); } + bool InRealOneof(const FieldDescriptor* field) const { + return field->containing_oneof() && + !field->containing_oneof()->is_synthetic(); + } + // Offset of a non-oneof field. Getting a field offset is slightly more // efficient when we know statically that it is not a oneof field. uint32 GetFieldOffsetNonOneof(const FieldDescriptor* field) const { - GOOGLE_DCHECK(!field->containing_oneof()); + GOOGLE_DCHECK(!InRealOneof(field)); return OffsetValue(offsets_[field->index()], field->type()); } // Offset of any field. uint32 GetFieldOffset(const FieldDescriptor* field) const { - if (field->containing_oneof()) { + if (InRealOneof(field)) { size_t offset = static_cast(field->containing_type()->field_count() + field->containing_oneof()->index()); @@ -144,7 +149,7 @@ struct ReflectionSchema { } bool IsFieldInlined(const FieldDescriptor* field) const { - if (field->containing_oneof()) { + if (InRealOneof(field)) { size_t offset = static_cast(field->containing_type()->field_count() + field->containing_oneof()->index()); @@ -164,6 +169,7 @@ struct ReflectionSchema { // Bit index within the bit array of hasbits. Bit order is low-to-high. uint32 HasBitIndex(const FieldDescriptor* field) const { + if (has_bits_offset_ == -1) return -1; GOOGLE_DCHECK(HasHasbits()); return has_bit_indices_[field->index()]; } diff --git a/src/google/protobuf/proto3_arena_unittest.cc b/src/google/protobuf/proto3_arena_unittest.cc index 60440d520..8e6e01528 100644 --- a/src/google/protobuf/proto3_arena_unittest.cc +++ b/src/google/protobuf/proto3_arena_unittest.cc @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -196,6 +197,22 @@ TEST(Proto3ArenaTest, MessageFieldClearViaReflection) { } TEST(Proto3OptionalTest, OptionalFields) { + protobuf_unittest::TestProto3Optional msg; + EXPECT_FALSE(msg.has_optional_int32()); + msg.set_optional_int32(0); + EXPECT_TRUE(msg.has_optional_int32()); + + string serialized; + msg.SerializeToString(&serialized); + EXPECT_GT(serialized.size(), 0); + + msg.clear_optional_int32(); + EXPECT_FALSE(msg.has_optional_int32()); + msg.SerializeToString(&serialized); + EXPECT_EQ(serialized.size(), 0); +} + +TEST(Proto3OptionalTest, OptionalFieldDescriptor) { const Descriptor* d = protobuf_unittest::TestProto3Optional::descriptor(); for (int i = 0; i < d->field_count(); i++) { @@ -206,6 +223,250 @@ TEST(Proto3OptionalTest, OptionalFields) { } } +TEST(Proto3OptionalTest, OptionalField) { + protobuf_unittest::TestProto3Optional msg; + EXPECT_FALSE(msg.has_optional_int32()); + msg.set_optional_int32(0); + EXPECT_TRUE(msg.has_optional_int32()); + + string serialized; + msg.SerializeToString(&serialized); + EXPECT_GT(serialized.size(), 0); + + msg.clear_optional_int32(); + EXPECT_FALSE(msg.has_optional_int32()); + msg.SerializeToString(&serialized); + EXPECT_EQ(serialized.size(), 0); +} + +TEST(Proto3OptionalTest, OptionalFieldReflection) { + // Tests that oneof reflection works on synthetic oneofs. + // + // We test this more deeply elsewhere by parsing/serializing TextFormat (which + // doesn't treat synthetic oneofs specially, so reflects over them normally). + protobuf_unittest::TestProto3Optional msg; + const google::protobuf::Descriptor* d = msg.GetDescriptor(); + const google::protobuf::Reflection* r = msg.GetReflection(); + const google::protobuf::FieldDescriptor* f = d->FindFieldByName("optional_int32"); + const google::protobuf::OneofDescriptor* o = d->FindOneofByName("_optional_int32"); + GOOGLE_CHECK(f); + GOOGLE_CHECK(o); + EXPECT_TRUE(o->is_synthetic()); + + EXPECT_FALSE(r->HasField(msg, f)); + EXPECT_FALSE(r->HasOneof(msg, o)); + EXPECT_TRUE(r->GetOneofFieldDescriptor(msg, o) == nullptr); + + r->SetInt32(&msg, f, 123); + EXPECT_EQ(123, msg.optional_int32()); + EXPECT_EQ(123, r->GetInt32(msg, f)); + EXPECT_TRUE(r->HasField(msg, f)); + EXPECT_TRUE(r->HasOneof(msg, o)); + EXPECT_EQ(f, r->GetOneofFieldDescriptor(msg, o)); + + std::vector fields; + r->ListFields(msg, &fields); + EXPECT_EQ(1, fields.size()); + EXPECT_EQ(f, fields[0]); + + r->ClearOneof(&msg, o); + EXPECT_FALSE(r->HasField(msg, f)); + EXPECT_FALSE(r->HasOneof(msg, o)); + EXPECT_TRUE(r->GetOneofFieldDescriptor(msg, o) == nullptr); + + msg.set_optional_int32(123); + EXPECT_EQ(123, r->GetInt32(msg, f)); + EXPECT_TRUE(r->HasField(msg, f)); + EXPECT_TRUE(r->HasOneof(msg, o)); + EXPECT_EQ(f, r->GetOneofFieldDescriptor(msg, o)); + + r->ClearOneof(&msg, o); + EXPECT_FALSE(r->HasField(msg, f)); + EXPECT_FALSE(r->HasOneof(msg, o)); + EXPECT_TRUE(r->GetOneofFieldDescriptor(msg, o) == nullptr); +} + +void SetAllFieldsZero(protobuf_unittest::TestProto3Optional* msg) { + msg->set_optional_int32(0); + msg->set_optional_int64(0); + msg->set_optional_uint32(0); + msg->set_optional_uint64(0); + msg->set_optional_sint32(0); + msg->set_optional_sint64(0); + msg->set_optional_fixed32(0); + msg->set_optional_fixed64(0); + msg->set_optional_sfixed32(0); + msg->set_optional_sfixed64(0); + msg->set_optional_float(0); + msg->set_optional_double(0); + msg->set_optional_bool(false); + msg->set_optional_string(""); + msg->set_optional_bytes(""); + msg->mutable_optional_nested_message(); + msg->mutable_lazy_nested_message(); + msg->set_optional_nested_enum( + protobuf_unittest::TestProto3Optional::UNSPECIFIED); +} + +void SetAllFieldsNonZero(protobuf_unittest::TestProto3Optional* msg) { + msg->set_optional_int32(101); + msg->set_optional_int64(102); + msg->set_optional_uint32(103); + msg->set_optional_uint64(104); + msg->set_optional_sint32(105); + msg->set_optional_sint64(106); + msg->set_optional_fixed32(107); + msg->set_optional_fixed64(108); + msg->set_optional_sfixed32(109); + msg->set_optional_sfixed64(110); + msg->set_optional_float(111); + msg->set_optional_double(112); + msg->set_optional_bool(true); + msg->set_optional_string("abc"); + msg->set_optional_bytes("def"); + msg->mutable_optional_nested_message(); + msg->mutable_lazy_nested_message(); + msg->set_optional_nested_enum(protobuf_unittest::TestProto3Optional::BAZ); +} + +void TestAllFieldsZero(const protobuf_unittest::TestProto3Optional& msg) { + EXPECT_EQ(0, msg.optional_int32()); + EXPECT_EQ(0, msg.optional_int64()); + EXPECT_EQ(0, msg.optional_uint32()); + EXPECT_EQ(0, msg.optional_uint64()); + EXPECT_EQ(0, msg.optional_sint32()); + EXPECT_EQ(0, msg.optional_sint64()); + EXPECT_EQ(0, msg.optional_fixed32()); + EXPECT_EQ(0, msg.optional_fixed64()); + EXPECT_EQ(0, msg.optional_sfixed32()); + EXPECT_EQ(0, msg.optional_sfixed64()); + EXPECT_EQ(0, msg.optional_float()); + EXPECT_EQ(0, msg.optional_double()); + EXPECT_EQ(0, msg.optional_bool()); + EXPECT_EQ("", msg.optional_string()); + EXPECT_EQ("", msg.optional_bytes()); + EXPECT_EQ(protobuf_unittest::TestProto3Optional::UNSPECIFIED, + msg.optional_nested_enum()); + + const Reflection* r = msg.GetReflection(); + const Descriptor* d = msg.GetDescriptor(); + EXPECT_EQ("", r->GetString(msg, d->FindFieldByName("optional_string"))); +} + +void TestAllFieldsNonZero(const protobuf_unittest::TestProto3Optional& msg) { + EXPECT_EQ(101, msg.optional_int32()); + EXPECT_EQ(102, msg.optional_int64()); + EXPECT_EQ(103, msg.optional_uint32()); + EXPECT_EQ(104, msg.optional_uint64()); + EXPECT_EQ(105, msg.optional_sint32()); + EXPECT_EQ(106, msg.optional_sint64()); + EXPECT_EQ(107, msg.optional_fixed32()); + EXPECT_EQ(108, msg.optional_fixed64()); + EXPECT_EQ(109, msg.optional_sfixed32()); + EXPECT_EQ(110, msg.optional_sfixed64()); + EXPECT_EQ(111, msg.optional_float()); + EXPECT_EQ(112, msg.optional_double()); + EXPECT_EQ(true, msg.optional_bool()); + EXPECT_EQ("abc", msg.optional_string()); + EXPECT_EQ("def", msg.optional_bytes()); + EXPECT_EQ(protobuf_unittest::TestProto3Optional::BAZ, + msg.optional_nested_enum()); +} + +void TestAllFieldsSet(const protobuf_unittest::TestProto3Optional& msg, + bool set) { + EXPECT_EQ(set, msg.has_optional_int32()); + EXPECT_EQ(set, msg.has_optional_int64()); + EXPECT_EQ(set, msg.has_optional_uint32()); + EXPECT_EQ(set, msg.has_optional_uint64()); + EXPECT_EQ(set, msg.has_optional_sint32()); + EXPECT_EQ(set, msg.has_optional_sint64()); + EXPECT_EQ(set, msg.has_optional_fixed32()); + EXPECT_EQ(set, msg.has_optional_fixed64()); + EXPECT_EQ(set, msg.has_optional_sfixed32()); + EXPECT_EQ(set, msg.has_optional_sfixed64()); + EXPECT_EQ(set, msg.has_optional_float()); + EXPECT_EQ(set, msg.has_optional_double()); + EXPECT_EQ(set, msg.has_optional_bool()); + EXPECT_EQ(set, msg.has_optional_string()); + EXPECT_EQ(set, msg.has_optional_bytes()); + EXPECT_EQ(set, msg.has_optional_nested_message()); + EXPECT_EQ(set, msg.has_lazy_nested_message()); + EXPECT_EQ(set, msg.has_optional_nested_enum()); +} + +TEST(Proto3OptionalTest, BinaryRoundTrip) { + protobuf_unittest::TestProto3Optional msg; + TestAllFieldsSet(msg, false); + SetAllFieldsZero(&msg); + TestAllFieldsZero(msg); + TestAllFieldsSet(msg, true); + + protobuf_unittest::TestProto3Optional msg2; + std::string serialized; + msg.SerializeToString(&serialized); + EXPECT_TRUE(msg2.ParseFromString(serialized)); + TestAllFieldsZero(msg2); + TestAllFieldsSet(msg2, true); +} + +TEST(Proto3OptionalTest, TextFormatRoundTripZeros) { + protobuf_unittest::TestProto3Optional msg; + SetAllFieldsZero(&msg); + + protobuf_unittest::TestProto3Optional msg2; + std::string text; + EXPECT_TRUE(TextFormat::PrintToString(msg, &text)); + EXPECT_TRUE(TextFormat::ParseFromString(text, &msg2)); + TestAllFieldsSet(msg2, true); + TestAllFieldsZero(msg2); +} + +TEST(Proto3OptionalTest, TextFormatRoundTripNonZeros) { + protobuf_unittest::TestProto3Optional msg; + SetAllFieldsNonZero(&msg); + + protobuf_unittest::TestProto3Optional msg2; + std::string text; + EXPECT_TRUE(TextFormat::PrintToString(msg, &text)); + EXPECT_TRUE(TextFormat::ParseFromString(text, &msg2)); + TestAllFieldsSet(msg2, true); + TestAllFieldsNonZero(msg2); +} + +TEST(Proto3OptionalTest, SwapRoundTripZero) { + protobuf_unittest::TestProto3Optional msg; + SetAllFieldsZero(&msg); + TestAllFieldsSet(msg, true); + + protobuf_unittest::TestProto3Optional msg2; + msg.Swap(&msg2); + TestAllFieldsSet(msg2, true); + TestAllFieldsZero(msg2); +} + +TEST(Proto3OptionalTest, SwapRoundTripNonZero) { + protobuf_unittest::TestProto3Optional msg; + SetAllFieldsNonZero(&msg); + TestAllFieldsSet(msg, true); + + protobuf_unittest::TestProto3Optional msg2; + msg.Swap(&msg2); + TestAllFieldsSet(msg2, true); + TestAllFieldsNonZero(msg2); +} + +TEST(Proto3OptionalTest, ReflectiveSwapRoundTrip) { + protobuf_unittest::TestProto3Optional msg; + SetAllFieldsZero(&msg); + TestAllFieldsSet(msg, true); + + protobuf_unittest::TestProto3Optional msg2; + msg2.GetReflection()->Swap(&msg, &msg2); + TestAllFieldsSet(msg2, true); + TestAllFieldsZero(msg2); +} + TEST(Proto3OptionalTest, PlainFields) { const Descriptor* d = TestAllTypes::descriptor(); diff --git a/src/google/protobuf/source_context.pb.h b/src/google/protobuf/source_context.pb.h index 85062c910..1cf16f6dd 100644 --- a/src/google/protobuf/source_context.pb.h +++ b/src/google/protobuf/source_context.pb.h @@ -277,7 +277,6 @@ inline std::string* SourceContext::_internal_mutable_file_name() { } inline std::string* SourceContext::release_file_name() { // @@protoc_insertion_point(field_release:google.protobuf.SourceContext.file_name) - return file_name_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void SourceContext::set_allocated_file_name(std::string* file_name) { diff --git a/src/google/protobuf/type.pb.h b/src/google/protobuf/type.pb.h index 6e589f223..680679f0d 100644 --- a/src/google/protobuf/type.pb.h +++ b/src/google/protobuf/type.pb.h @@ -1475,7 +1475,6 @@ inline std::string* Type::_internal_mutable_name() { } inline std::string* Type::release_name() { // @@protoc_insertion_point(field_release:google.protobuf.Type.name) - return name_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Type::set_allocated_name(std::string* name) { @@ -1868,7 +1867,6 @@ inline std::string* Field::_internal_mutable_name() { } inline std::string* Field::release_name() { // @@protoc_insertion_point(field_release:google.protobuf.Field.name) - return name_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Field::set_allocated_name(std::string* name) { @@ -1950,7 +1948,6 @@ inline std::string* Field::_internal_mutable_type_url() { } inline std::string* Field::release_type_url() { // @@protoc_insertion_point(field_release:google.protobuf.Field.type_url) - return type_url_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Field::set_allocated_type_url(std::string* type_url) { @@ -2111,7 +2108,6 @@ inline std::string* Field::_internal_mutable_json_name() { } inline std::string* Field::release_json_name() { // @@protoc_insertion_point(field_release:google.protobuf.Field.json_name) - return json_name_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Field::set_allocated_json_name(std::string* json_name) { @@ -2193,7 +2189,6 @@ inline std::string* Field::_internal_mutable_default_value() { } inline std::string* Field::release_default_value() { // @@protoc_insertion_point(field_release:google.protobuf.Field.default_value) - return default_value_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Field::set_allocated_default_value(std::string* default_value) { @@ -2279,7 +2274,6 @@ inline std::string* Enum::_internal_mutable_name() { } inline std::string* Enum::release_name() { // @@protoc_insertion_point(field_release:google.protobuf.Enum.name) - return name_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Enum::set_allocated_name(std::string* name) { @@ -2538,7 +2532,6 @@ inline std::string* EnumValue::_internal_mutable_name() { } inline std::string* EnumValue::release_name() { // @@protoc_insertion_point(field_release:google.protobuf.EnumValue.name) - return name_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void EnumValue::set_allocated_name(std::string* name) { @@ -2683,7 +2676,6 @@ inline std::string* Option::_internal_mutable_name() { } inline std::string* Option::release_name() { // @@protoc_insertion_point(field_release:google.protobuf.Option.name) - return name_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void Option::set_allocated_name(std::string* name) { diff --git a/src/google/protobuf/unittest_proto3_optional.proto b/src/google/protobuf/unittest_proto3_optional.proto index b93e7550b..b32a5d22b 100644 --- a/src/google/protobuf/unittest_proto3_optional.proto +++ b/src/google/protobuf/unittest_proto3_optional.proto @@ -52,22 +52,24 @@ message TestProto3Optional { } // Singular - optional int32 optional_int32 = 1; - optional int64 optional_int64 = 2; - optional uint32 optional_uint32 = 3; - optional uint64 optional_uint64 = 4; - optional sint32 optional_sint32 = 5; - optional sint64 optional_sint64 = 6; - optional fixed32 optional_fixed32 = 7; - optional fixed64 optional_fixed64 = 8; - optional sfixed32 optional_sfixed32 = 9; + optional int32 optional_int32 = 1; + optional int64 optional_int64 = 2; + optional uint32 optional_uint32 = 3; + optional uint64 optional_uint64 = 4; + optional sint32 optional_sint32 = 5; + optional sint64 optional_sint64 = 6; + optional fixed32 optional_fixed32 = 7; + optional fixed64 optional_fixed64 = 8; + optional sfixed32 optional_sfixed32 = 9; optional sfixed64 optional_sfixed64 = 10; - optional float optional_float = 11; - optional double optional_double = 12; - optional bool optional_bool = 13; - optional string optional_string = 14; - optional bytes optional_bytes = 15; + optional float optional_float = 11; + optional double optional_double = 12; + optional bool optional_bool = 13; + optional string optional_string = 14; + optional bytes optional_bytes = 15; + optional string optional_cord = 16 [ctype = CORD]; - optional NestedMessage optional_nested_message = 18; - optional NestedEnum optional_nested_enum = 21; + optional NestedMessage optional_nested_message = 18; + optional NestedMessage lazy_nested_message = 19 [lazy = true]; + optional NestedEnum optional_nested_enum = 21; } diff --git a/src/google/protobuf/wrappers.pb.h b/src/google/protobuf/wrappers.pb.h index 5779c559c..7a1cc4dcc 100644 --- a/src/google/protobuf/wrappers.pb.h +++ b/src/google/protobuf/wrappers.pb.h @@ -1589,7 +1589,6 @@ inline std::string* StringValue::_internal_mutable_value() { } inline std::string* StringValue::release_value() { // @@protoc_insertion_point(field_release:google.protobuf.StringValue.value) - return value_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void StringValue::set_allocated_value(std::string* value) { @@ -1675,7 +1674,6 @@ inline std::string* BytesValue::_internal_mutable_value() { } inline std::string* BytesValue::release_value() { // @@protoc_insertion_point(field_release:google.protobuf.BytesValue.value) - return value_.Release(&::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } inline void BytesValue::set_allocated_value(std::string* value) {