Merge pull request #1454 from thomasvl/enum_defaults
Proper checking of enum with non zero default
diff --git a/objectivec/Tests/GPBMessageTests.m b/objectivec/Tests/GPBMessageTests.m
index 4354615..0779c5a 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 9483cb1..6bcfbf7 100644
--- a/objectivec/Tests/unittest_objc.proto
+++ b/objectivec/Tests/unittest_objc.proto
@@ -401,3 +401,13 @@
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 fda5180..196b39d 100644
--- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc
+++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc
@@ -757,10 +757,11 @@
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.
