diff --git a/objectivec/Tests/GPBMessageTests.m b/objectivec/Tests/GPBMessageTests.m index 43546156c1..0779c5ae76 100644 --- a/objectivec/Tests/GPBMessageTests.m +++ b/objectivec/Tests/GPBMessageTests.m @@ -1947,4 +1947,11 @@ EnumTestMsg_MyEnum_NegTwo); } +- (void)testOneBasedEnumHolder { + // Test case for https://github.com/google/protobuf/issues/1453 + // Message with no explicit defaults, but a non zero default for an enum. + MessageWithOneBasedEnum *enumMsg = [MessageWithOneBasedEnum message]; + XCTAssertEqual(enumMsg.enumField, MessageWithOneBasedEnum_OneBasedEnum_One); +} + @end diff --git a/objectivec/Tests/unittest_objc.proto b/objectivec/Tests/unittest_objc.proto index 9483cb1d7f..6bcfbf705a 100644 --- a/objectivec/Tests/unittest_objc.proto +++ b/objectivec/Tests/unittest_objc.proto @@ -401,3 +401,13 @@ message EnumTestMsg { repeated MyEnum mumble = 4; } + +// Test case for https://github.com/google/protobuf/issues/1453 +// Message with no explicit defaults, but a non zero default for an enum. +message MessageWithOneBasedEnum { + enum OneBasedEnum { + ONE = 1; + TWO = 2; + } + optional OneBasedEnum enum_field = 1; +} diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc index fda5180712..196b39dd93 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc @@ -757,10 +757,11 @@ bool HasNonZeroDefaultValue(const FieldDescriptor* field) { return false; } - if (!field->has_default_value()) { - // No custom default set in the proto file. - return false; - } + // As much as checking field->has_default_value() seems useful, it isn't + // because of enums. proto2 syntax allows the first item in an enum (the + // default) to be non zero. So checking field->has_default_value() would + // result in missing this non zero default. See MessageWithOneBasedEnum in + // objectivec/Tests/unittest_objc.proto for a test Message to confirm this. // Some proto file set the default to the zero value, so make sure the value // isn't the zero case.