Merge pull request #17840 from thomasvl/patch_objc_to_28

CherryPick the lastest work for Objective-C over the the 28 branch

objectivec/GPBUnknownField.m

@@ -165,9 +165,17 @@ - (id)copyWithZone:(NSZone *)zone {
     case GPBUnknownFieldTypeFixed32:
     case GPBUnknownFieldTypeFixed64:
     case GPBUnknownFieldTypeLengthDelimited:
-    case GPBUnknownFieldTypeGroup:
       // In these modes, the object isn't mutable, so just return self.
       return [self retain];
+    case GPBUnknownFieldTypeGroup: {
+      // The `GPBUnknownFields` for the group is mutable, so a new instance of this object and
+      // the group is needed.
+      GPBUnknownFields *copyGroup = [storage_.group copyWithZone:zone];
+      GPBUnknownField *copy = [[GPBUnknownField allocWithZone:zone] initWithNumber:number_
+                                                                             group:copyGroup];
+      [copyGroup release];
+      return copy;
+    }
     case GPBUnknownFieldTypeLegacy: {
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"

objectivec/GPBUnknownFields.h

@@ -109,6 +109,23 @@ __attribute__((objc_subclassing_restricted))
  **/
 - (nonnull GPBUnknownFields *)addGroupWithFieldNumber:(int32_t)fieldNumber;
 
+/**
+ * Add the copy of the given unknown field.
+ *
+ * This can be useful from processing one `GPBUnknownFields` to create another.
+ *
+ * NOTE: If the field being copied is an Group, this instance added is new and thus
+ * the `.group` of that result is also new, so if you intent is to modify the group
+ * it *must* be fetched out of the result.
+ *
+ * It is a programming error to call this when the `type` is a legacy field.
+ *
+ * @param field The field to add.
+ *
+ * @return The autoreleased field that was added.
+ **/
+- (GPBUnknownField *)addCopyOfField:(nonnull GPBUnknownField *)field;
+
 /**
  * Removes the given field from the set.
  *

objectivec/GPBUnknownFields.m

@@ -197,6 +197,12 @@ - (instancetype)initFromMessage:(nonnull GPBMessage *)message {
   self = [super init];
   if (self) {
     fields_ = [[NSMutableArray alloc] init];
+    // This shouldn't happen with the annotations, but just incase something claiming nonnull
+    // does return nil, block it.
+    if (!message) {
+      [self release];
+      [NSException raise:NSInvalidArgumentException format:@"Message cannot be nil"];
+    }
     NSData *data = GPBMessageUnknownFieldsData(message);
     if (data) {
       GPBCodedInputStream *input = [[GPBCodedInputStream alloc] initWithData:data];
@@ -222,9 +228,8 @@ - (instancetype)init {
 }
 
 - (id)copyWithZone:(NSZone *)zone {
-  GPBUnknownFields *copy = [[GPBUnknownFields alloc] init];
-  // Fields are r/o in this api, so just copy the array.
-  copy->fields_ = [fields_ mutableCopyWithZone:zone];
+  GPBUnknownFields *copy = [[GPBUnknownFields allocWithZone:zone] init];
+  copy->fields_ = [[NSMutableArray allocWithZone:zone] initWithArray:fields_ copyItems:YES];
   return copy;
 }
 
@@ -318,6 +323,16 @@ - (GPBUnknownFields *)addGroupWithFieldNumber:(int32_t)fieldNumber {
   return [group autorelease];
 }
 
+- (GPBUnknownField *)addCopyOfField:(nonnull GPBUnknownField *)field {
+  if (field->type_ == GPBUnknownFieldTypeLegacy) {
+    [NSException raise:NSInternalInconsistencyException
+                format:@"GPBUnknownField is the wrong type"];
+  }
+  GPBUnknownField *result = [field copy];
+  [fields_ addObject:result];
+  return [result autorelease];
+}
+
 - (void)removeField:(nonnull GPBUnknownField *)field {
   NSUInteger count = fields_.count;
   [fields_ removeObjectIdenticalTo:field];

objectivec/Tests/GPBUnknownFieldsTest.m

@@ -662,6 +662,37 @@ - (void)testFastEnumeration {
   XCTAssertEqual(loop, 10);
 }
 
+- (void)testAddCopyOfField {
+  GPBUnknownFields* ufs = [[[GPBUnknownFields alloc] init] autorelease];
+  [ufs addFieldNumber:1 varint:10];
+  [ufs addFieldNumber:2 fixed32:11];
+  [ufs addFieldNumber:3 fixed64:12];
+  [ufs addFieldNumber:4 lengthDelimited:DataFromCStr("foo")];
+  GPBUnknownFields* group = [ufs addGroupWithFieldNumber:5];
+  [group addFieldNumber:10 varint:100];
+  GPBUnknownFields* subGroup = [group addGroupWithFieldNumber:100];
+  [subGroup addFieldNumber:50 varint:50];
+
+  GPBUnknownFields* ufs2 = [[[GPBUnknownFields alloc] init] autorelease];
+  for (GPBUnknownField* field in ufs) {
+    GPBUnknownField* field2 = [ufs2 addCopyOfField:field];
+    XCTAssertEqualObjects(field, field2);
+    if (field.type == GPBUnknownFieldTypeGroup) {
+      // Group does a copy because the `.group` value is mutable.
+      XCTAssertTrue(field != field2);        // Pointer comparison.
+      XCTAssertTrue(group != field2.group);  // Pointer comparison.
+      XCTAssertEqualObjects(group, field2.group);
+      GPBUnknownFields* subGroupAdded = [field2.group firstGroup:100];
+      XCTAssertTrue(subGroupAdded != subGroup);  // Pointer comparison.
+      XCTAssertEqualObjects(subGroupAdded, subGroup);
+    } else {
+      // All other types are immutable, so they use the same object.
+      XCTAssertTrue(field == field2);  // Pointer comparision.
+    }
+  }
+  XCTAssertEqualObjects(ufs, ufs2);
+}
+
 - (void)testDescriptions {
   // Exercise description for completeness.
   GPBUnknownFields* ufs = [[[GPBUnknownFields alloc] init] autorelease];
@@ -683,25 +714,30 @@ - (void)testCopy {
   [ufs addFieldNumber:3 fixed64:3];
   [ufs addFieldNumber:4 lengthDelimited:DataFromCStr("foo")];
   GPBUnknownFields* group = [ufs addGroupWithFieldNumber:5];
+  [group addFieldNumber:10 varint:10];
+  GPBUnknownFields* subGroup = [group addGroupWithFieldNumber:100];
+  [subGroup addFieldNumber:20 varint:20];
 
   GPBUnknownFields* ufs2 = [[ufs copy] autorelease];
   XCTAssertTrue(ufs != ufs2);        // Different objects
   XCTAssertEqualObjects(ufs, ufs2);  // Equal contents
-  // All the actual field objects should be the same since they are immutable.
+  // All field objects but the group should be the same since they are immutable.
   XCTAssertTrue([[ufs fields:1] firstObject] == [[ufs2 fields:1] firstObject]);  // Same object
   XCTAssertTrue([[ufs fields:2] firstObject] == [[ufs2 fields:2] firstObject]);  // Same object
   XCTAssertTrue([[ufs fields:3] firstObject] == [[ufs2 fields:3] firstObject]);  // Same object
   XCTAssertTrue([[ufs fields:4] firstObject] == [[ufs2 fields:4] firstObject]);  // Same object
   XCTAssertTrue([[ufs fields:4] firstObject].lengthDelimited ==
-                [[ufs2 fields:4] firstObject].lengthDelimited);                  // Same object
-  XCTAssertTrue([[ufs fields:5] firstObject] == [[ufs2 fields:5] firstObject]);  // Same object
-  XCTAssertTrue(group == [[ufs2 fields:5] firstObject].group);                   // Same object
-
-  // Now force copies on the fields to confirm that is not making new objects either.
-  for (GPBUnknownField* field in ufs) {
-    GPBUnknownField* field2 = [[field copy] autorelease];
-    XCTAssertTrue(field == field2);  // Same object (since they aren't mutable).
-  }
+                [[ufs2 fields:4] firstObject].lengthDelimited);  // Same object
+  // Since the group holds another `GPBUnknownFields` object (which is mutable), it will be a
+  // different object.
+  XCTAssertTrue([[ufs fields:5] firstObject] != [[ufs2 fields:5] firstObject]);
+  XCTAssertTrue(group != [[ufs2 fields:5] firstObject].group);
+  XCTAssertEqualObjects(group, [[ufs2 fields:5] firstObject].group);
+  // And confirm that copy went deep so the nested group also is a different object.
+  GPBUnknownFields* groupCopied = [[ufs2 fields:5] firstObject].group;
+  XCTAssertTrue([[group fields:100] firstObject] != [[groupCopied fields:100] firstObject]);
+  XCTAssertTrue(subGroup != [[groupCopied fields:100] firstObject].group);
+  XCTAssertEqualObjects(subGroup, [[groupCopied fields:100] firstObject].group);
 }
 
 - (void)testInvalidFieldNumbers {