[GR-19220] Memcached fixes (#2752)

PullRequest: truffleruby/3519
oracle · Oct 19, 2022 · cb44f1b · cb44f1b
2 parents 5dfb664 + c28dad4
commit cb44f1b
Show file tree

Hide file tree

Showing 10 changed files with 209 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,9 @@ Bug fixes:
 Compatibility:
 
 * Fix `MatchData#[]` when passed unbounded Range (#2755, @andrykonchin).
+* Updated `rb_define_class`, `rb_define_class_under`, and `rb_define_class_id_under` to allow class names that aren't valid in Ruby (#2739, @nirvdrum).
+* Fixed `rb_gv_get` so that it no longer implicitly creates global variables (#2748, @nirvdrum).
+* Added implementations of `rb_gvar_val_getter` and `rb_define_virtual_variable` (#2750, @nirvdrum).
 
 Performance:
 

diff --git a/lib/cext/ABI_version.txt b/lib/cext/ABI_version.txt
@@ -1 +1 @@
-12
+13
diff --git a/lib/truffle/truffle/cext.rb b/lib/truffle/truffle/cext.rb
@@ -1173,8 +1173,8 @@ def rb_define_class_under(mod, name, superclass)
       raise ArgumentError, "no super class for `#{name}'"
     end
 
-    if mod.const_defined?(name, false)
-      current_class = mod.const_get(name, false)
+    if Primitive.module_const_defined?(mod, name, false, false)
+      current_class = Primitive.module_const_get(mod, name, false, false, false)
       unless current_class.class == Class
         raise TypeError, "#{mod}::#{name} is not a class"
       end
@@ -1183,7 +1183,7 @@ def rb_define_class_under(mod, name, superclass)
       end
       current_class
     else
-      mod.const_set name, Class.new(superclass)
+      rb_const_set(mod, name, Class.new(superclass))
     end
   end
 
@@ -1806,7 +1806,7 @@ def rb_gv_get(name)
     if name == '$~'
       rb_backref_get
     else
-      eval("#{name}")
+      Primitive.rb_gv_get(Primitive.to_java_string(name))
     end
   end
 

diff --git a/spec/ruby/optional/capi/class_spec.rb b/spec/ruby/optional/capi/class_spec.rb
@@ -323,6 +323,15 @@
         @s.rb_define_class("ClassSpecDefineClass4", nil)
       }.should raise_error(ArgumentError)
     end
+
+    it "allows arbitrary names, including constant names not valid in Ruby" do
+      cls = @s.rb_define_class("_INVALID_CLASS", CApiClassSpecs::Super)
+      cls.name.should == "_INVALID_CLASS"
+
+      -> {
+        Object.const_get(cls.name)
+      }.should raise_error(NameError, /wrong constant name/)
+    end
   end
 
   describe "rb_define_class_under" do
@@ -367,6 +376,15 @@
     it "raises a TypeError if class is defined and its superclass mismatches the given one" do
       -> { @s.rb_define_class_under(CApiClassSpecs, "Sub", Object) }.should raise_error(TypeError)
     end
+
+    it "allows arbitrary names, including constant names not valid in Ruby" do
+      cls = @s.rb_define_class_under(CApiClassSpecs, "_INVALID_CLASS", CApiClassSpecs::Super)
+      cls.name.should == "CApiClassSpecs::_INVALID_CLASS"
+
+      -> {
+        CApiClassSpecs.const_get(cls.name)
+      }.should raise_error(NameError, /wrong constant name/)
+    end
   end
 
   describe "rb_define_class_id_under" do
@@ -394,6 +412,15 @@
     it "raises a TypeError if class is defined and its superclass mismatches the given one" do
       -> { @s.rb_define_class_id_under(CApiClassSpecs, :Sub, Object) }.should raise_error(TypeError)
     end
+
+    it "allows arbitrary names, including constant names not valid in Ruby" do
+      cls = @s.rb_define_class_id_under(CApiClassSpecs, :_INVALID_CLASS2, CApiClassSpecs::Super)
+      cls.name.should == "CApiClassSpecs::_INVALID_CLASS2"
+
+      -> {
+        CApiClassSpecs.const_get(cls.name)
+      }.should raise_error(NameError, /wrong constant name/)
+    end
   end
 
   describe "rb_define_class_variable" do

diff --git a/spec/ruby/optional/capi/ext/globals_spec.c b/spec/ruby/optional/capi/ext/globals_spec.c
@@ -20,6 +20,16 @@ static VALUE sb_define_hooked_variable(VALUE self, VALUE var_name) {
   return Qnil;
 }
 
+static VALUE sb_define_hooked_variable_default_accessors(VALUE self, VALUE var_name) {
+  rb_define_hooked_variable(StringValuePtr(var_name), &g_hooked_var, NULL, NULL);
+  return Qnil;
+}
+
+static VALUE sb_define_hooked_variable_null_var(VALUE self, VALUE var_name) {
+    rb_define_hooked_variable(StringValuePtr(var_name), NULL, NULL, NULL);
+    return Qnil;
+}
+
 VALUE g_ro_var;
 
 static VALUE sb_define_readonly_variable(VALUE self, VALUE var_name, VALUE val) {
@@ -40,6 +50,26 @@ static VALUE sb_define_variable(VALUE self, VALUE var_name, VALUE val) {
   return Qnil;
 }
 
+long virtual_var_storage;
+
+VALUE incrementing_getter(ID id, VALUE *data) {
+  return LONG2FIX(virtual_var_storage++);
+}
+
+void incrementing_setter(VALUE val, ID id, VALUE *data) {
+  virtual_var_storage = FIX2LONG(val);
+}
+
+static VALUE sb_define_virtual_variable_default_accessors(VALUE self, VALUE name) {
+  rb_define_virtual_variable(StringValuePtr(name), NULL, NULL);
+  return Qnil;
+}
+
+static VALUE sb_define_virtual_variable_incrementing_accessors(VALUE self, VALUE name) {
+  rb_define_virtual_variable(StringValuePtr(name), incrementing_getter, incrementing_setter);
+  return Qnil;
+}
+
 static VALUE sb_f_global_variables(VALUE self) {
   return rb_f_global_variables();
 }
@@ -101,10 +131,14 @@ void Init_globals_spec(void) {
   VALUE cls = rb_define_class("CApiGlobalSpecs", rb_cObject);
   g_hooked_var = Qnil;
   rb_define_method(cls, "rb_define_hooked_variable_2x", sb_define_hooked_variable, 1);
+  rb_define_method(cls, "rb_define_hooked_variable_default_accessors", sb_define_hooked_variable_default_accessors, 1);
+  rb_define_method(cls, "rb_define_hooked_variable_null_var", sb_define_hooked_variable_null_var, 1);
   g_ro_var = Qnil;
   rb_define_method(cls, "rb_define_readonly_variable", sb_define_readonly_variable, 2);
   g_var = Qnil;
   rb_define_method(cls, "rb_define_variable", sb_define_variable, 2);
+  rb_define_method(cls, "rb_define_virtual_variable_default_accessors", sb_define_virtual_variable_default_accessors, 1);
+  rb_define_method(cls, "rb_define_virtual_variable_incrementing_accessors", sb_define_virtual_variable_incrementing_accessors, 1);
   rb_define_method(cls, "sb_get_global_value", sb_get_global_value, 0);
   rb_define_method(cls, "rb_f_global_variables", sb_f_global_variables, 0);
   rb_define_method(cls, "sb_gv_get", sb_gv_get, 1);

diff --git a/spec/ruby/optional/capi/globals_spec.rb b/spec/ruby/optional/capi/globals_spec.rb
@@ -9,7 +9,7 @@
   end
 
   it "correctly gets global values" do
-    @f.sb_gv_get("$BLAH").should == nil
+    suppress_warning { @f.sb_gv_get("$BLAH") }.should == nil
     @f.sb_gv_get("$\\").should == nil
     @f.sb_gv_get("\\").should == nil # rb_gv_get should change \ to $\
   end
@@ -21,7 +21,7 @@
   end
 
   it "correctly sets global values" do
-    @f.sb_gv_get("$BLAH").should == nil
+    suppress_warning { @f.sb_gv_get("$BLAH") }.should == nil
     @f.sb_gv_set("$BLAH", 10)
     begin
       @f.sb_gv_get("$BLAH").should == 10
@@ -42,6 +42,10 @@
   end
 
   it "rb_define_readonly_variable should define a new readonly global variable" do
+    # Check the gvar doesn't exist and ensure rb_gv_get doesn't implicitly declare the gvar,
+    # otherwise the rb_define_readonly_variable call will conflict.
+    suppress_warning { @f.sb_gv_get("ro_gvar") } .should == nil
+
     @f.rb_define_readonly_variable("ro_gvar", 15)
     $ro_gvar.should == 15
     -> { $ro_gvar = 10 }.should raise_error(NameError)
@@ -53,6 +57,52 @@
     $hooked_gvar.should == 4
   end
 
+  it "rb_define_hooked_variable should use default accessors if NULL ones are supplied" do
+    @f.rb_define_hooked_variable_default_accessors("$hooked_gvar_default_accessors")
+    $hooked_gvar_default_accessors = 10
+    $hooked_gvar_default_accessors.should == 10
+  end
+
+  it "rb_define_hooked_variable with default accessors should return nil for NULL variables" do
+    @f.rb_define_hooked_variable_null_var("$hooked_gvar_null_value")
+    $hooked_gvar_null_value.should == nil
+  end
+
+  describe "rb_define_virtual_variable" do
+    describe "with default accessors" do
+      before :all do
+        @f.rb_define_virtual_variable_default_accessors("$virtual_variable_default_accessors")
+      end
+
+      it "is read-only" do
+        -> { $virtual_variable_default_accessors = 10 }.should raise_error(NameError, /read-only/)
+      end
+
+      it "returns false with the default getter" do
+        $virtual_variable_default_accessors.should == false
+        $virtual_variable_default_accessors.should == false
+      end
+    end
+
+    describe "with supplied accessors" do
+      before :all do
+        @f.rb_define_virtual_variable_incrementing_accessors("$virtual_variable_incrementing_accessors")
+      end
+
+      it "returns a dynamically changing value" do
+        $virtual_variable_incrementing_accessors = 20
+        $virtual_variable_incrementing_accessors.should == 20
+        $virtual_variable_incrementing_accessors.should == 21
+        $virtual_variable_incrementing_accessors.should == 22
+
+        $virtual_variable_incrementing_accessors = 100
+        $virtual_variable_incrementing_accessors.should == 100
+        $virtual_variable_incrementing_accessors.should == 101
+        $virtual_variable_incrementing_accessors.should == 102
+      end
+    end
+  end
+
   describe "rb_fs" do
     before :each do
       @field_separator = $;

diff --git a/src/main/c/cext/globals.c b/src/main/c/cext/globals.c
@@ -12,8 +12,16 @@
 
 // Global variables, rb_gvar_*, rb_gv_*
 
-VALUE rb_gvar_var_getter(ID id, VALUE *data) {
-  return *data;
+VALUE rb_gvar_val_getter(ID id, VALUE *data) {
+  return (VALUE) data;
+}
+
+VALUE rb_gvar_var_getter(ID id, VALUE *var) {
+  if (!var) {
+    return Qnil;
+  }
+
+  return *var;
 }
 
 void rb_gvar_var_setter(VALUE val, ID id, VALUE *data) {
@@ -44,6 +52,18 @@ void rb_define_variable(const char *name, VALUE *var) {
   rb_define_hooked_variable(name, var, 0, 0);
 }
 
+void rb_define_virtual_variable(const char *name, rb_gvar_getter_t *getter, rb_gvar_setter_t *setter) {
+    if (!getter) {
+      getter = rb_gvar_val_getter;
+    }
+
+    if (!setter) {
+      setter = rb_gvar_readonly_setter;
+    }
+
+    rb_define_hooked_variable(name, 0, getter, setter);
+}
+
 VALUE rb_f_global_variables(void) {
   return RUBY_CEXT_INVOKE("rb_f_global_variables");
 }

diff --git a/src/main/java/org/truffleruby/cext/CExtNodes.java b/src/main/java/org/truffleruby/cext/CExtNodes.java
@@ -67,6 +67,7 @@
 import org.truffleruby.core.string.RubyString;
 import org.truffleruby.core.string.StringHelperNodes;
 import org.truffleruby.core.string.StringSupport;
+import org.truffleruby.core.string.StringUtils;
 import org.truffleruby.core.support.TypeNodes;
 import org.truffleruby.core.symbol.RubySymbol;
 import org.truffleruby.core.thread.ThreadManager;
@@ -84,6 +85,7 @@
 import org.truffleruby.language.RubyNode;
 import org.truffleruby.language.RubyRootNode;
 import org.truffleruby.language.Visibility;
+import org.truffleruby.language.WarnNode;
 import org.truffleruby.language.arguments.ArgumentsDescriptor;
 import org.truffleruby.language.arguments.EmptyArgumentsDescriptor;
 import org.truffleruby.language.arguments.KeywordArgumentsDescriptor;
@@ -103,6 +105,7 @@
 import org.truffleruby.language.control.ThrowException;
 import org.truffleruby.language.dispatch.DispatchNode;
 import org.truffleruby.language.dispatch.LiteralCallNode;
+import org.truffleruby.language.globals.ReadGlobalVariableNode;
 import org.truffleruby.language.library.RubyStringLibrary;
 import org.truffleruby.language.methods.DeclarationContext;
 import org.truffleruby.language.methods.InternalMethod;
@@ -1035,6 +1038,63 @@ protected Object rbConstSet(RubyModule module, String name, Object value,
 
     }
 
+    @Primitive(name = "rb_gv_get")
+    @ReportPolymorphism
+    public abstract static class RbGvGetNode extends PrimitiveArrayArgumentsNode {
+
+        @Child WarnNode warnNode;
+
+        @Specialization(guards = "name == cachedName", limit = "getCacheLimit()")
+        protected Object rbGvGetCached(VirtualFrame frame, String name,
+                @Cached("name") String cachedName,
+                @Cached("create(cachedName)") ReadGlobalVariableNode readGlobalVariableNode,
+                @Cached BranchProfile notExistsProfile) {
+            boolean exists = getContext().getCoreLibrary().globalVariables.contains(cachedName);
+
+            if (!exists) {
+                notExistsProfile.enter();
+                warn(StringUtils.format("global variable `%s' not initialized", cachedName));
+
+                return nil;
+            }
+
+            return readGlobalVariableNode.execute(frame);
+        }
+
+        @TruffleBoundary
+        @Specialization
+        protected Object rbGvGetUncached(String name,
+                @Cached DispatchNode dispatchNode) {
+            boolean exists = getContext().getCoreLibrary().globalVariables.contains(name);
+
+            if (!exists) {
+                warn(StringUtils.format("global variable `%s' not initialized", name));
+
+                return nil;
+            }
+
+            return dispatchNode.call(coreLibrary().topLevelBinding, "eval", name);
+        }
+
+        private void warn(String message) {
+            if (warnNode == null) {
+                CompilerDirectives.transferToInterpreterAndInvalidate();
+                warnNode = insert(new WarnNode());
+            }
+
+            if (warnNode.shouldWarn()) {
+                warnNode.warningMessage(
+                        getContext().getCallStack().getTopMostUserSourceSection(),
+                        message);
+            }
+        }
+
+        protected int getCacheLimit() {
+            return getLanguage().options.DEFAULT_CACHE;
+        }
+
+    }
+
     @CoreMethod(names = "cext_module_function", onSingleton = true, required = 2)
     public abstract static class CextModuleFunctionNode extends CoreMethodArrayArgumentsNode {
 

diff --git a/src/main/java/org/truffleruby/interop/InteropNodes.java b/src/main/java/org/truffleruby/interop/InteropNodes.java
@@ -1746,8 +1746,8 @@ protected boolean isJavaClassOrInterface(Object object) {
 
     }
 
-    @CoreMethod(names = "to_java_string", onSingleton = true, required = 1)
-    public abstract static class InteropToJavaStringNode extends CoreMethodArrayArgumentsNode {
+    @Primitive(name = "to_java_string")
+    public abstract static class ToJavaStringPrimitiveNode extends PrimitiveArrayArgumentsNode {
         @Specialization
         protected String toJavaString(Object value,
                 @Cached ToJavaStringNode toJavaStringNode) {

diff --git a/src/main/ruby/truffleruby/core/truffle/interop.rb b/src/main/ruby/truffleruby/core/truffle/interop.rb
@@ -220,6 +220,10 @@ def self.from_java_string(string)
       Primitive.foreign_string_to_ruby_string(string)
     end
 
+    def self.to_java_string(string)
+      Primitive.to_java_string(string)
+    end
+
     def self.java_array(*array)
       to_java_array(array)
     end