diff --git a/objectivec/GPBMessage.m b/objectivec/GPBMessage.m index db5d3b60cdc43..c86641171353f 100644 --- a/objectivec/GPBMessage.m +++ b/objectivec/GPBMessage.m @@ -3084,6 +3084,14 @@ static void ResolveIvarSet(__unsafe_unretained GPBFieldDescriptor *field, result->encodingSelector = @selector(set##NAME:); \ break; \ } +#define CASE_SET_COPY(NAME) \ + case GPBDataType##NAME: { \ + result->impToAdd = imp_implementationWithBlock(^(id obj, id value) { \ + return GPBSetRetainedObjectIvarWithFieldInternal(obj, field, [value copy], syntax); \ + }); \ + result->encodingSelector = @selector(set##NAME:); \ + break; \ + } CASE_SET(Bool, BOOL, Bool) CASE_SET(Fixed32, uint32_t, UInt32) CASE_SET(SFixed32, int32_t, Int32) @@ -3097,8 +3105,8 @@ static void ResolveIvarSet(__unsafe_unretained GPBFieldDescriptor *field, CASE_SET(SInt64, int64_t, Int64) CASE_SET(UInt32, uint32_t, UInt32) CASE_SET(UInt64, uint64_t, UInt64) - CASE_SET(Bytes, id, Object) - CASE_SET(String, id, Object) + CASE_SET_COPY(Bytes) + CASE_SET_COPY(String) CASE_SET(Message, id, Object) CASE_SET(Group, id, Object) CASE_SET(Enum, int32_t, Enum) @@ -3178,7 +3186,7 @@ + (BOOL)resolveInstanceMethod:(SEL)sel { // full lookup. const GPBFileSyntax syntax = descriptor.file.syntax; result.impToAdd = imp_implementationWithBlock(^(id obj, id value) { - return GPBSetObjectIvarWithFieldInternal(obj, field, value, syntax); + GPBSetObjectIvarWithFieldInternal(obj, field, value, syntax); }); result.encodingSelector = @selector(setArray:); break; diff --git a/objectivec/GPBUtilities.m b/objectivec/GPBUtilities.m index e2a12ca4d36e6..1aedb6ccbe76e 100644 --- a/objectivec/GPBUtilities.m +++ b/objectivec/GPBUtilities.m @@ -448,6 +448,36 @@ void GPBMaybeClearOneof(GPBMessage *self, GPBOneofDescriptor *oneof, //% GPBSetObjectIvarWithField(self, field, (id)value); //%} //% +//%PDDM-DEFINE IVAR_ALIAS_DEFN_COPY_OBJECT(NAME, TYPE) +//%// Only exists for public api, no core code should use this. +//%TYPE *GPBGetMessage##NAME##Field(GPBMessage *self, +//% TYPE$S NAME$S GPBFieldDescriptor *field) { +//%#if defined(DEBUG) && DEBUG +//% NSCAssert(DataTypesEquivalent(GPBGetFieldDataType(field), +//% GPBDataType##NAME), +//% @"Attempting to get value of TYPE from field %@ " +//% @"of %@ which is of type %@.", +//% [self class], field.name, +//% TypeToString(GPBGetFieldDataType(field))); +//%#endif +//% return (TYPE *)GPBGetObjectIvarWithField(self, field); +//%} +//% +//%// Only exists for public api, no core code should use this. +//%void GPBSetMessage##NAME##Field(GPBMessage *self, +//% NAME$S GPBFieldDescriptor *field, +//% NAME$S TYPE *value) { +//%#if defined(DEBUG) && DEBUG +//% NSCAssert(DataTypesEquivalent(GPBGetFieldDataType(field), +//% GPBDataType##NAME), +//% @"Attempting to set field %@ of %@ which is of type %@ with " +//% @"value of type TYPE.", +//% [self class], field.name, +//% TypeToString(GPBGetFieldDataType(field))); +//%#endif +//% GPBSetCopyObjectIvarWithField(self, field, (id)value); +//%} +//% // Object types are handled slightly differently, they need to be released // and retained. @@ -483,6 +513,24 @@ static void GPBSetObjectIvarWithField(GPBMessage *self, syntax); } +static void GPBSetCopyObjectIvarWithField(GPBMessage *self, + GPBFieldDescriptor *field, id value); + +// GPBSetCopyObjectIvarWithField is blocked from the analyzer because it flags +// a leak for the -copy even though GPBSetRetainedObjectIvarWithFieldInternal +// is marked as consuming the value. Note: For some reason this doesn't happen +// with the -retain in GPBSetObjectIvarWithField. +#if !defined(__clang_analyzer__) +// This exists only for briging some aliased types, nothing else should use it. +static void GPBSetCopyObjectIvarWithField(GPBMessage *self, + GPBFieldDescriptor *field, id value) { + if (self == nil || field == nil) return; + GPBFileSyntax syntax = [self descriptor].file.syntax; + GPBSetRetainedObjectIvarWithFieldInternal(self, field, [value copy], + syntax); +} +#endif // !defined(__clang_analyzer__) + void GPBSetObjectIvarWithFieldInternal(GPBMessage *self, GPBFieldDescriptor *field, id value, GPBFileSyntax syntax) { @@ -1168,7 +1216,7 @@ void GPBSetDoubleIvarWithFieldInternal(GPBMessage *self, // Aliases are function calls that are virtually the same. -//%PDDM-EXPAND IVAR_ALIAS_DEFN_OBJECT(String, NSString) +//%PDDM-EXPAND IVAR_ALIAS_DEFN_COPY_OBJECT(String, NSString) // This block of code is generated, do not edit it directly. // Only exists for public api, no core code should use this. @@ -1197,10 +1245,10 @@ void GPBSetMessageStringField(GPBMessage *self, [self class], field.name, TypeToString(GPBGetFieldDataType(field))); #endif - GPBSetObjectIvarWithField(self, field, (id)value); + GPBSetCopyObjectIvarWithField(self, field, (id)value); } -//%PDDM-EXPAND IVAR_ALIAS_DEFN_OBJECT(Bytes, NSData) +//%PDDM-EXPAND IVAR_ALIAS_DEFN_COPY_OBJECT(Bytes, NSData) // This block of code is generated, do not edit it directly. // Only exists for public api, no core code should use this. @@ -1229,7 +1277,7 @@ void GPBSetMessageBytesField(GPBMessage *self, [self class], field.name, TypeToString(GPBGetFieldDataType(field))); #endif - GPBSetObjectIvarWithField(self, field, (id)value); + GPBSetCopyObjectIvarWithField(self, field, (id)value); } //%PDDM-EXPAND IVAR_ALIAS_DEFN_OBJECT(Message, GPBMessage) diff --git a/objectivec/ProtocolBuffers_OSX.xcodeproj/project.pbxproj b/objectivec/ProtocolBuffers_OSX.xcodeproj/project.pbxproj index a41be9f2804a2..faf562ba45493 100644 --- a/objectivec/ProtocolBuffers_OSX.xcodeproj/project.pbxproj +++ b/objectivec/ProtocolBuffers_OSX.xcodeproj/project.pbxproj @@ -891,7 +891,7 @@ C01FCF4F08A954540054247B /* Debug */ = { isa = XCBuildConfiguration; buildSettings = { - ALWAYS_SEARCH_USER_PATHS = YES; + ALWAYS_SEARCH_USER_PATHS = NO; CLANG_ANALYZER_NONNULL = YES; CLANG_ANALYZER_SECURITY_FLOATLOOPCOUNTER = YES; CLANG_ANALYZER_SECURITY_INSECUREAPI_RAND = YES; @@ -961,7 +961,7 @@ C01FCF5008A954540054247B /* Release */ = { isa = XCBuildConfiguration; buildSettings = { - ALWAYS_SEARCH_USER_PATHS = YES; + ALWAYS_SEARCH_USER_PATHS = NO; CLANG_ANALYZER_NONNULL = YES; CLANG_ANALYZER_SECURITY_FLOATLOOPCOUNTER = YES; CLANG_ANALYZER_SECURITY_INSECUREAPI_RAND = YES; diff --git a/objectivec/ProtocolBuffers_iOS.xcodeproj/project.pbxproj b/objectivec/ProtocolBuffers_iOS.xcodeproj/project.pbxproj index 470652d07b9a2..3847bcc80d1a3 100644 --- a/objectivec/ProtocolBuffers_iOS.xcodeproj/project.pbxproj +++ b/objectivec/ProtocolBuffers_iOS.xcodeproj/project.pbxproj @@ -914,7 +914,7 @@ C01FCF4F08A954540054247B /* Debug */ = { isa = XCBuildConfiguration; buildSettings = { - ALWAYS_SEARCH_USER_PATHS = YES; + ALWAYS_SEARCH_USER_PATHS = NO; CLANG_ANALYZER_NONNULL = YES; CLANG_ANALYZER_SECURITY_FLOATLOOPCOUNTER = YES; CLANG_ANALYZER_SECURITY_INSECUREAPI_RAND = YES; @@ -985,7 +985,7 @@ C01FCF5008A954540054247B /* Release */ = { isa = XCBuildConfiguration; buildSettings = { - ALWAYS_SEARCH_USER_PATHS = YES; + ALWAYS_SEARCH_USER_PATHS = NO; CLANG_ANALYZER_NONNULL = YES; CLANG_ANALYZER_SECURITY_FLOATLOOPCOUNTER = YES; CLANG_ANALYZER_SECURITY_INSECUREAPI_RAND = YES; diff --git a/objectivec/Tests/GPBMessageTests+Runtime.m b/objectivec/Tests/GPBMessageTests+Runtime.m index 0058311b3bf09..b0d58c9c559e5 100644 --- a/objectivec/Tests/GPBMessageTests+Runtime.m +++ b/objectivec/Tests/GPBMessageTests+Runtime.m @@ -2491,6 +2491,72 @@ - (void)test_GPBSetMessageMapField { XCTAssertEqualObjects(@"bar", message.mapStringString[@"foo"]); } +- (void)test_StringFieldsCopy { + // ObjC conventions call for NSString properites to be copy, ensure + // that is done correctly and the string isn't simply retained. + + Message2 *msg1 = [Message2 message]; + Message2 *msg2 = [Message2 message]; + + GPBFieldDescriptor *fieldDesc = + [[Message2 descriptor] fieldWithNumber:Message2_FieldNumber_OptionalString]; + NSMutableString *mutableStr = [NSMutableString stringWithString:@"foo"]; + + msg1.optionalString = mutableStr; + GPBSetMessageStringField(msg2, fieldDesc, mutableStr); + + XCTAssertEqualObjects(msg1.optionalString, mutableStr); + XCTAssertEqualObjects(msg1.optionalString, @"foo"); + XCTAssertTrue(msg1.optionalString != mutableStr); // Ptr comparision. + + XCTAssertEqualObjects(msg2.optionalString, mutableStr); + XCTAssertEqualObjects(msg2.optionalString, @"foo"); + XCTAssertTrue(msg2.optionalString != mutableStr); // Ptr comparision. + + [mutableStr appendString:@"bar"]; + + XCTAssertNotEqualObjects(msg1.optionalString, mutableStr); + XCTAssertEqualObjects(msg1.optionalString, @"foo"); + XCTAssertTrue(msg1.optionalString != mutableStr); // Ptr comparision. + + XCTAssertNotEqualObjects(msg2.optionalString, mutableStr); + XCTAssertEqualObjects(msg2.optionalString, @"foo"); + XCTAssertTrue(msg2.optionalString != mutableStr); // Ptr comparision. +} + +- (void)test_BytesFieldsCopy { + // ObjC conventions call for NSData properites to be copy, ensure + // that is done correctly and the data isn't simply retained. + + Message2 *msg1 = [Message2 message]; + Message2 *msg2 = [Message2 message]; + + GPBFieldDescriptor *fieldDesc = + [[Message2 descriptor] fieldWithNumber:Message2_FieldNumber_OptionalBytes]; + NSMutableData *mutableData = [NSMutableData dataWithData:DataFromCStr("abc")]; + + msg1.optionalBytes = mutableData; + GPBSetMessageBytesField(msg2, fieldDesc, mutableData); + + XCTAssertEqualObjects(msg1.optionalBytes, mutableData); + XCTAssertEqualObjects(msg1.optionalBytes, DataFromCStr("abc")); + XCTAssertTrue(msg1.optionalBytes != mutableData); // Ptr comparision. + + XCTAssertEqualObjects(msg2.optionalBytes, mutableData); + XCTAssertEqualObjects(msg2.optionalBytes, DataFromCStr("abc")); + XCTAssertTrue(msg2.optionalBytes != mutableData); // Ptr comparision. + + [mutableData appendData:DataFromCStr("123")]; + + XCTAssertNotEqualObjects(msg1.optionalBytes, mutableData); + XCTAssertEqualObjects(msg1.optionalBytes, DataFromCStr("abc")); + XCTAssertTrue(msg1.optionalBytes != mutableData); // Ptr comparision. + + XCTAssertNotEqualObjects(msg2.optionalBytes, mutableData); + XCTAssertEqualObjects(msg2.optionalBytes, DataFromCStr("abc")); + XCTAssertTrue(msg2.optionalBytes != mutableData); // Ptr comparision. +} + #pragma mark - Subset from from map_tests.cc // TEST(GeneratedMapFieldTest, IsInitialized)