Skip to content

Commit 8f8f131

Browse files
committed
Fix Kernel#freeze to also freeze object's singleton class
1 parent 8033925 commit 8f8f131

File tree

6 files changed

+86
-2
lines changed

6 files changed

+86
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Bug fixes:
99
* Fixed return value of `Enumerable#first` (#2056, @LillianZ).
1010
* Improve reliability of the post install hook by disabling RubyGems (#2075).
1111
* Fixed top level exception handler to print exception cause (#2013).
12+
* Fixed issue with `Kernel#freeze` not freezing singleton class (#2093).
1213

1314
Compatibility:
1415

spec/ruby/core/kernel/freeze_spec.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,12 @@ def mutate; @foo = 1; end
8080
o.freeze
8181
-> {o.instance_variable_set(:@foo, 1)}.should raise_error(RuntimeError)
8282
end
83+
84+
it "freezes an object's singleton class" do
85+
o = Object.new
86+
c = o.singleton_class
87+
c.frozen?.should == false
88+
o.freeze
89+
c.frozen?.should == true
90+
end
8391
end

spec/ruby/language/def_spec.rb

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,22 @@ def foo(a, b); end
8989
def foo(a); end
9090
-> { foo 1, 2 }.should raise_error(ArgumentError, 'wrong number of arguments (given 2, expected 1)')
9191
end
92+
93+
it "raises FrozenError with the correct class name" do
94+
-> { Module.new do
95+
self.freeze
96+
def foo; end
97+
end }.should raise_error(FrozenError){ |e|
98+
e.message.should == "can't modify frozen module"
99+
}
100+
101+
-> { Class.new do
102+
self.freeze
103+
def foo; end
104+
end }.should raise_error(FrozenError){ |e|
105+
e.message.should == "can't modify frozen class"
106+
}
107+
end
92108
end
93109

94110
describe "An instance method definition with a splat" do
@@ -266,6 +282,25 @@ def obj.==(other)
266282
obj.freeze
267283
-> { def obj.foo; end }.should raise_error(FrozenError)
268284
end
285+
286+
it "raises FrozenError with the correct class name" do
287+
obj = Object.new
288+
obj.freeze
289+
-> { def obj.foo; end }.should raise_error(FrozenError){ |e|
290+
e.message.should == "can't modify frozen object"
291+
}
292+
293+
c = obj.singleton_class
294+
-> { def c.foo; end }.should raise_error(FrozenError){ |e|
295+
e.message.should == "can't modify frozen Class"
296+
}
297+
298+
m = Module.new
299+
m.freeze
300+
-> { def m.foo; end }.should raise_error(FrozenError){ |e|
301+
e.message.should == "can't modify frozen Module"
302+
}
303+
end
269304
end
270305

271306
describe "Redefining a singleton method" do

spec/tags/language/def_tags.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
fails:An instance method raises FrozenError with the correct class name
2+
fails:A singleton method definition raises FrozenError with the correct class name

src/main/java/org/truffleruby/core/kernel/KernelNodes.java

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
import org.truffleruby.language.RubyContextNode;
8585
import org.truffleruby.language.RubyContextSourceNode;
8686
import org.truffleruby.language.RubyDynamicObject;
87+
import org.truffleruby.language.RubyGuards;
8788
import org.truffleruby.language.RubyNode;
8889
import org.truffleruby.language.RubyRootNode;
8990
import org.truffleruby.language.RubySourceNode;
@@ -846,13 +847,29 @@ protected int getCacheLimit() {
846847
@CoreMethod(names = "freeze")
847848
public abstract static class KernelFreezeNode extends CoreMethodArrayArgumentsNode {
848849

849-
@Specialization(limit = "getRubyLibraryCacheLimit()")
850+
@Specialization(limit = "getRubyLibraryCacheLimit()", guards = "!isRubyDynamicObject(self)")
850851
protected Object freeze(Object self,
851852
@CachedLibrary("self") RubyLibrary rubyLibrary) {
852853
rubyLibrary.freeze(self);
853854
return self;
854855
}
855856

857+
@Specialization(limit = "getRubyLibraryCacheLimit()", guards = "isRubyDynamicObject(self)")
858+
protected Object freezeDynamicObject(Object self,
859+
@CachedLibrary("self") RubyLibrary rubyLibrary,
860+
@Cached("createBinaryProfile()") ConditionProfile singletonProfile,
861+
@Cached MetaClassNode metaClassNode) {
862+
final RubyClass metaClass = metaClassNode.execute(self);
863+
if (singletonProfile.profile(metaClass.isSingleton &&
864+
!(RubyGuards.isRubyClass(self) && ((RubyClass) self).isSingleton))) {
865+
if (!RubyLibrary.getUncached().isFrozen(metaClass)) {
866+
RubyLibrary.getUncached().freeze(metaClass);
867+
}
868+
}
869+
rubyLibrary.freeze(self);
870+
return self;
871+
}
872+
856873
}
857874

858875
@ReportPolymorphism

src/main/java/org/truffleruby/core/module/ModuleFields.java

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.truffleruby.core.klass.RubyClass;
3030
import org.truffleruby.core.method.MethodFilter;
3131
import org.truffleruby.core.string.RubyString;
32+
import org.truffleruby.core.string.StringUtils;
3233
import org.truffleruby.core.symbol.RubySymbol;
3334
import org.truffleruby.language.RubyConstant;
3435
import org.truffleruby.language.RubyDynamicObject;
@@ -204,7 +205,27 @@ public void initCopy(RubyModule from) {
204205

205206
public void checkFrozen(RubyContext context, Node currentNode) {
206207
if (context.getCoreLibrary() != null && RubyLibrary.getUncached().isFrozen(rubyModule)) {
207-
throw new RaiseException(context, context.getCoreExceptions().frozenError(rubyModule, currentNode));
208+
String name;
209+
if (rubyModule instanceof RubyClass) {
210+
final RubyClass cls = (RubyClass) rubyModule;
211+
name = "object";
212+
if (cls.isSingleton) {
213+
if (cls.attached instanceof RubyClass) {
214+
name = "Class";
215+
} else if (cls.attached instanceof RubyModule) {
216+
name = "Module";
217+
}
218+
} else {
219+
name = "class";
220+
}
221+
} else {
222+
name = "module";
223+
}
224+
throw new RaiseException(
225+
context,
226+
context.getCoreExceptions().frozenError(
227+
StringUtils.format("can't modify frozen %s", name),
228+
currentNode));
208229
}
209230
}
210231

0 commit comments

Comments
 (0)