Merge pull request #1454 from thomasvl/enum_defaults

Proper checking of enum with non zero default

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

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

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.