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

Both methods weren't checking the has_bits (where the bools are stored), so
it resulted in invalid results.

Add a test that should shake out something like this in the future also.
This commit is contained in:
Thomas Van Lenten 2016-04-27 13:11:16 -04:00
parent 66f074592d
commit 30646288ad
3 changed files with 114 additions and 5 deletions

View File

@ -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:

View File

@ -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

View File

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