From 18b6a321b5fd8f53e1029c436c2ad5dd2d3122df Mon Sep 17 00:00:00 2001
From: Thomas Van Lenten <thomasvl@google.com>
Date: Tue, 26 Apr 2016 14:40:11 -0400
Subject: [PATCH] Proper checking of enum with non zero default

proto2 syntax allows the first enum to have a non zero value. This means any
field using that default has a non zero default without having an explicit
default being set. So when deciding what runtime info is needed, don't rely
on an explicit default, always check that the values aren't zero.

Fixes https://github.com/google/protobuf/issues/1453
---
 objectivec/Tests/GPBMessageTests.m                     |  7 +++++++
 objectivec/Tests/unittest_objc.proto                   | 10 ++++++++++
 .../protobuf/compiler/objectivec/objectivec_helpers.cc |  9 +++++----
 3 files changed, 22 insertions(+), 4 deletions(-)

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.