From 547d8e8221b8ea7ff7010cbc601f4e5742ff1e51 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Fri, 7 Aug 2015 13:37:21 +0100 Subject: [PATCH] Make FieldDescriptor.IsPacked work appropriately. This is a bit of a grotty hack, as we need to sort of fake proto2 field presence, but with only a proto3 version of the descriptor messages (a bit like oneof detection). Should be okay, but will need to be careful of this if we ever implement proto2. --- .../Reflection/FieldAccessTest.cs | 2 +- .../Google.Protobuf/Reflection/FieldDescriptor.cs | 8 +++++--- .../src/Google.Protobuf/Reflection/PartialClasses.cs | 12 ++++++++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs index 6e1d804eac..936e41c68a 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs @@ -213,6 +213,6 @@ namespace Google.Protobuf.Reflection var descriptor = TestAllTypes.Descriptor; Assert.Throws(() => descriptor.Fields[999999].ToString()); Assert.Throws(() => descriptor.Fields["not found"].ToString()); - } + } } } diff --git a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs index bb8e9bbb9c..901cbf47a5 100644 --- a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs @@ -168,14 +168,16 @@ namespace Google.Protobuf.Reflection get { return fieldType == FieldType.Message && messageType.Proto.Options != null && messageType.Proto.Options.MapEntry; } } - // TODO(jonskeet): Check whether this is correct with proto3, where we default to packed... - /// /// Returns true if this field is a packed, repeated field; false otherwise. /// public bool IsPacked { - get { return Proto.Options != null && Proto.Options.Packed; } + // Note the || rather than && here - we're effectively defaulting to packed, because that *is* + // the default in proto3, which is all we support. We may give the wrong result for the protos + // within descriptor.proto, but that's okay, as they're never exposed and we don't use IsPacked + // within the runtime. + get { return Proto.Options == null || Proto.Options.Packed; } } /// diff --git a/csharp/src/Google.Protobuf/Reflection/PartialClasses.cs b/csharp/src/Google.Protobuf/Reflection/PartialClasses.cs index c7ed434249..8c055d6d92 100644 --- a/csharp/src/Google.Protobuf/Reflection/PartialClasses.cs +++ b/csharp/src/Google.Protobuf/Reflection/PartialClasses.cs @@ -44,4 +44,16 @@ namespace Google.Protobuf.Reflection OneofIndex = -1; } } + + internal partial class FieldOptions + { + // We can't tell the difference between "explicitly set to false" and "not set" + // in proto3, but we need to tell the difference for FieldDescriptor.IsPacked. + // This won't work if we ever need to support proto2, but at that point we'll be + // able to remove this hack and use field presence instead. + partial void OnConstruction() + { + Packed = true; + } + } } \ No newline at end of file