Tests and fixes for double-enumeration on AddRange and adding of null to PopsicleList

src/AddressBook/AddressBookProtos.cs

@@ -593,7 +593,7 @@ namespace Google.ProtocolBuffers.Examples.AddressBook {
           Email = other.Email;
         }
         if (other.phone_.Count != 0) {
-          base.AddRange(other.phone_, result.phone_);
+          result.phone_.Add(other.phone_);
         }
         this.MergeUnknownFields(other.UnknownFields);
         return this;
@@ -749,7 +749,7 @@ namespace Google.ProtocolBuffers.Examples.AddressBook {
         return this;
       }
       public Builder AddRangePhone(scg::IEnumerable<global::Google.ProtocolBuffers.Examples.AddressBook.Person.Types.PhoneNumber> values) {
-        base.AddRange(values, result.phone_);
+        result.phone_.Add(values);
         return this;
       }
       public Builder ClearPhone() {
@@ -926,7 +926,7 @@ namespace Google.ProtocolBuffers.Examples.AddressBook {
       public override Builder MergeFrom(AddressBook other) {
         if (other == global::Google.ProtocolBuffers.Examples.AddressBook.AddressBook.DefaultInstance) return this;
         if (other.person_.Count != 0) {
-          base.AddRange(other.person_, result.person_);
+          result.person_.Add(other.person_);
         }
         this.MergeUnknownFields(other.UnknownFields);
         return this;
@@ -1014,7 +1014,7 @@ namespace Google.ProtocolBuffers.Examples.AddressBook {
         return this;
       }
       public Builder AddRangePerson(scg::IEnumerable<global::Google.ProtocolBuffers.Examples.AddressBook.Person> values) {
-        base.AddRange(values, result.person_);
+        result.person_.Add(values);
         return this;
       }
       public Builder ClearPerson() {

src/ProtoGen/RepeatedEnumFieldGenerator.cs

@@ -88,7 +88,7 @@ namespace Google.ProtocolBuffers.ProtoGen
             writer.WriteLine("  return this;");
             writer.WriteLine("}");
             writer.WriteLine("public Builder AddRange{0}(scg::IEnumerable<{1}> values) {{", PropertyName, TypeName);
-            writer.WriteLine("  base.AddRange(values, result.{0}_);", Name);
+            writer.WriteLine("  result.{0}_.Add(values);", Name);
             writer.WriteLine("  return this;");
             writer.WriteLine("}");
             writer.WriteLine("public Builder Clear{0}() {{", PropertyName);
@@ -100,7 +100,7 @@ namespace Google.ProtocolBuffers.ProtoGen
         public void GenerateMergingCode(TextGenerator writer)
         {
             writer.WriteLine("if (other.{0}_.Count != 0) {{", Name);
-            writer.WriteLine("  base.AddRange(other.{0}_, result.{0}_);", Name);
+            writer.WriteLine("  result.{0}_.Add(other.{0}_);", Name);
             writer.WriteLine("}");
         }
 

src/ProtoGen/RepeatedMessageFieldGenerator.cs

@@ -98,7 +98,7 @@ namespace Google.ProtocolBuffers.ProtoGen
             writer.WriteLine("  return this;");
             writer.WriteLine("}");
             writer.WriteLine("public Builder AddRange{0}(scg::IEnumerable<{1}> values) {{", PropertyName, TypeName);
-            writer.WriteLine("  base.AddRange(values, result.{0}_);", Name);
+            writer.WriteLine("  result.{0}_.Add(values);", Name);
             writer.WriteLine("  return this;");
             writer.WriteLine("}");
             writer.WriteLine("public Builder Clear{0}() {{", PropertyName);
@@ -110,7 +110,7 @@ namespace Google.ProtocolBuffers.ProtoGen
         public void GenerateMergingCode(TextGenerator writer)
         {
             writer.WriteLine("if (other.{0}_.Count != 0) {{", Name);
-            writer.WriteLine("  base.AddRange(other.{0}_, result.{0}_);", Name);
+            writer.WriteLine("  result.{0}_.Add(other.{0}_);", Name);
             writer.WriteLine("}");
         }
 

src/ProtoGen/RepeatedPrimitiveFieldGenerator.cs

@@ -97,7 +97,7 @@ namespace Google.ProtocolBuffers.ProtoGen
             writer.WriteLine("}");
             AddClsComplianceCheck(writer);
             writer.WriteLine("public Builder AddRange{0}(scg::IEnumerable<{1}> values) {{", PropertyName, TypeName);
-            writer.WriteLine("  base.AddRange(values, result.{0}_);", Name);
+            writer.WriteLine("  result.{0}_.Add(values);", Name);
             writer.WriteLine("  return this;");
             writer.WriteLine("}");
             writer.WriteLine("public Builder Clear{0}() {{", PropertyName);
@@ -109,7 +109,7 @@ namespace Google.ProtocolBuffers.ProtoGen
         public void GenerateMergingCode(TextGenerator writer)
         {
             writer.WriteLine("if (other.{0}_.Count != 0) {{", Name);
-            writer.WriteLine("  base.AddRange(other.{0}_, result.{0}_);", Name);
+            writer.WriteLine("  result.{0}_.Add(other.{0}_);", Name);
             writer.WriteLine("}");
         }
 

src/ProtocolBuffers.Test/Collections/PopsicleListTest.cs

@@ -35,6 +35,7 @@
 #endregion
 
 using System;
+using System.Collections.Generic;
 using NUnit.Framework;
 
 internal delegate void Action();
@@ -94,6 +95,64 @@ namespace Google.ProtocolBuffers.Collections
             Assert.IsFalse(list.IsReadOnly);
         }
 
+        [Test]
+        public void DoesNotAddNullEnumerable()
+        {
+            PopsicleList<string> list = new PopsicleList<string>();
+            try
+            {
+                list.Add((IEnumerable<string>)null);
+            }
+            catch (ArgumentNullException)
+            { return; }
+
+            Assert.Fail("List should not allow nulls.");
+        }
+
+        [Test]
+        public void DoesNotAddRangeWithNull()
+        {
+            PopsicleList<string> list = new PopsicleList<string>();
+            try
+            {
+                list.Add(new[] { "a", "b", null });
+            }
+            catch (ArgumentNullException)
+            { return; }
+
+            Assert.Fail("List should not allow nulls.");
+        }
+
+        [Test]
+        public void DoesNotAddNull()
+        {
+            PopsicleList<string> list = new PopsicleList<string>();
+            try
+            {
+                list.Add((string)null);
+            }
+            catch (ArgumentNullException)
+            { return; }
+
+            Assert.Fail("List should not allow nulls.");
+        }
+
+        [Test]
+        public void DoesNotSetNull()
+        {
+            PopsicleList<string> list = new PopsicleList<string>();
+            list.Add("a");
+            try
+            {
+                list[0] = null;
+            }
+            catch (ArgumentNullException)
+            { return; }
+
+            Assert.Fail("List should not allow nulls.");
+        }
+
+
         private static void AssertNotSupported(Action action)
         {
             try

src/ProtocolBuffers.Test/GeneratedBuilderTest.cs

+using System;
+using System.Collections.Generic;
+using System.Text;
+using Google.ProtocolBuffers.TestProtos;
+using NUnit.Framework;
+
+namespace Google.ProtocolBuffers
+{
+    [TestFixture]
+    public class GeneratedBuilderTest
+    {
+        class OneTimeEnumerator<T> : IEnumerable<T>
+        {
+            readonly T _item;
+            bool _enumerated;
+            public OneTimeEnumerator(T item)
+            {
+                _item = item;
+            }
+            public IEnumerator<T> GetEnumerator()
+            {
+                Assert.IsFalse(_enumerated, "The collection {0} has already been enumerated", GetType());
+                _enumerated = true;
+                yield return _item;
+            }
+            System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
+            { return GetEnumerator(); }
+        }
+
+        [Test]
+        public void DoesNotEnumerateTwiceForMessageList()
+        {
+            TestAllTypes.Builder b = new TestAllTypes.Builder();
+            b.AddRangeRepeatedForeignMessage(
+                new OneTimeEnumerator<ForeignMessage>(
+                    ForeignMessage.DefaultInstance));
+        }
+        [Test]
+        public void DoesNotEnumerateTwiceForPrimitiveList()
+        {
+            TestAllTypes.Builder b = new TestAllTypes.Builder();
+            b.AddRangeRepeatedInt32(new OneTimeEnumerator<int>(1));
+        }
+        [Test]
+        public void DoesNotEnumerateTwiceForStringList()
+        {
+            TestAllTypes.Builder b = new TestAllTypes.Builder();
+            b.AddRangeRepeatedString(new OneTimeEnumerator<string>("test"));
+        }
+        [Test]
+        public void DoesNotEnumerateTwiceForEnumList()
+        {
+            TestAllTypes.Builder b = new TestAllTypes.Builder();
+            b.AddRangeRepeatedForeignEnum(new OneTimeEnumerator<ForeignEnum>(ForeignEnum.FOREIGN_BAR));
+        }
+
+        private static void AssertThrows<T>(System.Threading.ThreadStart method) where T : Exception
+        {
+            try
+            {
+                method();
+            }
+            catch (Exception error)
+            {
+                if (error is T)
+                    return;
+                throw;
+            }
+            Assert.Fail("Expected exception of type " + typeof(T));
+        }
+
+        [Test]
+        public void DoesNotAddNullToMessageListByAddRange()
+        {
+            TestAllTypes.Builder b = new TestAllTypes.Builder();
+            AssertThrows<ArgumentNullException>(
+                () => b.AddRangeRepeatedForeignMessage(new ForeignMessage[] { null })
+                    );
+        }
+        [Test]
+        public void DoesNotAddNullToMessageListByAdd()
+        {
+            TestAllTypes.Builder b = new TestAllTypes.Builder();
+            AssertThrows<ArgumentNullException>(
+                () => b.AddRepeatedForeignMessage((ForeignMessage)null)
+                    );
+        }
+        [Test]
+        public void DoesNotAddNullToMessageListBySet()
+        {
+            TestAllTypes.Builder b = new TestAllTypes.Builder();
+            b.AddRepeatedForeignMessage(ForeignMessage.DefaultInstance);
+            AssertThrows<ArgumentNullException>(
+                () => b.SetRepeatedForeignMessage(0, (ForeignMessage)null)
+                    );
+        }
+        [Test]
+        public void DoesNotAddNullToStringListByAddRange()
+        {
+            TestAllTypes.Builder b = new TestAllTypes.Builder();
+            AssertThrows<ArgumentNullException>(
+                () => b.AddRangeRepeatedString(new String[] { null })
+                    );
+        }
+        [Test]
+        public void DoesNotAddNullToStringListByAdd()
+        {
+            TestAllTypes.Builder b = new TestAllTypes.Builder();
+            AssertThrows<ArgumentNullException>(
+                () => b.AddRepeatedString(null)
+                    );
+        }
+        [Test]
+        public void DoesNotAddNullToStringListBySet()
+        {
+            TestAllTypes.Builder b = new TestAllTypes.Builder();
+            b.AddRepeatedString("one");
+            AssertThrows<ArgumentNullException>(
+                () => b.SetRepeatedString(0, null)
+                    );
+        }
+    }
+}

src/ProtocolBuffers.Test/ProtocolBuffers.Test.csproj

@@ -97,6 +97,7 @@
     <Compile Include="Descriptors\MessageDescriptorTest.cs" />
     <Compile Include="DynamicMessageTest.cs" />
     <Compile Include="ExtendableMessageTest.cs" />
+    <Compile Include="GeneratedBuilderTest.cs" />
     <Compile Include="GeneratedMessageTest.cs" />
     <Compile Include="MessageStreamIteratorTest.cs" />
     <Compile Include="MessageStreamWriterTest.cs" />

src/ProtocolBuffers.Test/TestProtos/UnitTestEmbedOptimizeForProtoFile.cs

@@ -246,7 +246,7 @@ namespace Google.ProtocolBuffers.TestProtos {
           MergeOptionalMessage(other.OptionalMessage);
         }
         if (other.repeatedMessage_.Count != 0) {
-          base.AddRange(other.repeatedMessage_, result.repeatedMessage_);
+          result.repeatedMessage_.Add(other.repeatedMessage_);
         }
         this.MergeUnknownFields(other.UnknownFields);
         return this;
@@ -379,7 +379,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeRepeatedMessage(scg::IEnumerable<global::Google.ProtocolBuffers.TestProtos.TestOptimizedForSize> values) {
-        base.AddRange(values, result.repeatedMessage_);
+        result.repeatedMessage_.Add(values);
         return this;
       }
       public Builder ClearRepeatedMessage() {

src/ProtocolBuffers.Test/TestProtos/UnitTestGoogleSizeProtoFile.cs

@@ -877,7 +877,7 @@ namespace Google.ProtocolBuffers.TestProtos {
       }
       [global::System.CLSCompliant(false)]
       public Builder AddRangeField5(scg::IEnumerable<ulong> values) {
-        base.AddRange(values, result.field5_);
+        result.field5_.Add(values);
         return this;
       }
       public Builder ClearField5() {
@@ -2515,7 +2515,7 @@ namespace Google.ProtocolBuffers.TestProtos {
             return this;
           }
           public Builder AddRangeField14(scg::IEnumerable<string> values) {
-            base.AddRange(values, result.field14_);
+            result.field14_.Add(values);
             return this;
           }
           public Builder ClearField14() {
@@ -2656,7 +2656,7 @@ namespace Google.ProtocolBuffers.TestProtos {
             return this;
           }
           public Builder AddRangeField22(scg::IEnumerable<string> values) {
-            base.AddRange(values, result.field22_);
+            result.field22_.Add(values);
             return this;
           }
           public Builder ClearField22() {
@@ -2682,7 +2682,7 @@ namespace Google.ProtocolBuffers.TestProtos {
             return this;
           }
           public Builder AddRangeField73(scg::IEnumerable<int> values) {
-            base.AddRange(values, result.field73_);
+            result.field73_.Add(values);
             return this;
           }
           public Builder ClearField73() {
@@ -3591,7 +3591,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeGroup1(scg::IEnumerable<global::Google.ProtocolBuffers.TestProtos.SizeMessage2.Types.Group1> values) {
-        base.AddRange(values, result.group1_);
+        result.group1_.Add(values);
         return this;
       }
       public Builder ClearGroup1() {
@@ -3619,7 +3619,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeField128(scg::IEnumerable<string> values) {
-        base.AddRange(values, result.field128_);
+        result.field128_.Add(values);
         return this;
       }
       public Builder ClearField128() {
@@ -3665,7 +3665,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeField127(scg::IEnumerable<string> values) {
-        base.AddRange(values, result.field127_);
+        result.field127_.Add(values);
         return this;
       }
       public Builder ClearField127() {
@@ -3709,7 +3709,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeField130(scg::IEnumerable<long> values) {
-        base.AddRange(values, result.field130_);
+        result.field130_.Add(values);
         return this;
       }
       public Builder ClearField130() {

src/ProtocolBuffers.Test/TestProtos/UnitTestGoogleSpeedProtoFile.cs

@@ -975,7 +975,7 @@ namespace Google.ProtocolBuffers.TestProtos {
           Field4 = other.Field4;
         }
         if (other.field5_.Count != 0) {
-          base.AddRange(other.field5_, result.field5_);
+          result.field5_.Add(other.field5_);
         }
         if (other.HasField59) {
           Field59 = other.Field59;
@@ -1511,7 +1511,7 @@ namespace Google.ProtocolBuffers.TestProtos {
       }
       [global::System.CLSCompliant(false)]
       public Builder AddRangeField5(scg::IEnumerable<ulong> values) {
-        base.AddRange(values, result.field5_);
+        result.field5_.Add(values);
         return this;
       }
       public Builder ClearField5() {
@@ -3569,7 +3569,7 @@ namespace Google.ProtocolBuffers.TestProtos {
               Field13 = other.Field13;
             }
             if (other.field14_.Count != 0) {
-              base.AddRange(other.field14_, result.field14_);
+              result.field14_.Add(other.field14_);
             }
             if (other.HasField15) {
               Field15 = other.Field15;
@@ -3590,10 +3590,10 @@ namespace Google.ProtocolBuffers.TestProtos {
               Field16 = other.Field16;
             }
             if (other.field22_.Count != 0) {
-              base.AddRange(other.field22_, result.field22_);
+              result.field22_.Add(other.field22_);
             }
             if (other.field73_.Count != 0) {
-              base.AddRange(other.field73_, result.field73_);
+              result.field73_.Add(other.field73_);
             }
             if (other.HasField20) {
               Field20 = other.Field20;
@@ -3820,7 +3820,7 @@ namespace Google.ProtocolBuffers.TestProtos {
             return this;
           }
           public Builder AddRangeField14(scg::IEnumerable<string> values) {
-            base.AddRange(values, result.field14_);
+            result.field14_.Add(values);
             return this;
           }
           public Builder ClearField14() {
@@ -3961,7 +3961,7 @@ namespace Google.ProtocolBuffers.TestProtos {
             return this;
           }
           public Builder AddRangeField22(scg::IEnumerable<string> values) {
-            base.AddRange(values, result.field22_);
+            result.field22_.Add(values);
             return this;
           }
           public Builder ClearField22() {
@@ -3987,7 +3987,7 @@ namespace Google.ProtocolBuffers.TestProtos {
             return this;
           }
           public Builder AddRangeField73(scg::IEnumerable<int> values) {
-            base.AddRange(values, result.field73_);
+            result.field73_.Add(values);
             return this;
           }
           public Builder ClearField73() {
@@ -4764,22 +4764,22 @@ namespace Google.ProtocolBuffers.TestProtos {
           Field63 = other.Field63;
         }
         if (other.group1_.Count != 0) {
-          base.AddRange(other.group1_, result.group1_);
+          result.group1_.Add(other.group1_);
         }
         if (other.field128_.Count != 0) {
-          base.AddRange(other.field128_, result.field128_);
+          result.field128_.Add(other.field128_);
         }
         if (other.HasField131) {
           Field131 = other.Field131;
         }
         if (other.field127_.Count != 0) {
-          base.AddRange(other.field127_, result.field127_);
+          result.field127_.Add(other.field127_);
         }
         if (other.HasField129) {
           Field129 = other.Field129;
         }
         if (other.field130_.Count != 0) {
-          base.AddRange(other.field130_, result.field130_);
+          result.field130_.Add(other.field130_);
         }
         if (other.HasField205) {
           Field205 = other.Field205;
@@ -5389,7 +5389,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeGroup1(scg::IEnumerable<global::Google.ProtocolBuffers.TestProtos.SpeedMessage2.Types.Group1> values) {
-        base.AddRange(values, result.group1_);
+        result.group1_.Add(values);
         return this;
       }
       public Builder ClearGroup1() {
@@ -5417,7 +5417,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeField128(scg::IEnumerable<string> values) {
-        base.AddRange(values, result.field128_);
+        result.field128_.Add(values);
         return this;
       }
       public Builder ClearField128() {
@@ -5463,7 +5463,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeField127(scg::IEnumerable<string> values) {
-        base.AddRange(values, result.field127_);
+        result.field127_.Add(values);
         return this;
       }
       public Builder ClearField127() {
@@ -5507,7 +5507,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeField130(scg::IEnumerable<long> values) {
-        base.AddRange(values, result.field130_);
+        result.field130_.Add(values);
         return this;
       }
       public Builder ClearField130() {

src/ProtocolBuffers.Test/TestProtos/UnitTestMessageSetProtoFile.cs

@@ -1499,7 +1499,7 @@ namespace Google.ProtocolBuffers.TestProtos {
       public override Builder MergeFrom(RawMessageSet other) {
         if (other == global::Google.ProtocolBuffers.TestProtos.RawMessageSet.DefaultInstance) return this;
         if (other.item_.Count != 0) {
-          base.AddRange(other.item_, result.item_);
+          result.item_.Add(other.item_);
         }
         this.MergeUnknownFields(other.UnknownFields);
         return this;
@@ -1587,7 +1587,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeItem(scg::IEnumerable<global::Google.ProtocolBuffers.TestProtos.RawMessageSet.Types.Item> values) {
-        base.AddRange(values, result.item_);
+        result.item_.Add(values);
         return this;
       }
       public Builder ClearItem() {

src/ProtocolBuffers.Test/TestProtos/UnitTestRpcInterop.cs

@@ -245,7 +245,7 @@ namespace Google.ProtocolBuffers.TestProtos {
       public override Builder MergeFrom(SearchRequest other) {
         if (other == global::Google.ProtocolBuffers.TestProtos.SearchRequest.DefaultInstance) return this;
         if (other.criteria_.Count != 0) {
-          base.AddRange(other.criteria_, result.criteria_);
+          result.criteria_.Add(other.criteria_);
         }
         this.MergeUnknownFields(other.UnknownFields);
         return this;
@@ -323,7 +323,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeCriteria(scg::IEnumerable<string> values) {
-        base.AddRange(values, result.criteria_);
+        result.criteria_.Add(values);
         return this;
       }
       public Builder ClearCriteria() {
@@ -791,7 +791,7 @@ namespace Google.ProtocolBuffers.TestProtos {
       public override Builder MergeFrom(SearchResponse other) {
         if (other == global::Google.ProtocolBuffers.TestProtos.SearchResponse.DefaultInstance) return this;
         if (other.results_.Count != 0) {
-          base.AddRange(other.results_, result.results_);
+          result.results_.Add(other.results_);
         }
         this.MergeUnknownFields(other.UnknownFields);
         return this;
@@ -879,7 +879,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeResults(scg::IEnumerable<global::Google.ProtocolBuffers.TestProtos.SearchResponse.Types.ResultItem> values) {
-        base.AddRange(values, result.results_);
+        result.results_.Add(values);
         return this;
       }
       public Builder ClearResults() {
@@ -1076,7 +1076,7 @@ namespace Google.ProtocolBuffers.TestProtos {
       public override Builder MergeFrom(RefineSearchRequest other) {
         if (other == global::Google.ProtocolBuffers.TestProtos.RefineSearchRequest.DefaultInstance) return this;
         if (other.criteria_.Count != 0) {
-          base.AddRange(other.criteria_, result.criteria_);
+          result.criteria_.Add(other.criteria_);
         }
         if (other.HasPreviousResults) {
           MergePreviousResults(other.PreviousResults);
@@ -1166,7 +1166,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeCriteria(scg::IEnumerable<string> values) {
-        base.AddRange(values, result.criteria_);
+        result.criteria_.Add(values);
         return this;
       }
       public Builder ClearCriteria() {

src/ProtocolBuffers.Test/TestProtos/UnitTestXmlSerializerTestProtoFile.cs

@@ -318,7 +318,7 @@ namespace Google.ProtocolBuffers.TestProtos {
       public override Builder MergeFrom(TestXmlChild other) {
         if (other == global::Google.ProtocolBuffers.TestProtos.TestXmlChild.DefaultInstance) return this;
         if (other.options_.Count != 0) {
-          base.AddRange(other.options_, result.options_);
+          result.options_.Add(other.options_);
         }
         if (other.HasBinary) {
           Binary = other.Binary;
@@ -411,7 +411,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeOptions(scg::IEnumerable<global::Google.ProtocolBuffers.TestProtos.EnumOptions> values) {
-        base.AddRange(values, result.options_);
+        result.options_.Add(values);
         return this;
       }
       public Builder ClearOptions() {
@@ -1119,7 +1119,7 @@ namespace Google.ProtocolBuffers.TestProtos {
           public override Builder MergeFrom(Children other) {
             if (other == global::Google.ProtocolBuffers.TestProtos.TestXmlMessage.Types.Children.DefaultInstance) return this;
             if (other.options_.Count != 0) {
-              base.AddRange(other.options_, result.options_);
+              result.options_.Add(other.options_);
             }
             if (other.HasBinary) {
               Binary = other.Binary;
@@ -1212,7 +1212,7 @@ namespace Google.ProtocolBuffers.TestProtos {
             return this;
           }
           public Builder AddRangeOptions(scg::IEnumerable<global::Google.ProtocolBuffers.TestProtos.EnumOptions> values) {
-            base.AddRange(values, result.options_);
+            result.options_.Add(values);
             return this;
           }
           public Builder ClearOptions() {
@@ -1500,13 +1500,13 @@ namespace Google.ProtocolBuffers.TestProtos {
           Number = other.Number;
         }
         if (other.numbers_.Count != 0) {
-          base.AddRange(other.numbers_, result.numbers_);
+          result.numbers_.Add(other.numbers_);
         }
         if (other.HasText) {
           Text = other.Text;
         }
         if (other.textlines_.Count != 0) {
-          base.AddRange(other.textlines_, result.textlines_);
+          result.textlines_.Add(other.textlines_);
         }
         if (other.HasValid) {
           Valid = other.Valid;
@@ -1515,7 +1515,7 @@ namespace Google.ProtocolBuffers.TestProtos {
           MergeChild(other.Child);
         }
         if (other.children_.Count != 0) {
-          base.AddRange(other.children_, result.children_);
+          result.children_.Add(other.children_);
         }
           this.MergeExtensionFields(other);
         this.MergeUnknownFields(other.UnknownFields);
@@ -1640,7 +1640,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeNumbers(scg::IEnumerable<int> values) {
-        base.AddRange(values, result.numbers_);
+        result.numbers_.Add(values);
         return this;
       }
       public Builder ClearNumbers() {
@@ -1687,7 +1687,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeTextlines(scg::IEnumerable<string> values) {
-        base.AddRange(values, result.textlines_);
+        result.textlines_.Add(values);
         return this;
       }
       public Builder ClearTextlines() {
@@ -1779,7 +1779,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeChildren(scg::IEnumerable<global::Google.ProtocolBuffers.TestProtos.TestXmlMessage.Types.Children> values) {
-        base.AddRange(values, result.children_);
+        result.children_.Add(values);
         return this;
       }
       public Builder ClearChildren() {

src/ProtocolBuffers/Collections/PopsicleList.cs

@@ -42,6 +42,7 @@ namespace Google.ProtocolBuffers.Collections
     /// </summary>
     public sealed class PopsicleList<T> : IPopsicleList<T>, ICastArray
     {
+        private static readonly bool CheckForNull = default(T) == null;
         private static readonly T[] EmptySet = new T[0];
 
         private List<T> items;
@@ -65,6 +66,10 @@ namespace Google.ProtocolBuffers.Collections
         public void Insert(int index, T item)
         {
             ValidateModification();
+            if (CheckForNull)
+            {
+                ThrowHelper.ThrowIfNull(item);
+            }
             items.Insert(index, item);
         }
 
@@ -87,6 +92,10 @@ namespace Google.ProtocolBuffers.Collections
             set
             {
                 ValidateModification();
+                if (CheckForNull)
+                {
+                    ThrowHelper.ThrowIfNull(value);
+                }
                 items[index] = value;
             }
         }
@@ -94,6 +103,10 @@ namespace Google.ProtocolBuffers.Collections
         public void Add(T item)
         {
             ValidateModification();
+            if (CheckForNull)
+            {
+                ThrowHelper.ThrowIfNull(item);
+            }
             items.Add(item);
         }
 
@@ -145,15 +158,30 @@ namespace Google.ProtocolBuffers.Collections
 
         public void Add(IEnumerable<T> collection)
         {
-            if (readOnly)
+            ValidateModification();
+            ThrowHelper.ThrowIfNull(collection);
+
+            if (!CheckForNull || collection is PopsicleList<T>)
             {
-                throw new NotSupportedException("List is read-only");
+                items.AddRange(collection);
             }
-            if (items == null)
+            else
             {
-                items = new List<T>();
+                // Assumption, it's ok to enumerate collections more than once.
+                if (collection is ICollection<T>)
+                {
+                    ThrowHelper.ThrowIfAnyNull(collection);
+                    items.AddRange(collection);
+                }
+                else
+                {
+                    foreach (T item in collection)
+                    {
+                        ThrowHelper.ThrowIfNull(item);
+                        items.Add(item);
+                    }
+                }
             }
-            items.AddRange(collection);
         }
 
         private void ValidateModification()

src/ProtocolBuffers/GeneratedBuilder.cs

@@ -84,32 +84,6 @@ namespace Google.ProtocolBuffers
             set { InternalFieldAccessors[field].SetValue(ThisBuilder, value); }
         }
 
-        /// <summary>
-        /// Adds all of the specified values to the given collection.
-        /// </summary>
-        /// <exception cref="ArgumentNullException">Any element of the list is null</exception>
-        protected void AddRange<T>(IEnumerable<T> source, IList<T> destination)
-        {
-            ThrowHelper.ThrowIfNull(source);
-            // We only need to check this for nullable types.
-            if (default(T) == null)
-            {
-                ThrowHelper.ThrowIfAnyNull(source);
-            }
-            List<T> list = destination as List<T>;
-            if (list != null)
-            {
-                list.AddRange(source);
-            }
-            else
-            {
-                foreach (T element in source)
-                {
-                    destination.Add(element);
-                }
-            }
-        }
-
         /// <summary>
         /// Called by derived classes to parse an unknown field.
         /// </summary>

src/ProtocolBuffers/GeneratedBuilderLite.cs

@@ -66,32 +66,6 @@ namespace Google.ProtocolBuffers
             get { return MessageBeingBuilt.IsInitialized; }
         }
 
-        /// <summary>
-        /// Adds all of the specified values to the given collection.
-        /// </summary>
-        /// <exception cref="ArgumentNullException">Any element of the list is null</exception>
-        protected void AddRange<T>(IEnumerable<T> source, IList<T> destination)
-        {
-            ThrowHelper.ThrowIfNull(source);
-            // We only need to check this for nullable types.
-            if (default(T) == null)
-            {
-                ThrowHelper.ThrowIfAnyNull(source);
-            }
-            List<T> list = destination as List<T>;
-            if (list != null)
-            {
-                list.AddRange(source);
-            }
-            else
-            {
-                foreach (T element in source)
-                {
-                    destination.Add(element);
-                }
-            }
-        }
-
         /// <summary>
         /// Called by derived classes to parse an unknown field.
         /// </summary>

src/ProtocolBuffersLite.Test/TestProtos/UnitTestExtrasFullProtoFile.cs

@@ -809,7 +809,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeCodes(scg::IEnumerable<int> values) {
-        base.AddRange(values, result.codes_);
+        result.codes_.Add(values);
         return this;
       }
       public Builder ClearCodes() {
@@ -847,7 +847,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangePhone(scg::IEnumerable<global::Google.ProtocolBuffers.TestProtos.TestInteropPerson.Types.PhoneNumber> values) {
-        base.AddRange(values, result.phone_);
+        result.phone_.Add(values);
         return this;
       }
       public Builder ClearPhone() {
@@ -885,7 +885,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeAddresses(scg::IEnumerable<global::Google.ProtocolBuffers.TestProtos.TestInteropPerson.Types.Addresses> values) {
-        base.AddRange(values, result.addresses_);
+        result.addresses_.Add(values);
         return this;
       }
       public Builder ClearAddresses() {
@@ -1231,7 +1231,7 @@ namespace Google.ProtocolBuffers.TestProtos {
             return this;
           }
           public Builder AddRangeValues(scg::IEnumerable<string> values) {
-            base.AddRange(values, result.values_);
+            result.values_.Add(values);
             return this;
           }
           public Builder ClearValues() {
@@ -1630,7 +1630,7 @@ namespace Google.ProtocolBuffers.TestProtos {
             return this;
           }
           public Builder AddRangeValues(scg::IEnumerable<string> values) {
-            base.AddRange(values, result.values_);
+            result.values_.Add(values);
             return this;
           }
           public Builder ClearValues() {

src/ProtocolBuffersLite.Test/TestProtos/UnitTestExtrasLiteProtoFile.cs

@@ -1354,13 +1354,13 @@ namespace Google.ProtocolBuffers.TestProtos {
           Email = other.Email;
         }
         if (other.codes_.Count != 0) {
-          base.AddRange(other.codes_, result.codes_);
+          result.codes_.Add(other.codes_);
         }
         if (other.phone_.Count != 0) {
-          base.AddRange(other.phone_, result.phone_);
+          result.phone_.Add(other.phone_);
         }
         if (other.addresses_.Count != 0) {
-          base.AddRange(other.addresses_, result.addresses_);
+          result.addresses_.Add(other.addresses_);
         }
           this.MergeExtensionFields(other);
         return this;
@@ -1500,7 +1500,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeCodes(scg::IEnumerable<int> values) {
-        base.AddRange(values, result.codes_);
+        result.codes_.Add(values);
         return this;
       }
       public Builder ClearCodes() {
@@ -1538,7 +1538,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangePhone(scg::IEnumerable<global::Google.ProtocolBuffers.TestProtos.TestInteropPersonLite.Types.PhoneNumber> values) {
-        base.AddRange(values, result.phone_);
+        result.phone_.Add(values);
         return this;
       }
       public Builder ClearPhone() {
@@ -1576,7 +1576,7 @@ namespace Google.ProtocolBuffers.TestProtos {
         return this;
       }
       public Builder AddRangeAddresses(scg::IEnumerable<global::Google.ProtocolBuffers.TestProtos.TestInteropPersonLite.Types.Addresses> values) {
-        base.AddRange(values, result.addresses_);
+        result.addresses_.Add(values);
         return this;
       }
       public Builder ClearAddresses() {