[GR-18163] Fix rb_str_locktmp() and rb_str_unlocktmp() to accept Strings that are already frozen

PullRequest: truffleruby/4435

Author: andrykonchin

CHANGELOG.md

@@ -22,6 +22,7 @@ Compatibility:
 * Fix numeric coercing when `#coerce` method is not public (#3848, @andrykonchin).
 * Fix `Kernel#raise` and don't override `cause` at exception re-raising (#3831, @andrykonchin).
 * Return a pointer with `#type_size` of 1 for `Pointer#read_pointer` (@eregon).
+* Fix `rb_str_locktmp()` and `rb_str_unlocktmp()` to raise `FrozenError` when string argument is frozen (#3752, @andrykonchin).
 
 Performance:
 

spec/ruby/optional/capi/string_spec.rb

@@ -1201,28 +1201,50 @@ def inspect
 
   describe "rb_str_locktmp" do
     it "raises an error when trying to lock an already locked string" do
-      str = "test"
+      str = +"test"
       @s.rb_str_locktmp(str).should == str
       -> { @s.rb_str_locktmp(str) }.should raise_error(RuntimeError, 'temporal locking already locked string')
     end
 
     it "locks a string so that modifications would raise an error" do
-      str = "test"
+      str = +"test"
       @s.rb_str_locktmp(str).should == str
       -> { str.upcase! }.should raise_error(RuntimeError, 'can\'t modify string; temporarily locked')
     end
+
+    ruby_bug "#20998", ""..."3.5" do
+      it "raises FrozenError if string is frozen" do
+        str = -"rb_str_locktmp"
+        -> { @s.rb_str_locktmp(str) }.should raise_error(FrozenError)
+
+        str = +"rb_str_locktmp"
+        str.freeze
+        -> { @s.rb_str_locktmp(str) }.should raise_error(FrozenError)
+      end
+    end
   end
 
   describe "rb_str_unlocktmp" do
     it "unlocks a locked string" do
-      str = "test"
+      str = +"test"
       @s.rb_str_locktmp(str)
       @s.rb_str_unlocktmp(str).should == str
       str.upcase!.should == "TEST"
     end
 
     it "raises an error when trying to unlock an already unlocked string" do
-      -> { @s.rb_str_unlocktmp("test") }.should raise_error(RuntimeError, 'temporal unlocking already unlocked string')
+      -> { @s.rb_str_unlocktmp(+"test") }.should raise_error(RuntimeError, 'temporal unlocking already unlocked string')
+    end
+
+    ruby_bug "#20998", ""..."3.5" do
+      it "raises FrozenError if string is frozen" do
+        str = -"rb_str_locktmp"
+        -> { @s.rb_str_unlocktmp(str) }.should raise_error(FrozenError)
+
+        str = +"rb_str_locktmp"
+        str.freeze
+        -> { @s.rb_str_unlocktmp(str) }.should raise_error(FrozenError)
+      end
     end
   end
 

src/main/java/org/truffleruby/cext/CExtNodes.java

@@ -965,7 +965,10 @@ public abstract static class RbStrLockTmpNode extends CoreMethodArrayArgumentsNo
 
         @Specialization
         RubyString rbStrLockTmp(RubyString string,
-                @Cached InlinedBranchProfile errorProfile) {
+                @Cached InlinedBranchProfile errorProfile,
+                @Cached TypeNodes.CheckFrozenNode raiseIfFrozenNode) {
+            raiseIfFrozenNode.execute(this, string);
+
             if (string.locked) {
                 errorProfile.enter(this);
                 throw new RaiseException(getContext(),
@@ -977,8 +980,7 @@ RubyString rbStrLockTmp(RubyString string,
 
         @Specialization
         RubyString rbStrLockTmpImmutable(ImmutableRubyString string) {
-            throw new RaiseException(getContext(),
-                    coreExceptions().runtimeError("temporal locking immutable string", this));
+            throw new RaiseException(getContext(this), coreExceptions(this).frozenError(string, this));
         }
 
     }
@@ -988,7 +990,10 @@ public abstract static class RbStrUnlockTmpNode extends CoreMethodArrayArguments
 
         @Specialization
         RubyString rbStrUnlockTmp(RubyString string,
-                @Cached InlinedBranchProfile errorProfile) {
+                @Cached InlinedBranchProfile errorProfile,
+                @Cached TypeNodes.CheckFrozenNode raiseIfFrozenNode) {
+            raiseIfFrozenNode.execute(this, string);
+
             if (!string.locked) {
                 errorProfile.enter(this);
                 throw new RaiseException(getContext(),
@@ -1000,8 +1005,7 @@ RubyString rbStrUnlockTmp(RubyString string,
 
         @Specialization
         ImmutableRubyString rbStrUnlockTmpImmutable(ImmutableRubyString string) {
-            throw new RaiseException(getContext(),
-                    coreExceptions().runtimeError("temporal unlocking immutable string", this));
+            throw new RaiseException(getContext(this), coreExceptions(this).frozenError(string, this));
         }
 
     }