From f4cfd2def3941402f3531a55b52915e5e2f9cc4f Mon Sep 17 00:00:00 2001 From: Sydney Acksman Date: Sun, 5 May 2019 14:51:13 -0500 Subject: [PATCH] Remove dead HasValue code for ExtensionValue and add null-checks to ExtensionSet.Set --- csharp/src/Google.Protobuf/ExtensionSet.cs | 2 + csharp/src/Google.Protobuf/ExtensionValue.cs | 39 +++++++------------ .../Reflection/CustomOptions.cs | 14 ++----- 3 files changed, 20 insertions(+), 35 deletions(-) diff --git a/csharp/src/Google.Protobuf/ExtensionSet.cs b/csharp/src/Google.Protobuf/ExtensionSet.cs index f6d1ff2d1..5b3b3167a 100644 --- a/csharp/src/Google.Protobuf/ExtensionSet.cs +++ b/csharp/src/Google.Protobuf/ExtensionSet.cs @@ -115,6 +115,8 @@ namespace Google.Protobuf /// public static void Set(ref ExtensionSet set, Extension extension, TValue value) where TTarget : IExtendableMessage { + ProtoPreconditions.CheckNotNullUnconstrained(value, nameof(value)); + IExtensionValue extensionValue; if (set == null) { diff --git a/csharp/src/Google.Protobuf/ExtensionValue.cs b/csharp/src/Google.Protobuf/ExtensionValue.cs index 986b2fce0..6ee737a7b 100644 --- a/csharp/src/Google.Protobuf/ExtensionValue.cs +++ b/csharp/src/Google.Protobuf/ExtensionValue.cs @@ -47,7 +47,6 @@ namespace Google.Protobuf internal sealed class ExtensionValue : IExtensionValue { - private bool hasValue; private T field; private FieldCodec codec; @@ -59,10 +58,6 @@ namespace Google.Protobuf public int CalculateSize() { - if (!hasValue) - { - return 0; - } return codec.CalculateSizeWithTag(field); } @@ -70,7 +65,6 @@ namespace Google.Protobuf { return new ExtensionValue(codec) { - hasValue = hasValue, field = field is IDeepCloneable ? (field as IDeepCloneable).Clone() : field }; } @@ -82,7 +76,6 @@ namespace Google.Protobuf return other is ExtensionValue && codec.Equals((other as ExtensionValue).codec) - && hasValue.Equals((other as ExtensionValue).hasValue) && Equals(field, (other as ExtensionValue).field); // we check for equality in the codec since we could have equal field values however the values could be written in different ways } @@ -92,7 +85,6 @@ namespace Google.Protobuf unchecked { int hash = 17; - hash = hash * 31 + hasValue.GetHashCode(); hash = hash * 31 + field.GetHashCode(); hash = hash * 31 + codec.GetHashCode(); return hash; @@ -101,7 +93,6 @@ namespace Google.Protobuf public void MergeFrom(CodedInputStream input) { - hasValue = true; codec.ValueMerger(input, ref field); } @@ -109,24 +100,18 @@ namespace Google.Protobuf { if (value is ExtensionValue) { - var extensionValue = value as ExtensionValue; - if (extensionValue.hasValue) - { - hasValue |= codec.FieldMerger(ref field, extensionValue.field); - } + var extensionValue = value as ExtensionValue; + codec.FieldMerger(ref field, extensionValue.field); } } public void WriteTo(CodedOutputStream output) { - if (hasValue) + output.WriteTag(codec.Tag); + codec.ValueWriter(output, field); + if (codec.EndTag != 0) { - output.WriteTag(codec.Tag); - codec.ValueWriter(output, field); - if (codec.EndTag != 0) - { - output.WriteTag(codec.EndTag); - } + output.WriteTag(codec.EndTag); } } @@ -134,15 +119,19 @@ namespace Google.Protobuf public void SetValue(T value) { - hasValue = true; field = value; } - public bool HasValue => hasValue; - public bool IsInitialized() { - return HasValue && field is IMessage && (field as IMessage).IsInitialized(); + if (field is IMessage) + { + return (field as IMessage).IsInitialized(); + } + else + { + return true; + } } } diff --git a/csharp/src/Google.Protobuf/Reflection/CustomOptions.cs b/csharp/src/Google.Protobuf/Reflection/CustomOptions.cs index 57df2ed4a..9fc376688 100644 --- a/csharp/src/Google.Protobuf/Reflection/CustomOptions.cs +++ b/csharp/src/Google.Protobuf/Reflection/CustomOptions.cs @@ -254,11 +254,8 @@ namespace Google.Protobuf.Reflection if (extensionValue is ExtensionValue) { ExtensionValue single = extensionValue as ExtensionValue; - if (single.HasValue) - { - value = single.GetValue(); - return true; - } + value = single.GetValue(); + return true; } else if (extensionValue is RepeatedExtensionValue) { @@ -279,11 +276,8 @@ namespace Google.Protobuf.Reflection var typeArgs = typeInfo.GenericTypeArguments; if (typeArgs.Length == 1 && typeArgs[0].GetTypeInfo().IsEnum) { - if ((bool)typeInfo.GetDeclaredProperty(nameof(ExtensionValue.HasValue)).GetValue(extensionValue)) - { - value = (T)typeInfo.GetDeclaredMethod(nameof(ExtensionValue.GetValue)).Invoke(extensionValue, EmptyParameters); - return true; - } + value = (T)typeInfo.GetDeclaredMethod(nameof(ExtensionValue.GetValue)).Invoke(extensionValue, EmptyParameters); + return true; } } else if (type.GetGenericTypeDefinition() == typeof(RepeatedExtensionValue<>))