Simplify deep freeze logic.

protocolbuffers · Nov 8, 2023 · 16cc9e3 · 16cc9e3
1 parent 34b0dea
commit 16cc9e3
Show file tree

Hide file tree

Showing 9 changed files with 60 additions and 86 deletions.
diff --git a/ruby/ext/google/protobuf_c/map.c b/ruby/ext/google/protobuf_c/map.c
@@ -578,21 +578,16 @@ static VALUE Map_freeze(VALUE _self) {
  */
 VALUE Map_internal_deep_freeze(VALUE _self) {
   Map* self = ruby_to_Map(_self);
+  Map_freeze(_self);
+  if (self->value_type_info.type == kUpb_CType_Message) {
+    size_t iter = kUpb_Map_Begin;
+    upb_MessageValue key, val;
 
-  if (!RB_OBJ_FROZEN(_self)) {
-    Map_freeze(_self);
-
-    if (self->value_type_info.type == kUpb_CType_Message) {
-      size_t iter = kUpb_Map_Begin;
-      upb_MessageValue key, val;
-
-      while (upb_Map_Next(self->map, &key, &val, &iter)) {
-        VALUE val_val = Convert_UpbToRuby(val, self->value_type_info, self->arena);
-        Message_internal_deep_freeze(val_val);
-      }
+    while (upb_Map_Next(self->map, &key, &val, &iter)) {
+      VALUE val_val = Convert_UpbToRuby(val, self->value_type_info, self->arena);
+      Message_internal_deep_freeze(val_val);
     }
   }
-
   return _self;
 }
 

diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c
@@ -865,26 +865,23 @@ static VALUE Message_freeze(VALUE _self) {
  */
 VALUE Message_internal_deep_freeze(VALUE _self) {
   Message* self = ruby_to_Message(_self);
-  if (!RB_OBJ_FROZEN(_self)) {
-    Message_freeze(_self);
-
-    int n = upb_MessageDef_FieldCount(self->msgdef);
-    for (int i = 0; i < n; i++) {
-      const upb_FieldDef* f = upb_MessageDef_Field(self->msgdef, i);
-      VALUE field = Message_getfield(_self, f);
-
-      if (field != Qnil) {
-        if (upb_FieldDef_IsMap(f)) {
-          Map_internal_deep_freeze(field);
-        } else if (upb_FieldDef_IsRepeated(f)) {
-          RepeatedField_internal_deep_freeze(field);
-        } else if (upb_FieldDef_IsSubMessage(f)) {
-          Message_internal_deep_freeze(field);
-        }
+  Message_freeze(_self);
+
+  int n = upb_MessageDef_FieldCount(self->msgdef);
+  for (int i = 0; i < n; i++) {
+    const upb_FieldDef* f = upb_MessageDef_Field(self->msgdef, i);
+    VALUE field = Message_getfield(_self, f);
+
+    if (field != Qnil) {
+      if (upb_FieldDef_IsMap(f)) {
+        Map_internal_deep_freeze(field);
+      } else if (upb_FieldDef_IsRepeated(f)) {
+        RepeatedField_internal_deep_freeze(field);
+      } else if (upb_FieldDef_IsSubMessage(f)) {
+        Message_internal_deep_freeze(field);
       }
     }
   }
-
   return _self;
 }
 

diff --git a/ruby/ext/google/protobuf_c/repeated_field.c b/ruby/ext/google/protobuf_c/repeated_field.c
@@ -493,22 +493,16 @@ static VALUE RepeatedField_freeze(VALUE _self) {
  */
 VALUE RepeatedField_internal_deep_freeze(VALUE _self) {
   RepeatedField* self = ruby_to_RepeatedField(_self);
-
-  if (!RB_OBJ_FROZEN(_self)) {
-    RepeatedField_freeze(_self);
-
-    if (self->type_info.type == kUpb_CType_Message) {
-      int size = upb_Array_Size(self->array);
-      int i;
-
-      for (i = 0; i < size; i++) {
-        upb_MessageValue msgval = upb_Array_Get(self->array, i);
-        VALUE val = Convert_UpbToRuby(msgval, self->type_info, self->arena);
-        Message_internal_deep_freeze(val);
-      }
+  RepeatedField_freeze(_self);
+  if (self->type_info.type == kUpb_CType_Message) {
+    int size = upb_Array_Size(self->array);
+    int i;
+    for (i = 0; i < size; i++) {
+      upb_MessageValue msgval = upb_Array_Get(self->array, i);
+      VALUE val = Convert_UpbToRuby(msgval, self->type_info, self->arena);
+      Message_internal_deep_freeze(val);
     }
   }
-
   return _self;
 }
 

diff --git a/ruby/lib/google/protobuf/ffi/map.rb b/ruby/lib/google/protobuf/ffi/map.rb
@@ -270,13 +270,11 @@ def each &block
       include Google::Protobuf::Internal::Convert
 
       def internal_deep_freeze
-        unless frozen?
-          freeze
-          if value_type == :message
-            internal_iterator do |iterator|
-              value_message_value = Google::Protobuf::FFI.map_value(@map_ptr, iterator)
-              convert_upb_to_ruby(value_message_value, value_type, descriptor, arena).send :internal_deep_freeze
-            end
+        freeze
+        if value_type == :message
+          internal_iterator do |iterator|
+            value_message_value = Google::Protobuf::FFI.map_value(@map_ptr, iterator)
+            convert_upb_to_ruby(value_message_value, value_type, descriptor, arena).send :internal_deep_freeze
           end
         end
         self

diff --git a/ruby/lib/google/protobuf/ffi/message.rb b/ruby/lib/google/protobuf/ffi/message.rb
@@ -294,13 +294,11 @@ def self.encode_json(message, options = {})
           include Google::Protobuf::Internal::Convert
 
           def internal_deep_freeze
-            unless frozen?
-              freeze
-              self.class.descriptor.each do |field_descriptor|
-                next if field_descriptor.has_presence? && !Google::Protobuf::FFI.get_message_has(@msg, field_descriptor)
-                if field_descriptor.map? or field_descriptor.repeated? or field_descriptor.sub_message?
-                  get_field(field_descriptor).send :internal_deep_freeze
-                end
+            freeze
+            self.class.descriptor.each do |field_descriptor|
+              next if field_descriptor.has_presence? && !Google::Protobuf::FFI.get_message_has(@msg, field_descriptor)
+              if field_descriptor.map? or field_descriptor.repeated? or field_descriptor.sub_message?
+                get_field(field_descriptor).send :internal_deep_freeze
               end
             end
             self

diff --git a/ruby/lib/google/protobuf/ffi/repeated_field.rb b/ruby/lib/google/protobuf/ffi/repeated_field.rb
@@ -253,12 +253,10 @@ def concat(other)
       attr :name, :arena, :array, :type, :descriptor
 
       def internal_deep_freeze
-        unless frozen?
-          freeze
-          if type == :message
-            each do |element|
-              element.send :internal_deep_freeze
-            end
+        freeze
+        if type == :message
+          each do |element|
+            element.send :internal_deep_freeze
           end
         end
         self

diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMap.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMap.java
@@ -373,12 +373,10 @@ public RubyHash toHash(ThreadContext context) {
   }
 
   protected IRubyObject deepFreeze(ThreadContext context) {
-    if (!isFrozen()) {
-      setFrozen(true);
-      if (valueType == FieldDescriptor.Type.MESSAGE) {
-        for (IRubyObject key : table.keySet()) {
-          ((RubyMessage)table.get(key)).deepFreeze(context);
-        }
+    setFrozen(true);
+    if (valueType == FieldDescriptor.Type.MESSAGE) {
+      for (IRubyObject key : table.keySet()) {
+        ((RubyMessage)table.get(key)).deepFreeze(context);
       }
     }
     return this;

diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java
@@ -817,17 +817,15 @@ public IRubyObject toHash(ThreadContext context) {
   }
 
   protected IRubyObject deepFreeze(ThreadContext context) {
-    if (!isFrozen()) {
-      setFrozen(true);
-      for (FieldDescriptor fdef : descriptor.getFields()) {
-        if (fdef.isMapField()) {
-          ((RubyMap) fields.get(fdef)).deepFreeze(context);
-        } else if (fdef.isRepeated()) {
-          this.getRepeatedField(context, fdef).deepFreeze(context);
-        } else if (fields.containsKey(fdef)) {
-          if (fdef.getType() == FieldDescriptor.Type.MESSAGE) {
-            ((RubyMessage) fields.get(fdef)).deepFreeze(context);
-          }
+    setFrozen(true);
+    for (FieldDescriptor fdef : descriptor.getFields()) {
+      if (fdef.isMapField()) {
+        ((RubyMap) fields.get(fdef)).deepFreeze(context);
+      } else if (fdef.isRepeated()) {
+        this.getRepeatedField(context, fdef).deepFreeze(context);
+      } else if (fields.containsKey(fdef)) {
+        if (fdef.getType() == FieldDescriptor.Type.MESSAGE) {
+          ((RubyMessage) fields.get(fdef)).deepFreeze(context);
         }
       }
     }

diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java
@@ -359,12 +359,10 @@ public IRubyObject inspect() {
   }
 
   protected IRubyObject deepFreeze(ThreadContext context) {
-    if (!isFrozen()) {
-      setFrozen(true);
-      if (fieldType == FieldDescriptor.Type.MESSAGE) {
-        for (int i = 0; i < size(); i++) {
-          ((RubyMessage)storage.eltInternal(i)).deepFreeze(context);
-        }
+    setFrozen(true);
+    if (fieldType == FieldDescriptor.Type.MESSAGE) {
+      for (int i = 0; i < size(); i++) {
+        ((RubyMessage)storage.eltInternal(i)).deepFreeze(context);
       }
     }
     return this;