Merge branch 'sync-piper' into sync-integrate

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                      \

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)

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()

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):

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;
+}

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):

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;

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)
 
 

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) {

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) {

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;

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);

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_(

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;

src/google/protobuf/compiler/cpp/cpp_message.cc

@@ -201,10 +201,11 @@ RunMap FindRuns(const std::vector<const FieldDescriptor*>& 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;
       }
-    }
-    for (auto field : FieldRange(descriptor_)) {
-      if (field->options().weak() && IsFieldUsed(field, options_)) {
-        format("  const ::$proto_ns$::Message* $1$_;\n", FieldName(field));
+      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));
     }
   }
 }
@@ -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<const void*>(::" + 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<const "
           "void*>(::$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<std::string, std::string> 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<unsigned>(
         field->is_repeated() ? internal::kRepeatedMask : 0);
-    processing_type |= static_cast<unsigned>(
-        field->containing_oneof() ? internal::kOneofMask : 0);
+    processing_type |=
+        static_cast<unsigned>(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<std::string, std::string> 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<size_t, size_t> 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<size_t, size_t> 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<size_t, size_t> 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 {

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"

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 {

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;

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) {

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<uint32>(
             &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<ArenaStringPtr>(field).Get();
-        if (field->containing_oneof() && !HasOneofField(*message, field)) {
+        if (schema_.InRealOneof(field) && !HasOneofField(*message, field)) {
           ClearOneof(message, field->containing_oneof());
           MutableField<ArenaStringPtr>(message, field)
               ->UnsafeSetDefault(default_ptr);
@@ -1475,7 +1479,7 @@ Message* Reflection::MutableMessage(Message* message,
 
     Message** result_holder = MutableRaw<Message*>(message, field);
 
-    if (field->containing_oneof()) {
+    if (schema_.InRealOneof(field)) {
       if (!HasOneofField(*message, field)) {
         ClearOneof(message, field->containing_oneof());
         result_holder = MutableField<Message*>(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 <typename Type>
 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<Type>(field);
   }
   return GetConstRefAtOffset<Type>(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<uint32>(
       message, schema_.GetOneofCaseOffset(oneof_descriptor));
 }
 
 uint32* Reflection::MutableOneofCase(
     Message* message, const OneofDescriptor* oneof_descriptor) const {
+  GOOGLE_DCHECK(!oneof_descriptor->is_synthetic());
   return GetPointerAtOffset<uint32>(
       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<uint32>(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 <typename Type>
 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<Type>(message, field) = value;
-  field->containing_oneof() ? SetOneofCase(message, field)
-                            : SetBit(message, field);
+  real_oneof ? SetOneofCase(message, field) : SetBit(message, field);
 }
 
 template <typename Type>
 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<Type>(message, field);
 }
 

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<uint32>(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<size_t>(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<size_t>(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()];
   }

src/google/protobuf/proto3_arena_unittest.cc

@@ -37,6 +37,7 @@
 #include <google/protobuf/unittest_proto3_arena.pb.h>
 #include <google/protobuf/unittest_proto3_optional.pb.h>
 #include <google/protobuf/arena.h>
+#include <google/protobuf/text_format.h>
 #include <google/protobuf/testing/googletest.h>
 #include <gtest/gtest.h>
 
@@ -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<const FieldDescriptor*> 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();
 

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) {

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) {

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;
 }

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) {