Change ReadTag and PeekTag to just use 0 as a return value for "end of stream", rather than using an awkward out parameter.

This simplifies quite a lot of code.

Generated code in next commit.

csharp/src/Google.Protobuf.Test/CodedInputStreamExtensions.cs

@@ -38,8 +38,7 @@ namespace Google.Protobuf
     {
         public static void AssertNextTag(this CodedInputStream input, uint expectedTag)
         {
-            uint tag;
+            uint tag = input.ReadTag();
-            Assert.IsTrue(input.ReadTag(out tag));
             Assert.AreEqual(expectedTag, tag);
         }
 

csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs

@@ -279,9 +279,7 @@ namespace Google.Protobuf
             ms.Position = 0;
 
             CodedInputStream input = new CodedInputStream(ms);
-            uint testtag;
+            Assert.AreEqual(tag, input.ReadTag());
-            Assert.IsTrue(input.ReadTag(out testtag));
-            Assert.AreEqual(tag, testtag);
 
             // TODO(jonskeet): Should this be ArgumentNullException instead?
             Assert.Throws<InvalidProtocolBufferException>(() => input.ReadBytes());
@@ -377,9 +375,7 @@ namespace Google.Protobuf
 
             CodedInputStream input = new CodedInputStream(ms);
 
-            uint actualTag;
+            Assert.AreEqual(tag, input.ReadTag());
-            Assert.IsTrue(input.ReadTag(out actualTag));
-            Assert.AreEqual(tag, actualTag);
             string text = input.ReadString();
             Assert.AreEqual('\ufffd', text[0]);
         }
@@ -430,15 +426,21 @@ namespace Google.Protobuf
                 ms.Position = 0;
                 CodedInputStream input = new CodedInputStream(ms, new byte[ms.Length / 2]);
 
-                uint tag;
+                uint tag = input.ReadTag();
-                Assert.IsTrue(input.ReadTag(out tag));
                 Assert.AreEqual(1, WireFormat.GetTagFieldNumber(tag));
                 Assert.AreEqual(100, input.ReadBytes().Length);
 
-                Assert.IsTrue(input.ReadTag(out tag));
+                tag = input.ReadTag();
                 Assert.AreEqual(2, WireFormat.GetTagFieldNumber(tag));
                 Assert.AreEqual(100, input.ReadBytes().Length);
             }
         }
+
+        [Test]
+        public void Tag0Throws()
+        {
+            var input = new CodedInputStream(new byte[] { 0 });
+            Assert.Throws<InvalidProtocolBufferException>(() => input.ReadTag());
+        }
     }
 }

csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs

@@ -335,15 +335,16 @@ namespace Google.Protobuf
             // Now test Input stream:
             {
                 CodedInputStream cin = new CodedInputStream(new MemoryStream(bytes), new byte[50]);
-                uint tag;
                 Assert.AreEqual(0, cin.Position);
                 // Field 1:
-                Assert.IsTrue(cin.ReadTag(out tag) && tag >> 3 == 1);
+                uint tag = cin.ReadTag();
+                Assert.AreEqual(1, tag >> 3);
                 Assert.AreEqual(1, cin.Position);
                 Assert.AreEqual(500, cin.ReadInt32());
                 Assert.AreEqual(3, cin.Position);
                 //Field 2:
-                Assert.IsTrue(cin.ReadTag(out tag) && tag >> 3 == 2);
+                tag = cin.ReadTag();
+                Assert.AreEqual(2, tag >> 3);
                 Assert.AreEqual(4, cin.Position);
                 int childlen = cin.ReadLength();
                 Assert.AreEqual(120, childlen);
@@ -353,19 +354,22 @@ namespace Google.Protobuf
                 // Now we are reading child message
                 {
                     // Field 11: numeric value: 500
-                    Assert.IsTrue(cin.ReadTag(out tag) && tag >> 3 == 11);
+                    tag = cin.ReadTag();
+                    Assert.AreEqual(11, tag >> 3);
                     Assert.AreEqual(6, cin.Position);
                     Assert.AreEqual(500, cin.ReadInt32());
                     Assert.AreEqual(8, cin.Position);
                     //Field 12: length delimited 120 bytes
-                    Assert.IsTrue(cin.ReadTag(out tag) && tag >> 3 == 12);
+                    tag = cin.ReadTag();
+                    Assert.AreEqual(12, tag >> 3);
                     Assert.AreEqual(9, cin.Position);
                     ByteString bstr = cin.ReadBytes();
                     Assert.AreEqual(110, bstr.Length);
                     Assert.AreEqual((byte) 109, bstr[109]);
                     Assert.AreEqual(120, cin.Position);
                     // Field 13: fixed numeric value: 501
-                    Assert.IsTrue(cin.ReadTag(out tag) && tag >> 3 == 13);
+                    tag = cin.ReadTag();
+                    Assert.AreEqual(13, tag >> 3);
                     // ROK - Previously broken here, this returned 126 failing to account for bufferSizeAfterLimit
                     Assert.AreEqual(121, cin.Position);
                     Assert.AreEqual(501, cin.ReadSFixed32());
@@ -375,7 +379,8 @@ namespace Google.Protobuf
                 cin.PopLimit(oldlimit);
                 Assert.AreEqual(125, cin.Position);
                 // Field 3: fixed numeric value: 501
-                Assert.IsTrue(cin.ReadTag(out tag) && tag >> 3 == 3);
+                tag = cin.ReadTag();
+                Assert.AreEqual(3, tag >> 3);
                 Assert.AreEqual(126, cin.Position);
                 Assert.AreEqual(501, cin.ReadSFixed32());
                 Assert.AreEqual(130, cin.Position);

csharp/src/Google.Protobuf.Test/Collections/RepeatedFieldTest.cs

@@ -455,7 +455,7 @@ namespace Google.Protobuf.Collections
             Assert.AreEqual(0, output.SpaceLeft);
 
             CodedInputStream input = new CodedInputStream(bytes);
-            Assert.IsTrue(input.ReadTag(out tag));
+            tag = input.ReadTag();
 
             RepeatedField<SampleEnum> values = new RepeatedField<SampleEnum>();
             values.AddEntriesFrom(input, FieldCodec.ForEnum(tag, x => (int)x, x => (SampleEnum)x));
@@ -493,7 +493,7 @@ namespace Google.Protobuf.Collections
             Assert.AreEqual(0, output.SpaceLeft);
 
             CodedInputStream input = new CodedInputStream(bytes);
-            Assert.IsTrue(input.ReadTag(out tag));
+            tag = input.ReadTag();
 
             RepeatedField<SampleEnum> values = new RepeatedField<SampleEnum>();
             values.AddEntriesFrom(input, FieldCodec.ForEnum(tag, x => (int)x, x => (SampleEnum)x));

csharp/src/Google.Protobuf/CodedInputStream.cs

@@ -254,37 +254,35 @@ namespace Google.Protobuf
         #region Reading of tags etc
 
         /// <summary>
-        /// Attempts to peek at the next field tag.
+        /// Peeks at the next field tag. This is like calling <see cref="ReadTag"/>, but the
+        /// tag is not consumed. (So a subsequent call to <see cref="ReadTag"/> will return the
+        /// same value.)
         /// </summary>
-        public bool PeekNextTag(out uint fieldTag)
+        public uint PeekTag()
         {
             if (hasNextTag)
             {
-                fieldTag = nextTag;
+                return nextTag;
-                return true;
             }
 
             uint savedLast = lastTag;
-            hasNextTag = ReadTag(out nextTag);
+            nextTag = ReadTag();
-            lastTag = savedLast;
+            hasNextTag = true;
-            fieldTag = nextTag;
+            lastTag = savedLast; // Undo the side effect of ReadTag
-            return hasNextTag;
+            return nextTag;
         }
 
         /// <summary>
-        /// Attempts to read a field tag, returning false if we have reached the end
+        /// Reads a field tag, returning the tag of 0 for "end of stream".
-        /// of the input data.
         /// </summary>
-        /// <param name="fieldTag">The 'tag' of the field (id * 8 + wire-format)</param>
+        /// <returns>The next field tag, or 0 for end of stream. (0 is never a valid tag.)</returns>
-        /// <returns>true if the next fieldTag was read</returns>
+        public uint ReadTag()
-        public bool ReadTag(out uint fieldTag)
         {
             if (hasNextTag)
             {
-                fieldTag = nextTag;
+                lastTag = nextTag;
-                lastTag = fieldTag;
                 hasNextTag = false;
-                return true;
+                return lastTag;
             }
 
             // Optimize for the incredibly common case of having at least two bytes left in the buffer,
@@ -294,7 +292,7 @@ namespace Google.Protobuf
                 int tmp = buffer[bufferPos++];
                 if (tmp < 128)
                 {
-                    fieldTag = (uint)tmp;
+                    lastTag = (uint)tmp;
                 }
                 else
                 {
@@ -302,13 +300,13 @@ namespace Google.Protobuf
                     if ((tmp = buffer[bufferPos++]) < 128)
                     {
                         result |= tmp << 7;
-                        fieldTag = (uint) result;
+                        lastTag = (uint) result;
                     }
                     else
                     {
                         // Nope, rewind and go the potentially slow route.
                         bufferPos -= 2;
-                        fieldTag = ReadRawVarint32();
+                        lastTag = ReadRawVarint32();
                     }
                 }
             }
@@ -316,20 +314,18 @@ namespace Google.Protobuf
             {
                 if (IsAtEnd)
                 {
-                    fieldTag = 0;
+                    lastTag = 0;
-                    lastTag = fieldTag;
+                    return 0; // This is the only case in which we return 0.
-                    return false;
                 }
 
-                fieldTag = ReadRawVarint32();
+                lastTag = ReadRawVarint32();
             }
-            lastTag = fieldTag;
             if (lastTag == 0)
             {
                 // If we actually read zero, that's not a valid tag.
                 throw InvalidProtocolBufferException.InvalidTag();
             }
-            return true;
+            return lastTag;
         }
 
         /// <summary>
@@ -580,14 +576,10 @@ namespace Google.Protobuf
         /// </summary>
         public bool MaybeConsumeTag(uint tag)
         {
-            uint next;
+            if (PeekTag() == tag)
-            if (PeekNextTag(out next))
             {
-                if (next == tag)
+                hasNextTag = false;
-                {
+                return true;
-                    hasNextTag = false;
-                    return true;
-                }
             }
             return false;
         }

csharp/src/Google.Protobuf/Collections/MapField.cs

@@ -627,12 +627,8 @@ namespace Google.Protobuf.Collections
                 public void MergeFrom(CodedInputStream input)
                 {
                     uint tag;
-                    while (input.ReadTag(out tag))
+                    while ((tag = input.ReadTag()) != 0)
                     {
-                        if (tag == 0)
-                        {
-                            throw InvalidProtocolBufferException.InvalidTag();
-                        }
                         if (tag == codec.keyCodec.Tag)
                         {
                             Key = codec.keyCodec.Read(input);

csharp/src/Google.Protobuf/FieldCodec.cs

@@ -298,12 +298,8 @@ namespace Google.Protobuf
 
                 uint tag;
                 T value = codec.DefaultValue;
-                while (input.ReadTag(out tag))
+                while ((tag = input.ReadTag()) != 0)
                 {
-                    if (tag == 0)
-                    {
-                        throw InvalidProtocolBufferException.InvalidTag();
-                    }
                     if (tag == codec.Tag)
                     {
                         value = codec.Read(input);

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

@@ -417,13 +417,11 @@ void MessageGenerator::GenerateMergingMethods(io::Printer* printer) {
   printer->Indent();
   printer->Print(
     "uint tag;\n"
-    "while (input.ReadTag(out tag)) {\n"
+    "while ((tag = input.ReadTag()) != 0) {\n"
     "  switch(tag) {\n");
   printer->Indent();
   printer->Indent();
   printer->Print(
-    "case 0:\n"  // 0 signals EOF / limit reached
-    "  throw pb::InvalidProtocolBufferException.InvalidTag();\n"
     "default:\n"
     "  if (pb::WireFormat.IsEndGroupTag(tag)) {\n"
     "    return;\n"