Merge pull request #534 from jskeet/proto3-freeze

Implement Freeze in C# protos

csharp/buildall.sh

@@ -11,12 +11,13 @@ SRC=$(dirname $0)/src
 
 set -ex
 
-echo Building the solution.
+# echo Building the solution.
-xbuild /p:Configuration=$CONFIG $SRC/ProtocolBuffers.sln
+# TODO(jonskeet): Re-enable building the whole solution when we have ProtoBench et al
+# working again.
+# xbuild /p:Configuration=$CONFIG $SRC/ProtocolBuffers.sln
 
-echo Running tests.
+xbuild /p:Configuration=$CONFIG $SRC/ProtocolBuffers/ProtocolBuffers.csproj
-$NUNIT_CONSOLE $SRC/ProtocolBuffers.Test/bin/$CONFIG/Google.ProtocolBuffers.Test.dll
+xbuild /p:Configuration=$CONFIG $SRC/ProtocolBuffers.Test/ProtocolBuffers.Test.csproj
-
-$NUNIT_CONSOLE $SRC/ProtocolBuffersLite.Test/bin/$CONFIG/Google.ProtocolBuffersLite.Test.dll
 
-$NUNIT_CONSOLE $SRC/ProtocolBuffersLite.Test/bin/$CONFIG/Google.ProtocolBuffersMixedLite.Test.dll
+echo Running tests.
+$NUNIT_CONSOLE $SRC/ProtocolBuffers.Test/bin/$CONFIG/Google.Protobuf.Test.dll

csharp/src/AddressBook/Addressbook.cs

@@ -76,6 +76,9 @@ namespace Google.ProtocolBuffers.Examples.AddressBook {
       get { return global::Google.ProtocolBuffers.Examples.AddressBook.Addressbook.internal__static_tutorial_Person__FieldAccessorTable; }
     }
 
+    private bool _frozen = false;
+    public bool IsFrozen { get { return _frozen; } }
+
     public Person() { }
 
     public Person(Person other) {
@@ -89,30 +92,44 @@ namespace Google.ProtocolBuffers.Examples.AddressBook {
       return new Person(this);
     }
 
+    public void Freeze() {
+      if (IsFrozen) {
+        return;
+      }
+      _frozen = true;
+      phone_.Freeze();
+    }
+
     public const int NameFieldNumber = 1;
     private string name_ = "";
     public string Name {
       get { return name_; }
-      set { name_ = value ?? ""; }
+      set {
+        pb::Freezable.CheckMutable(this);
+        name_ = value ?? "";
+      }
     }
 
-
     public const int IdFieldNumber = 2;
     private int id_;
     public int Id {
       get { return id_; }
-      set { id_ = value; }
+      set {
+        pb::Freezable.CheckMutable(this);
+        id_ = value;
+      }
     }
 
-
     public const int EmailFieldNumber = 3;
     private string email_ = "";
     public string Email {
       get { return email_; }
-      set { email_ = value ?? ""; }
+      set {
+        pb::Freezable.CheckMutable(this);
+        email_ = value ?? "";
+      }
     }
 
-
     public const int PhoneFieldNumber = 4;
     private readonly pbc::RepeatedField<global::Google.ProtocolBuffers.Examples.AddressBook.Person.Types.PhoneNumber> phone_ = new pbc::RepeatedField<global::Google.ProtocolBuffers.Examples.AddressBook.Person.Types.PhoneNumber>();
     public pbc::RepeatedField<global::Google.ProtocolBuffers.Examples.AddressBook.Person.Types.PhoneNumber> Phone {
@@ -254,6 +271,9 @@ namespace Google.ProtocolBuffers.Examples.AddressBook {
           get { return global::Google.ProtocolBuffers.Examples.AddressBook.Addressbook.internal__static_tutorial_Person_PhoneNumber__FieldAccessorTable; }
         }
 
+        private bool _frozen = false;
+        public bool IsFrozen { get { return _frozen; } }
+
         public PhoneNumber() { }
 
         public PhoneNumber(PhoneNumber other) {
@@ -265,22 +285,33 @@ namespace Google.ProtocolBuffers.Examples.AddressBook {
           return new PhoneNumber(this);
         }
 
+        public void Freeze() {
+          if (IsFrozen) {
+            return;
+          }
+          _frozen = true;
+        }
+
         public const int NumberFieldNumber = 1;
         private string number_ = "";
         public string Number {
           get { return number_; }
-          set { number_ = value ?? ""; }
+          set {
+            pb::Freezable.CheckMutable(this);
+            number_ = value ?? "";
+          }
         }
 
-
         public const int TypeFieldNumber = 2;
         private global::Google.ProtocolBuffers.Examples.AddressBook.Person.Types.PhoneType type_ = global::Google.ProtocolBuffers.Examples.AddressBook.Person.Types.PhoneType.HOME;
         public global::Google.ProtocolBuffers.Examples.AddressBook.Person.Types.PhoneType Type {
           get { return type_; }
-          set { type_ = value; }
+          set {
+            pb::Freezable.CheckMutable(this);
+            type_ = value;
+          }
         }
 
-
         public override bool Equals(object other) {
           return Equals(other as PhoneNumber);
         }
@@ -382,6 +413,9 @@ namespace Google.ProtocolBuffers.Examples.AddressBook {
       get { return global::Google.ProtocolBuffers.Examples.AddressBook.Addressbook.internal__static_tutorial_AddressBook__FieldAccessorTable; }
     }
 
+    private bool _frozen = false;
+    public bool IsFrozen { get { return _frozen; } }
+
     public AddressBook() { }
 
     public AddressBook(AddressBook other) {
@@ -392,6 +426,14 @@ namespace Google.ProtocolBuffers.Examples.AddressBook {
       return new AddressBook(this);
     }
 
+    public void Freeze() {
+      if (IsFrozen) {
+        return;
+      }
+      _frozen = true;
+      person_.Freeze();
+    }
+
     public const int PersonFieldNumber = 1;
     private readonly pbc::RepeatedField<global::Google.ProtocolBuffers.Examples.AddressBook.Person> person_ = new pbc::RepeatedField<global::Google.ProtocolBuffers.Examples.AddressBook.Person>();
     public pbc::RepeatedField<global::Google.ProtocolBuffers.Examples.AddressBook.Person> Person {

csharp/src/ProtocolBuffers.Test/ByteStringTest.cs

@@ -40,6 +40,7 @@ using NUnit.Framework;
 
 namespace Google.Protobuf
 {
+    [TestFixture]
     public class ByteStringTest
     {
         [Test]

csharp/src/ProtocolBuffers.Test/GeneratedMessageTest.cs

@@ -1,4 +1,5 @@
-using Google.Protobuf.TestProtos;
+using System;
+using Google.Protobuf.TestProtos;
 using NUnit.Framework;
 
 namespace Google.Protobuf
@@ -257,5 +258,22 @@ namespace Google.Protobuf
             original.OneofNestedMessage.Bb = 30;
             Assert.AreNotEqual(original, clone);
         }
+
+        [Test]
+        public void Freeze()
+        {
+            var frozen = new TestAllTypes();
+            frozen.Freeze();
+            Assert.IsTrue(frozen.IsFrozen);
+
+            Assert.Throws<InvalidOperationException>(() => frozen.ClearOneofField());
+            Assert.Throws<InvalidOperationException>(() => frozen.SingleInt32 = 0);
+            Assert.Throws<InvalidOperationException>(() => frozen.SingleNestedMessage = null);
+            Assert.Throws<InvalidOperationException>(() => frozen.SingleNestedEnum = 0);
+            Assert.Throws<InvalidOperationException>(() => frozen.OneofString = null);
+            Assert.Throws<InvalidOperationException>(() => frozen.OneofUint32 = 0U);
+            Assert.Throws<InvalidOperationException>(() => frozen.RepeatedDouble.Add(0.0));
+            Assert.Throws<InvalidOperationException>(() => frozen.RepeatedNestedMessage.Add(new TestAllTypes.Types.NestedMessage()));
+        }
     }
 }

csharp/src/ProtocolBuffers.Test/RepeatedFieldTest.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using Google.Protobuf.Collections;
+using Google.Protobuf.TestProtos;
 using NUnit.Framework;
 
 namespace Google.Protobuf
@@ -11,8 +12,8 @@ namespace Google.Protobuf
         public void NullValuesRejected()
         {
             var list = new RepeatedField<string>();
-            Assert.Throws<ArgumentNullException>(() => list.Add((string) null));
+            Assert.Throws<ArgumentNullException>(() => list.Add((string)null));
-            Assert.Throws<ArgumentNullException>(() => list.Add((IEnumerable<string>) null));
+            Assert.Throws<ArgumentNullException>(() => list.Add((IEnumerable<string>)null));
             Assert.Throws<ArgumentNullException>(() => list.Add((RepeatedField<string>)null));
             Assert.Throws<ArgumentNullException>(() => list.Contains(null));
             Assert.Throws<ArgumentNullException>(() => list.IndexOf(null));
@@ -47,5 +48,47 @@ namespace Google.Protobuf
             Assert.AreEqual("foo", list[1]);
             Assert.AreEqual("bar", list[2]);
         }
+
+        [Test]
+        public void Freeze_FreezesElements()
+        {
+            var list = new RepeatedField<TestAllTypes> { new TestAllTypes() };
+            Assert.IsFalse(list[0].IsFrozen);
+            list.Freeze();
+            Assert.IsTrue(list[0].IsFrozen);
+        }
+
+        [Test]
+        public void Freeze_PreventsMutations()
+        {
+            var list = new RepeatedField<int> { 0 };
+            list.Freeze();
+            Assert.Throws<InvalidOperationException>(() => list.Add(1));
+            Assert.Throws<InvalidOperationException>(() => list[0] = 1);
+            Assert.Throws<InvalidOperationException>(() => list.Clear());
+            Assert.Throws<InvalidOperationException>(() => list.RemoveAt(0));
+            Assert.Throws<InvalidOperationException>(() => list.Remove(0));
+            Assert.Throws<InvalidOperationException>(() => list.Insert(0, 0));
+        }
+
+        [Test]
+        public void Freeze_ReportsFrozen()
+        {
+            var list = new RepeatedField<int> { 0 };
+            Assert.IsFalse(list.IsFrozen);
+            Assert.IsFalse(list.IsReadOnly);
+            list.Freeze();
+            Assert.IsTrue(list.IsFrozen);
+            Assert.IsTrue(list.IsReadOnly);
+        }
+
+        [Test]
+        public void Clone_ReturnsMutable()
+        {
+            var list = new RepeatedField<int> { 0 };
+            list.Freeze();
+            var clone = list.Clone();
+            clone[0] = 1;
+        }
     }
 }

csharp/src/ProtocolBuffers.Test/TestProtos/UnittestImportProto3.cs

@@ -74,6 +74,9 @@ namespace Google.Protobuf.TestProtos {
       get { return global::Google.Protobuf.TestProtos.UnittestImportProto3.internal__static_protobuf_unittest_import_ImportMessage__FieldAccessorTable; }
     }
 
+    private bool _frozen = false;
+    public bool IsFrozen { get { return _frozen; } }
+
     public ImportMessage() { }
 
     public ImportMessage(ImportMessage other) {
@@ -84,14 +87,23 @@ namespace Google.Protobuf.TestProtos {
       return new ImportMessage(this);
     }
 
+    public void Freeze() {
+      if (IsFrozen) {
+        return;
+      }
+      _frozen = true;
+    }
+
     public const int DFieldNumber = 1;
     private int d_;
     public int D {
       get { return d_; }
-      set { d_ = value; }
+      set {
+        pb::Freezable.CheckMutable(this);
+        d_ = value;
+      }
     }
 
-
     public override bool Equals(object other) {
       return Equals(other as ImportMessage);
     }

csharp/src/ProtocolBuffers.Test/TestProtos/UnittestImportPublicProto3.cs

@@ -59,6 +59,9 @@ namespace Google.Protobuf.TestProtos {
       get { return global::Google.Protobuf.TestProtos.UnittestImportPublicProto3.internal__static_protobuf_unittest_import_PublicImportMessage__FieldAccessorTable; }
     }
 
+    private bool _frozen = false;
+    public bool IsFrozen { get { return _frozen; } }
+
     public PublicImportMessage() { }
 
     public PublicImportMessage(PublicImportMessage other) {
@@ -69,14 +72,23 @@ namespace Google.Protobuf.TestProtos {
       return new PublicImportMessage(this);
     }
 
+    public void Freeze() {
+      if (IsFrozen) {
+        return;
+      }
+      _frozen = true;
+    }
+
     public const int EFieldNumber = 1;
     private int e_;
     public int E {
       get { return e_; }
-      set { e_ = value; }
+      set {
+        pb::Freezable.CheckMutable(this);
+        e_ = value;
+      }
     }
 
-
     public override bool Equals(object other) {
       return Equals(other as PublicImportMessage);
     }

csharp/src/ProtocolBuffers.Test/TestProtos/UnittestIssues.cs

@@ -104,6 +104,9 @@ namespace UnitTest.Issues.TestProtos {
       get { return global::UnitTest.Issues.TestProtos.UnittestIssues.internal__static_unittest_issues_NegativeEnumMessage__FieldAccessorTable; }
     }
 
+    private bool _frozen = false;
+    public bool IsFrozen { get { return _frozen; } }
+
     public NegativeEnumMessage() { }
 
     public NegativeEnumMessage(NegativeEnumMessage other) {
@@ -116,14 +119,25 @@ namespace UnitTest.Issues.TestProtos {
       return new NegativeEnumMessage(this);
     }
 
+    public void Freeze() {
+      if (IsFrozen) {
+        return;
+      }
+      _frozen = true;
+      values_.Freeze();
+      packedValues_.Freeze();
+    }
+
     public const int ValueFieldNumber = 1;
     private global::UnitTest.Issues.TestProtos.NegativeEnum value_ = global::UnitTest.Issues.TestProtos.NegativeEnum.NEGATIVE_ENUM_ZERO;
     public global::UnitTest.Issues.TestProtos.NegativeEnum Value {
       get { return value_; }
-      set { value_ = value; }
+      set {
+        pb::Freezable.CheckMutable(this);
+        value_ = value;
+      }
     }
 
-
     public const int ValuesFieldNumber = 2;
     private readonly pbc::RepeatedField<global::UnitTest.Issues.TestProtos.NegativeEnum> values_ = new pbc::RepeatedField<global::UnitTest.Issues.TestProtos.NegativeEnum>();
     public pbc::RepeatedField<global::UnitTest.Issues.TestProtos.NegativeEnum> Values {
@@ -255,6 +269,9 @@ namespace UnitTest.Issues.TestProtos {
       get { return global::UnitTest.Issues.TestProtos.UnittestIssues.internal__static_unittest_issues_DeprecatedChild__FieldAccessorTable; }
     }
 
+    private bool _frozen = false;
+    public bool IsFrozen { get { return _frozen; } }
+
     public DeprecatedChild() { }
 
     public DeprecatedChild(DeprecatedChild other) {
@@ -264,6 +281,13 @@ namespace UnitTest.Issues.TestProtos {
       return new DeprecatedChild(this);
     }
 
+    public void Freeze() {
+      if (IsFrozen) {
+        return;
+      }
+      _frozen = true;
+    }
+
     public override bool Equals(object other) {
       return Equals(other as DeprecatedChild);
     }
@@ -328,6 +352,9 @@ namespace UnitTest.Issues.TestProtos {
       get { return global::UnitTest.Issues.TestProtos.UnittestIssues.internal__static_unittest_issues_DeprecatedFieldsMessage__FieldAccessorTable; }
     }
 
+    private bool _frozen = false;
+    public bool IsFrozen { get { return _frozen; } }
+
     public DeprecatedFieldsMessage() { }
 
     public DeprecatedFieldsMessage(DeprecatedFieldsMessage other) {
@@ -343,15 +370,28 @@ namespace UnitTest.Issues.TestProtos {
       return new DeprecatedFieldsMessage(this);
     }
 
+    public void Freeze() {
+      if (IsFrozen) {
+        return;
+      }
+      _frozen = true;
+      primitiveArray_.Freeze();
+      if (messageValue_ != null) MessageValue.Freeze();
+      messageArray_.Freeze();
+      enumArray_.Freeze();
+    }
+
     public const int PrimitiveValueFieldNumber = 1;
     private int primitiveValue_;
     [global::System.ObsoleteAttribute()]
     public int PrimitiveValue {
       get { return primitiveValue_; }
-      set { primitiveValue_ = value; }
+      set {
+        pb::Freezable.CheckMutable(this);
+        primitiveValue_ = value;
+      }
     }
 
-
     public const int PrimitiveArrayFieldNumber = 2;
     private readonly pbc::RepeatedField<int> primitiveArray_ = new pbc::RepeatedField<int>();
     [global::System.ObsoleteAttribute()]
@@ -364,7 +404,10 @@ namespace UnitTest.Issues.TestProtos {
     [global::System.ObsoleteAttribute()]
     public global::UnitTest.Issues.TestProtos.DeprecatedChild MessageValue {
       get { return messageValue_; }
-      set { messageValue_ = value; }
+      set {
+        pb::Freezable.CheckMutable(this);
+        messageValue_ = value;
+      }
     }
 
     public const int MessageArrayFieldNumber = 4;
@@ -379,10 +422,12 @@ namespace UnitTest.Issues.TestProtos {
     [global::System.ObsoleteAttribute()]
     public global::UnitTest.Issues.TestProtos.DeprecatedEnum EnumValue {
       get { return enumValue_; }
-      set { enumValue_ = value; }
+      set {
+        pb::Freezable.CheckMutable(this);
+        enumValue_ = value;
+      }
     }
 
-
     public const int EnumArrayFieldNumber = 6;
     private readonly pbc::RepeatedField<global::UnitTest.Issues.TestProtos.DeprecatedEnum> enumArray_ = new pbc::RepeatedField<global::UnitTest.Issues.TestProtos.DeprecatedEnum>();
     [global::System.ObsoleteAttribute()]
@@ -403,7 +448,8 @@ namespace UnitTest.Issues.TestProtos {
       }
       if (PrimitiveValue != other.PrimitiveValue) return false;
       if(!primitiveArray_.Equals(other.primitiveArray_)) return false;
-      if (!object.Equals(MessageValue, other.MessageValue)) return false;if(!messageArray_.Equals(other.messageArray_)) return false;
+      if (!object.Equals(MessageValue, other.MessageValue)) return false;
+      if(!messageArray_.Equals(other.messageArray_)) return false;
       if (EnumValue != other.EnumValue) return false;
       if(!enumArray_.Equals(other.enumArray_)) return false;
       return true;
@@ -563,6 +609,9 @@ namespace UnitTest.Issues.TestProtos {
       get { return global::UnitTest.Issues.TestProtos.UnittestIssues.internal__static_unittest_issues_ItemField__FieldAccessorTable; }
     }
 
+    private bool _frozen = false;
+    public bool IsFrozen { get { return _frozen; } }
+
     public ItemField() { }
 
     public ItemField(ItemField other) {
@@ -573,14 +622,23 @@ namespace UnitTest.Issues.TestProtos {
       return new ItemField(this);
     }
 
+    public void Freeze() {
+      if (IsFrozen) {
+        return;
+      }
+      _frozen = true;
+    }
+
     public const int ItemFieldNumber = 1;
     private int item_;
     public int Item {
       get { return item_; }
-      set { item_ = value; }
+      set {
+        pb::Freezable.CheckMutable(this);
+        item_ = value;
+      }
     }
 
-
     public override bool Equals(object other) {
       return Equals(other as ItemField);
     }

csharp/src/ProtocolBuffers.sln

@@ -4,8 +4,6 @@ VisualStudioVersion = 14.0.22823.1
 MinimumVisualStudioVersion = 10.0.40219.1
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ProtocolBuffers", "ProtocolBuffers\ProtocolBuffers.csproj", "{6908BDCE-D925-43F3-94AC-A531E6DF2591}"
 EndProject
-Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ProtocolBuffers.Serialization", "ProtocolBuffers.Serialization\ProtocolBuffers.Serialization.csproj", "{231391AF-449C-4A39-986C-AD7F270F4750}"
-EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ProtocolBuffers.Test", "ProtocolBuffers.Test\ProtocolBuffers.Test.csproj", "{DD01ED24-3750-4567-9A23-1DB676A15610}"
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "AddressBook", "AddressBook\AddressBook.csproj", "{A31F5FB2-4FF3-432A-B35B-5CD203606311}"
@@ -26,10 +24,6 @@ Global
 		{6908BDCE-D925-43F3-94AC-A531E6DF2591}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{6908BDCE-D925-43F3-94AC-A531E6DF2591}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{6908BDCE-D925-43F3-94AC-A531E6DF2591}.Release|Any CPU.Build.0 = Release|Any CPU
-		{231391AF-449C-4A39-986C-AD7F270F4750}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
-		{231391AF-449C-4A39-986C-AD7F270F4750}.Debug|Any CPU.Build.0 = Debug|Any CPU
-		{231391AF-449C-4A39-986C-AD7F270F4750}.Release|Any CPU.ActiveCfg = Release|Any CPU
-		{231391AF-449C-4A39-986C-AD7F270F4750}.Release|Any CPU.Build.0 = Release|Any CPU
 		{DD01ED24-3750-4567-9A23-1DB676A15610}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
 		{DD01ED24-3750-4567-9A23-1DB676A15610}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{DD01ED24-3750-4567-9A23-1DB676A15610}.Release|Any CPU.ActiveCfg = Release|Any CPU

csharp/src/ProtocolBuffers/Collections/RepeatedField.cs

@@ -4,10 +4,16 @@ using System.Collections.Generic;
 
 namespace Google.Protobuf.Collections
 {
-    public sealed class RepeatedField<T> : IList<T>, IEquatable<RepeatedField<T>>
+    /// <summary>
+    /// The contents of a repeated field: essentially, a collection with some extra
+    /// restrictions (no null values) and capabilities (deep cloning and freezing).
+    /// </summary>
+    /// <typeparam name="T">The element type of the repeated field.</typeparam>
+    public sealed class RepeatedField<T> : IList<T>, IDeepCloneable<RepeatedField<T>>, IEquatable<RepeatedField<T>>, IFreezable
     {
         private static readonly T[] EmptyArray = new T[0];
 
+        private bool frozen;
         private const int MinArraySize = 8;
         private T[] array = EmptyArray;
         private int count = 0;
@@ -26,6 +32,7 @@ namespace Google.Protobuf.Collections
         public RepeatedField<T> Clone()
         {
             RepeatedField<T> clone = new RepeatedField<T>();
+            // Clone is implicitly *not* frozen, even if this object is.
             if (array != EmptyArray)
             {
                 clone.array = (T[])array.Clone();
@@ -42,6 +49,21 @@ namespace Google.Protobuf.Collections
             return clone;
         }
 
+        public bool IsFrozen { get { return frozen; } }
+
+        public void Freeze()
+        {
+            frozen = true;
+            IFreezable[] freezableArray = array as IFreezable[];
+            if (freezableArray != null)
+            {
+                for (int i = 0; i < count; i++)
+                {
+                    freezableArray[i].Freeze();
+                }
+            }
+        }
+
         private void EnsureSize(int size)
         {
             size = Math.Max(size, MinArraySize);
@@ -60,6 +82,7 @@ namespace Google.Protobuf.Collections
             {
                 throw new ArgumentNullException("item");
             }
+            this.CheckMutable();
             EnsureSize(count + 1);
             array[count++] = item;
         }
@@ -70,6 +93,7 @@ namespace Google.Protobuf.Collections
         /// <param name="readEnum"></param>
         internal void AddInt32(int item)
         {
+            this.CheckMutable();
             EnsureSize(count + 1);
             int[] castArray = (int[]) (object) array;
             castArray[count++] = item;
@@ -77,6 +101,7 @@ namespace Google.Protobuf.Collections
 
         public void Clear()
         {
+            this.CheckMutable();
             array = EmptyArray;
             count = 0;
         }
@@ -93,6 +118,7 @@ namespace Google.Protobuf.Collections
 
         public bool Remove(T item)
         {
+            this.CheckMutable();
             int index = IndexOf(item);
             if (index == -1)
             {
@@ -107,7 +133,7 @@ namespace Google.Protobuf.Collections
         public int Count { get { return count; } }
 
         // TODO(jonskeet): If we implement freezing, make this reflect it.
-        public bool IsReadOnly { get { return false; } }
+        public bool IsReadOnly { get { return IsFrozen; } }
 
         public void Add(RepeatedField<T> values)
         {
@@ -115,6 +141,7 @@ namespace Google.Protobuf.Collections
             {
                 throw new ArgumentNullException("values");
             }
+            this.CheckMutable();
             EnsureSize(count + values.count);
             // We know that all the values will be valid, because it's a RepeatedField.
             Array.Copy(values.array, 0, array, count, values.count);
@@ -127,6 +154,7 @@ namespace Google.Protobuf.Collections
             {
                 throw new ArgumentNullException("values");
             }
+            this.CheckMutable();
             // TODO: Check for ICollection and get the Count?
             foreach (T item in values)
             {
@@ -227,6 +255,7 @@ namespace Google.Protobuf.Collections
             {
                 throw new ArgumentOutOfRangeException("index");
             }
+            this.CheckMutable();
             EnsureSize(count + 1);
             Array.Copy(array, index, array, index + 1, count - index);
             count++;
@@ -238,6 +267,7 @@ namespace Google.Protobuf.Collections
             {
                 throw new ArgumentOutOfRangeException("index");
             }
+            this.CheckMutable();
             Array.Copy(array, index + 1, array, index, count - index - 1);
             count--;
             array[count] = default(T);
@@ -259,6 +289,7 @@ namespace Google.Protobuf.Collections
                 {
                     throw new ArgumentOutOfRangeException("index");
                 }
+                this.CheckMutable();
                 if (value == null)
                 {
                     throw new ArgumentNullException("value");

csharp/src/ProtocolBuffers/Freezable.cs

@@ -0,0 +1,60 @@
+#region Copyright notice and license
+
+// Protocol Buffers - Google's data interchange format
+// Copyright 2015 Google Inc.  All rights reserved.
+// http://github.com/google/protobuf
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#endregion
+
+using System;
+
+namespace Google.Protobuf
+{
+    /// <summary>
+    /// Extension methods for <see cref="IFreezable"/> types.
+    /// </summary>
+    public static class Freezable
+    {
+        /// <summary>
+        /// Throws an <see cref="InvalidOperationException"/> if <paramref name="target"/>
+        /// is frozen.
+        /// </summary>
+        /// <remarks>
+        /// This is a convenience methods that freezable types can call before all
+        /// mutations, to protect frozen objects.
+        /// </remarks>
+        public static void CheckMutable(this IFreezable target)
+        {
+            if (target.IsFrozen)
+            {
+                throw new InvalidOperationException("Attempt to mutate frozen object");
+            }
+        }
+    }
+}

csharp/src/ProtocolBuffers/IMessage.cs

@@ -41,6 +41,7 @@ namespace Google.Protobuf
 {
 
     // TODO(jonskeet): Do we want a "weak" (non-generic) version of IReflectedMessage?
+    // TODO(jonskeet): Split these interfaces into separate files when we're happy with them.
 
     /// <summary>
     /// Reflection support for a specific message type. message
@@ -85,7 +86,7 @@ namespace Google.Protobuf
     /// the implementation class.
     /// </summary>
     /// <typeparam name="T">The message type.</typeparam>
-    public interface IMessage<T> : IMessage, IEquatable<T>, IDeepCloneable<T> where T : IMessage<T>
+    public interface IMessage<T> : IMessage, IEquatable<T>, IDeepCloneable<T>, IFreezable where T : IMessage<T>
     {
         /// <summary>
         /// Merges the given message into this one.
@@ -102,6 +103,11 @@ namespace Google.Protobuf
     /// All generated messages implement this interface, but so do some non-message types.
     /// Additionally, due to the type constraint on <c>T</c> in <see cref="IMessage{T}"/>,
     /// it is simpler to keep this as a separate interface.
+    /// </para>
+    /// <para>
+    /// Freezable types which implement this interface should always return a mutable clone,
+    /// even if the original object is frozen.
+    /// </para>
     /// </remarks>
     /// <typeparam name="T">The type itself, returned by the <see cref="Clone"/> method.</typeparam>
     public interface IDeepCloneable<T>
@@ -112,4 +118,32 @@ namespace Google.Protobuf
         /// <returns>A deep clone of this object.</returns>
         T Clone();
     }
+
+    /// <summary>
+    /// Provides a mechanism for freezing a message (or repeated field collection)
+    /// to make it immutable.
+    /// </summary>
+    /// <remarks>
+    /// Implementations are under no obligation to make this thread-safe: if a freezable
+    /// type instance is shared between threads before being frozen, and one thread then
+    /// freezes it, it is possible for other threads to make changes during the freezing
+    /// operation and also to observe stale values for mutated fields. Objects should be
+    /// frozen before being made available to other threads.
+    /// </remarks>
+    public interface IFreezable
+    {
+        /// <summary>
+        /// Freezes this object.
+        /// </summary>
+        /// <remarks>
+        /// If the object is already frozen, this method has no effect.
+        /// </remarks>
+        void Freeze();
+
+        /// <summary>
+        /// Returns whether or not this object is frozen (and therefore immutable).
+        /// </summary>
+        /// <value><c>true</c> if this object is frozen; <c>false</c> otherwise.</value>
+        bool IsFrozen { get; }
+    }
 }

csharp/src/ProtocolBuffers/ProtocolBuffers.csproj

@@ -85,6 +85,7 @@
     <Compile Include="Descriptors\PackageDescriptor.cs" />
     <Compile Include="Descriptors\ServiceDescriptor.cs" />
     <Compile Include="FrameworkPortability.cs" />
+    <Compile Include="Freezable.cs" />
     <Compile Include="MessageExtensions.cs" />
     <Compile Include="FieldAccess\FieldAccessorBase.cs" />
     <Compile Include="FieldAccess\ReflectionUtil.cs" />

src/google/protobuf/compiler/csharp/csharp_field_base.cc

@@ -57,7 +57,7 @@ void FieldGeneratorBase::SetCommonFieldVariables(
   // repeated fields varies by wire format. The wire format is encoded in the bottom 3 bits, which
   // never effects the tag size.
   int tag_size = internal::WireFormat::TagSize(descriptor_->number(), descriptor_->type());
-  uint tag = internal::WireFormat::MakeTag(descriptor_);
+  uint tag = FixedMakeTag(descriptor_);
   uint8 tag_array[5];
   io::CodedOutputStream::WriteTagToArray(tag, tag_array);
   string tag_bytes = SimpleItoa(tag_array[0]);
@@ -107,6 +107,11 @@ FieldGeneratorBase::FieldGeneratorBase(const FieldDescriptor* descriptor,
 FieldGeneratorBase::~FieldGeneratorBase() {
 }
 
+void FieldGeneratorBase::GenerateFreezingCode(io::Printer* printer) {
+  // No-op: only message fields and repeated fields need
+  // special handling for freezing, so default to not generating any code.
+}
+
 void FieldGeneratorBase::AddDeprecatedFlag(io::Printer* printer) {
   if (descriptor_->options().deprecated())
   {

src/google/protobuf/compiler/csharp/csharp_field_base.h

@@ -48,6 +48,7 @@ class FieldGeneratorBase : public SourceGeneratorBase {
   ~FieldGeneratorBase();
 
   virtual void GenerateCloningCode(io::Printer* printer) = 0;
+  virtual void GenerateFreezingCode(io::Printer* printer);
   virtual void GenerateMembers(io::Printer* printer) = 0;
   virtual void GenerateMergingCode(io::Printer* printer) = 0;
   virtual void GenerateParsingCode(io::Printer* printer) = 0;

src/google/protobuf/compiler/csharp/csharp_helpers.cc

@@ -338,6 +338,17 @@ std::string FileDescriptorToBase64(const FileDescriptor* descriptor) {
   return StringToBase64(fdp_bytes);
 }
 
+// TODO(jonskeet): Remove this when internal::WireFormat::MakeTag works
+// properly...
+// Workaround for issue #493
+uint FixedMakeTag(const FieldDescriptor* field) {
+  internal::WireFormatLite::WireType field_type = field->is_packed()
+      ? internal::WireFormatLite::WIRETYPE_LENGTH_DELIMITED
+      : internal::WireFormat::WireTypeForFieldType(field->type());
+
+  return internal::WireFormatLite::MakeTag(field->number(), field_type);
+}
+
 FieldGeneratorBase* CreateFieldGenerator(const FieldDescriptor* descriptor,
                                          int fieldOrdinal) {
   switch (descriptor->type()) {

src/google/protobuf/compiler/csharp/csharp_helpers.h

@@ -97,6 +97,8 @@ std::string StringToBase64(const std::string& input);
 
 std::string FileDescriptorToBase64(const FileDescriptor* descriptor);
 
+uint FixedMakeTag(const FieldDescriptor* descriptor);
+
 FieldGeneratorBase* CreateFieldGenerator(const FieldDescriptor* descriptor, int fieldOrdinal);
 
 bool HasRequiredFields(const Descriptor* descriptor);

src/google/protobuf/compiler/csharp/csharp_message.cc

@@ -194,8 +194,7 @@ void MessageGenerator::Generate(io::Printer* printer) {
       "slash", field_names().size() > 0 ? "\"" : "");
   std::vector<std::string> tags;
   for (int i = 0; i < field_names().size(); i++) {
-    uint32 tag = internal::WireFormat::MakeTag(
+    uint32 tag = FixedMakeTag(descriptor_->FindFieldByName(field_names()[i]));
-        descriptor_->FindFieldByName(field_names()[i]));
     tags.push_back(SimpleItoa(tag));
   }
   printer->Print(
@@ -211,7 +210,9 @@ void MessageGenerator::Generate(io::Printer* printer) {
     "public pb::FieldAccess.FieldAccessorTable<$class_name$> Fields {\n"
     "  get { return $umbrella_class_name$.internal__$identifier$__FieldAccessorTable; }\n"
     "}\n"
-    "\n");
+    "\n"
+    "private bool _frozen = false;\n"
+    "public bool IsFrozen { get { return _frozen; } }\n\n");
 
   // Parameterless constructor
   printer->Print(
@@ -219,6 +220,7 @@ void MessageGenerator::Generate(io::Printer* printer) {
     "public $class_name$() { }\n\n");
 
   GenerateCloningCode(printer);
+  GenerateFreezingCode(printer);
 
   // Fields/properties
   for (int i = 0; i < descriptor_->field_count(); i++) {
@@ -260,6 +262,7 @@ void MessageGenerator::Generate(io::Printer* printer) {
       "  get { return $name$Case_; }\n"
       "}\n\n"
       "public void Clear$property_name$() {\n"
+      "  pb::Freezable.CheckMutable(this);\n"
       "  $name$Case_ = $property_name$OneofCase.None;\n"
       "  $name$_ = null;\n"
       "}\n\n");
@@ -346,6 +349,36 @@ void MessageGenerator::GenerateCloningCode(io::Printer* printer) {
     "}\n\n");
 }
 
+void MessageGenerator::GenerateFreezingCode(io::Printer* printer) {
+    map<string, string> vars;
+    vars["class_name"] = class_name();
+    printer->Print(
+      "public void Freeze() {\n"
+      "  if (IsFrozen) {\n"
+      "    return;\n"
+      "  }\n"
+      "  _frozen = true;\n");
+    printer->Indent();
+    // Freeze non-oneof fields first (only messages and repeated fields will actually generate any code)
+    for (int i = 0; i < descriptor_->field_count(); i++) {
+        if (!descriptor_->field(i)->containing_oneof()) {
+            scoped_ptr<FieldGeneratorBase> generator(
+                CreateFieldGeneratorInternal(descriptor_->field(i)));
+            generator->GenerateFreezingCode(printer);
+        }
+    }
+
+    // For each oneof, if the value is freezable, freeze it. We don't actually need to know which type it was.
+    for (int i = 0; i < descriptor_->oneof_decl_count(); ++i) {
+        vars["name"] = UnderscoresToCamelCase(descriptor_->oneof_decl(i)->name(), false);
+        printer->Print(vars,
+            "if ($name$_ is IFreezable) ((IFreezable) $name$_).Freeze();\n");
+    }
+
+    printer->Outdent();
+    printer->Print("}\n\n");
+}
+
 void MessageGenerator::GenerateFrameworkMethods(io::Printer* printer) {
     map<string, string> vars;
     vars["class_name"] = class_name();

src/google/protobuf/compiler/csharp/csharp_message.h

@@ -51,6 +51,7 @@ class MessageGenerator : public SourceGeneratorBase {
   ~MessageGenerator();
 
   void GenerateCloningCode(io::Printer* printer);
+  void GenerateFreezingCode(io::Printer* printer);
   void GenerateFrameworkMethods(io::Printer* printer);
   void GenerateStaticVariables(io::Printer* printer);
   void GenerateStaticVariableInitializers(io::Printer* printer);

src/google/protobuf/compiler/csharp/csharp_message_field.cc

@@ -66,7 +66,10 @@ void MessageFieldGenerator::GenerateMembers(io::Printer* printer) {
     variables_,
     "public $type_name$ $property_name$ {\n"
     "  get { return $name$_; }\n"
-    "  set { $name$_ = value; }\n"
+    "  set {\n"
+    "    pb::Freezable.CheckMutable(this);\n"
+    "    $name$_ = value;\n"
+    "  }\n"
     "}\n");
 }
 
@@ -116,7 +119,7 @@ void MessageFieldGenerator::WriteHash(io::Printer* printer) {
 void MessageFieldGenerator::WriteEquals(io::Printer* printer) {
   printer->Print(
     variables_,
-    "if (!object.Equals($property_name$, other.$property_name$)) return false;");
+    "if (!object.Equals($property_name$, other.$property_name$)) return false;\n");
 }
 void MessageFieldGenerator::WriteToString(io::Printer* printer) {
   variables_["field_name"] = GetFieldName(descriptor_);
@@ -130,6 +133,11 @@ void MessageFieldGenerator::GenerateCloningCode(io::Printer* printer) {
     "$property_name$ = other.$has_property_check$ ? other.$property_name$.Clone() : null;\n");
 }
 
+void MessageFieldGenerator::GenerateFreezingCode(io::Printer* printer) {
+  printer->Print(variables_,
+    "if ($has_property_check$) $property_name$.Freeze();\n");
+}
+
 MessageOneofFieldGenerator::MessageOneofFieldGenerator(const FieldDescriptor* descriptor,
 						       int fieldOrdinal)
     : MessageFieldGenerator(descriptor, fieldOrdinal) {
@@ -147,6 +155,7 @@ void MessageOneofFieldGenerator::GenerateMembers(io::Printer* printer) {
     "public $type_name$ $property_name$ {\n"
     "  get { return $has_property_check$ ? ($type_name$) $oneof_name$_ : null; }\n"
     "  set {\n"
+    "    pb::Freezable.CheckMutable(this);\n"
     "    $oneof_name$_ = value;\n"
     "    $oneof_name$Case_ = value == null ? $oneof_property_name$OneofCase.None : $oneof_property_name$OneofCase.$property_name$;\n"
     "  }\n"

src/google/protobuf/compiler/csharp/csharp_message_field.h

@@ -47,6 +47,7 @@ class MessageFieldGenerator : public FieldGeneratorBase {
   ~MessageFieldGenerator();
 
   virtual void GenerateCloningCode(io::Printer* printer);
+  virtual void GenerateFreezingCode(io::Printer* printer);
   virtual void GenerateMembers(io::Printer* printer);
   virtual void GenerateMergingCode(io::Printer* printer);
   virtual void GenerateParsingCode(io::Printer* printer);

src/google/protobuf/compiler/csharp/csharp_primitive_field.cc

@@ -72,17 +72,21 @@ void PrimitiveFieldGenerator::GenerateMembers(io::Printer* printer) {
   printer->Print(
     variables_,
     "public $type_name$ $property_name$ {\n"
-    "  get { return $name$_; }\n");
+    "  get { return $name$_; }\n"
+    "  set {\n"
+    "    pb::Freezable.CheckMutable(this);\n");
   if (is_value_type) {
     printer->Print(
       variables_,
-      "  set { $name$_ = value; }\n");
+      "    $name$_ = value;\n");
   } else {
     printer->Print(
       variables_,
-      "  set { $name$_ = value ?? $default_value$; }\n");
+      "    $name$_ = value ?? $default_value$;\n");
   }
-  printer->Print("}\n\n");
+  printer->Print(
+    "  }\n"
+    "}\n");
 }
 
 void PrimitiveFieldGenerator::GenerateMergingCode(io::Printer* printer) {
@@ -166,7 +170,8 @@ void PrimitiveOneofFieldGenerator::GenerateMembers(io::Printer* printer) {
     variables_,
     "public $type_name$ $property_name$ {\n"
     "  get { return $has_property_check$ ? ($type_name$) $oneof_name$_ : $default_value$; }\n"
-    "  set {\n");
+    "  set {\n"
+    "    pb::Freezable.CheckMutable(this);\n");
     if (is_value_type) {
       printer->Print(
         variables_,

src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc

@@ -147,6 +147,11 @@ void RepeatedEnumFieldGenerator::GenerateCloningCode(io::Printer* printer) {
     "$name$_ = other.$name$_.Clone();\n");
 }
 
+void RepeatedEnumFieldGenerator::GenerateFreezingCode(io::Printer* printer) {
+  printer->Print(variables_,
+    "$name$_.Freeze();\n");
+}
+
 }  // namespace csharp
 }  // namespace compiler
 }  // namespace protobuf

src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.h

@@ -49,6 +49,7 @@ class RepeatedEnumFieldGenerator : public FieldGeneratorBase {
   ~RepeatedEnumFieldGenerator();
 
   virtual void GenerateCloningCode(io::Printer* printer);
+  virtual void GenerateFreezingCode(io::Printer* printer);
   virtual void GenerateMembers(io::Printer* printer);
   virtual void GenerateMergingCode(io::Printer* printer);
   virtual void GenerateParsingCode(io::Printer* printer);

src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc

@@ -123,6 +123,11 @@ void RepeatedMessageFieldGenerator::GenerateCloningCode(io::Printer* printer) {
     "$name$_ = other.$name$_.Clone();\n");
 }
 
+void RepeatedMessageFieldGenerator::GenerateFreezingCode(io::Printer* printer) {
+  printer->Print(variables_,
+    "$name$_.Freeze();\n");
+}
+
 }  // namespace csharp
 }  // namespace compiler
 }  // namespace protobuf

src/google/protobuf/compiler/csharp/csharp_repeated_message_field.h

@@ -47,6 +47,7 @@ class RepeatedMessageFieldGenerator : public FieldGeneratorBase {
   ~RepeatedMessageFieldGenerator();
 
   virtual void GenerateCloningCode(io::Printer* printer);
+  virtual void GenerateFreezingCode(io::Printer* printer);
   virtual void GenerateMembers(io::Printer* printer);
   virtual void GenerateMergingCode(io::Printer* printer);
   virtual void GenerateParsingCode(io::Printer* printer);

src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc

@@ -153,6 +153,11 @@ void RepeatedPrimitiveFieldGenerator::GenerateCloningCode(io::Printer* printer)
     "$name$_ = other.$name$_.Clone();\n");
 }
 
+void RepeatedPrimitiveFieldGenerator::GenerateFreezingCode(io::Printer* printer) {
+  printer->Print(variables_,
+    "$name$_.Freeze();\n");
+}
+
 }  // namespace csharp
 }  // namespace compiler
 }  // namespace protobuf

src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.h

@@ -47,6 +47,7 @@ class RepeatedPrimitiveFieldGenerator : public FieldGeneratorBase {
   ~RepeatedPrimitiveFieldGenerator();
 
   virtual void GenerateCloningCode(io::Printer* printer);
+  virtual void GenerateFreezingCode(io::Printer* printer);
   virtual void GenerateMembers(io::Printer* printer);
   virtual void GenerateMergingCode(io::Printer* printer);
   virtual void GenerateParsingCode(io::Printer* printer);

src/google/protobuf/descriptor.pb.cc

@@ -753,10 +753,9 @@ void protobuf_AddDesc_google_2fprotobuf_2fdescriptor_2eproto() {
     "ion\032\206\001\n\010Location\022\020\n\004path\030\001 \003(\005B\002\020\001\022\020\n\004sp"
     "an\030\002 \003(\005B\002\020\001\022\030\n\020leading_comments\030\003 \001(\t\022\031"
     "\n\021trailing_comments\030\004 \001(\t\022!\n\031leading_det"
-    "ached_comments\030\006 \003(\tBe\n\023com.google.proto"
+    "ached_comments\030\006 \003(\tB^\n\023com.google.proto"
     "bufB\020DescriptorProtosH\001Z\ndescriptor\242\002\003GP"
-    "B\252\002\'Google.ProtocolBuffers.DescriptorPro"
+    "B\252\002 Google.Protobuf.DescriptorProtos", 4956);
-    "tos", 4963);
   ::google::protobuf::MessageFactory::InternalRegisterGeneratedFile(
     "google/protobuf/descriptor.proto", &protobuf_RegisterTypes);
   FileDescriptorSet::default_instance_ = new FileDescriptorSet();

src/google/protobuf/wire_format.h

@@ -290,7 +290,7 @@ class LIBPROTOBUF_EXPORT UnknownFieldSetFieldSkipper : public FieldSkipper {
 
 inline WireFormatLite::WireType WireFormat::WireTypeForField(
     const FieldDescriptor* field) {
-  if (field->is_packed()) {
+  if (field->options().packed()) {
     return WireFormatLite::WIRETYPE_LENGTH_DELIMITED;
   } else {
     return WireTypeForFieldType(field->type());