Merge pull request #1461 from thomasvl/fix_bool_handing

Fix up -hash/-isEqual: for bool storage.

objectivec/GPBMessage.m

@@ -2603,9 +2603,13 @@ static void MergeRepeatedNotPackedFieldFromCodedInputStream(
       size_t fieldOffset = field->description_->offset;
       switch (fieldDataType) {
         case GPBDataTypeBool: {
-          BOOL *selfValPtr = (BOOL *)&selfStorage[fieldOffset];
-          BOOL *otherValPtr = (BOOL *)&otherStorage[fieldOffset];
-          if (*selfValPtr != *otherValPtr) {
+          // Bools are stored in has_bits to avoid needing explicit space in
+          // the storage structure.
+          // (the field number passed to the HasIvar helper doesn't really
+          // matter since the offset is never negative)
+          BOOL selfValue = GPBGetHasIvar(self, (int32_t)(fieldOffset), 0);
+          BOOL otherValue = GPBGetHasIvar(other, (int32_t)(fieldOffset), 0);
+          if (selfValue != otherValue) {
             return NO;
           }
           break;
@@ -2714,8 +2718,12 @@ static void MergeRepeatedNotPackedFieldFromCodedInputStream(
       size_t fieldOffset = field->description_->offset;
       switch (fieldDataType) {
         case GPBDataTypeBool: {
-          BOOL *valPtr = (BOOL *)&storage[fieldOffset];
-          result = prime * result + *valPtr;
+          // Bools are stored in has_bits to avoid needing explicit space in
+          // the storage structure.
+          // (the field number passed to the HasIvar helper doesn't really
+          // matter since the offset is never negative)
+          BOOL value = GPBGetHasIvar(self, (int32_t)(fieldOffset), 0);
+          result = prime * result + value;
           break;
         }
         case GPBDataTypeSFixed32:

objectivec/Tests/GPBMessageTests.m

@@ -1954,4 +1954,69 @@
   XCTAssertEqual(enumMsg.enumField, MessageWithOneBasedEnum_OneBasedEnum_One);
 }
 
+- (void)testBoolOffsetUsage {
+  // Bools use storage within has_bits; this test ensures that this is honored
+  // in all places where things should crash or fail based on reading out of
+  // field storage instead.
+  BoolOnlyMessage *msg1 = [BoolOnlyMessage message];
+  BoolOnlyMessage *msg2 = [BoolOnlyMessage message];
+
+  msg1.boolField1 = YES;
+  msg2.boolField1 = YES;
+  msg1.boolField3 = YES;
+  msg2.boolField3 = YES;
+  msg1.boolField5 = YES;
+  msg2.boolField5 = YES;
+  msg1.boolField7 = YES;
+  msg2.boolField7 = YES;
+  msg1.boolField9 = YES;
+  msg2.boolField9 = YES;
+  msg1.boolField11 = YES;
+  msg2.boolField11 = YES;
+  msg1.boolField13 = YES;
+  msg2.boolField13 = YES;
+  msg1.boolField15 = YES;
+  msg2.boolField15 = YES;
+  msg1.boolField17 = YES;
+  msg2.boolField17 = YES;
+  msg1.boolField19 = YES;
+  msg2.boolField19 = YES;
+  msg1.boolField21 = YES;
+  msg2.boolField21 = YES;
+  msg1.boolField23 = YES;
+  msg2.boolField23 = YES;
+  msg1.boolField25 = YES;
+  msg2.boolField25 = YES;
+  msg1.boolField27 = YES;
+  msg2.boolField27 = YES;
+  msg1.boolField29 = YES;
+  msg2.boolField29 = YES;
+  msg1.boolField31 = YES;
+  msg2.boolField31 = YES;
+
+  msg1.boolField32 = YES;
+  msg2.boolField32 = YES;
+
+  XCTAssertTrue(msg1 != msg2); // Different pointers.
+  XCTAssertEqual([msg1 hash], [msg2 hash]);
+  XCTAssertEqualObjects(msg1, msg2);
+
+  BoolOnlyMessage *msg1Prime = [[msg1 copy] autorelease];
+  XCTAssertTrue(msg1Prime != msg1); // Different pointers.
+  XCTAssertEqual([msg1 hash], [msg1Prime hash]);
+  XCTAssertEqualObjects(msg1, msg1Prime);
+
+  // Field set in one, but not the other means they don't match (even if
+  // set to default value).
+  msg1Prime.boolField2 = NO;
+  XCTAssertNotEqualObjects(msg1Prime, msg1);
+  // And when set to different values.
+  msg1.boolField2 = YES;
+  XCTAssertNotEqualObjects(msg1Prime, msg1);
+  // And then they match again.
+  msg1.boolField2 = NO;
+  XCTAssertEqualObjects(msg1Prime, msg1);
+  XCTAssertEqual([msg1 hash], [msg1Prime hash]);
+}
+
 @end

objectivec/Tests/unittest_objc.proto

@@ -411,3 +411,39 @@ message MessageWithOneBasedEnum {
   }
   optional OneBasedEnum enum_field = 1;
 }
+
+// Message with all bools for testing things related to bool storage.
+message BoolOnlyMessage {
+  optional bool bool_field_1 = 1;
+  optional bool bool_field_2 = 2;
+  optional bool bool_field_3 = 3;
+  optional bool bool_field_4 = 4;
+  optional bool bool_field_5 = 5;
+  optional bool bool_field_6 = 6;
+  optional bool bool_field_7 = 7;
+  optional bool bool_field_8 = 8;
+  optional bool bool_field_9 = 9;
+  optional bool bool_field_10 = 10;
+  optional bool bool_field_11 = 11;
+  optional bool bool_field_12 = 12;
+  optional bool bool_field_13 = 13;
+  optional bool bool_field_14 = 14;
+  optional bool bool_field_15 = 15;
+  optional bool bool_field_16 = 16;
+  optional bool bool_field_17 = 17;
+  optional bool bool_field_18 = 18;
+  optional bool bool_field_19 = 19;
+  optional bool bool_field_20 = 20;
+  optional bool bool_field_21 = 21;
+  optional bool bool_field_22 = 22;
+  optional bool bool_field_23 = 23;
+  optional bool bool_field_24 = 24;
+  optional bool bool_field_25 = 25;
+  optional bool bool_field_26 = 26;
+  optional bool bool_field_27 = 27;
+  optional bool bool_field_28 = 28;
+  optional bool bool_field_29 = 29;
+  optional bool bool_field_30 = 30;
+  optional bool bool_field_31 = 31;
+  optional bool bool_field_32 = 32;
+}