Skip to content

Commit 0a70aa4

Browse files
bjfishbjfish
committed
Fix Module#const_get to raise a NameError when nested modules do not exist
1 parent ca9a132 commit 0a70aa4

File tree

10 files changed

+85
-31
lines changed

10 files changed

+85
-31
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ Bug fixes:
1515
* Fix a resource leak from allocators defined in C extensions (@aardvark179).
1616
* `SIGINT`/`Interrupt`/`Ctrl+C` now shows the backtrace and exits as signaled, like CRuby (@eregon).
1717
* Update patch feature finding to prefer the longest matching load path (#2605, @bjfish).
18+
* Fix `Module#const_get` to raise a `NameError` when nested modules do not exist (#2610, @bjfish).
1819

1920
Compatibility:
2021

‎lib/truffle/truffle/cext.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1160,7 +1160,7 @@ def rb_define_class_under(mod, name, superclass)
11601160

11611161
def rb_define_module_under(mod, name)
11621162
if Primitive.module_const_defined?(mod, name, false, false)
1163-
val = Primitive.module_const_get(mod, name, false, false)
1163+
val = Primitive.module_const_get(mod, name, false, false, false)
11641164
unless val.class == Module
11651165
raise TypeError, "#{mod}::#{name} is not a module"
11661166
end

‎spec/ruby/.mspec.constants

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ CodingUS_ASCII
3939
CodingUTF_8
4040
ComparisonTest
4141
ConstantSpecsIncludedModule
42+
ConstantSpecsTwo
43+
ConstantSpecsThree
4244
ConstantVisibility
4345
Coverage
4446
CoverageSpecs

‎spec/ruby/core/module/const_get_spec.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,16 @@
100100
ConstantSpecs.const_get("::CS_CONST1").should == :const1
101101
end
102102

103+
it "accepts a toplevel scope qualifier when inherit is false" do
104+
ConstantSpecs.const_get("::CS_CONST1", false).should == :const1
105+
-> { ConstantSpecs.const_get("CS_CONST1", false) }.should raise_error(NameError)
106+
end
107+
108+
it "returns a constant whose module is defined the the toplevel" do
109+
ConstantSpecs.const_get("ConstantSpecsTwo::Foo").should == :cs_two_foo
110+
ConstantSpecsThree.const_get("ConstantSpecsTwo::Foo").should == :cs_three_foo
111+
end
112+
103113
it "accepts a scoped constant name" do
104114
ConstantSpecs.const_get("ClassA::CS_CONST10").should == :const10_10
105115
end
@@ -140,6 +150,10 @@
140150
Object.const_get('CSAutoloadD::InnerModule').name.should == 'CSAutoloadD::InnerModule'
141151
end
142152

153+
it "raises a NameError when the nested constant does not exist on the module but exists in Object" do
154+
-> { Object.const_get('ConstantSpecs::CS_CONST1') }.should raise_error(NameError)
155+
end
156+
143157
describe "with statically assigned constants" do
144158
it "searches the immediate class or module first" do
145159
ConstantSpecs::ClassA.const_get(:CS_CONST10).should == :const10_10

‎spec/ruby/fixtures/constants.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,4 +299,14 @@ class ClassD < ClassC
299299
private_constant :CS_PRIVATE
300300
end
301301

302+
module ConstantSpecsThree
303+
module ConstantSpecsTwo
304+
Foo = :cs_three_foo
305+
end
306+
end
307+
308+
module ConstantSpecsTwo
309+
Foo = :cs_two_foo
310+
end
311+
302312
include ConstantSpecs::ModuleA

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -792,7 +792,7 @@ protected ImmutableRubyString rbStrUnlockTmpImmutable(ImmutableRubyString string
792792
@NodeChild(value = "name", type = RubyNode.class)
793793
public abstract static class RbConstGetNode extends CoreMethodNode {
794794

795-
@Child private LookupConstantNode lookupConstantNode = LookupConstantNode.create(true, false, true);
795+
@Child private LookupConstantNode lookupConstantNode = LookupConstantNode.create(true, true);
796796
@Child private GetConstantNode getConstantNode = GetConstantNode.create();
797797

798798
@CreateCast("name")
@@ -813,7 +813,7 @@ protected Object rbConstGet(RubyModule module, String name) {
813813
@NodeChild(value = "name", type = RubyNode.class)
814814
public abstract static class RbConstGetFromNode extends CoreMethodNode {
815815

816-
@Child private LookupConstantNode lookupConstantNode = LookupConstantNode.create(true, false, false);
816+
@Child private LookupConstantNode lookupConstantNode = LookupConstantNode.create(true, false);
817817
@Child private GetConstantNode getConstantNode = GetConstantNode.create();
818818

819819
@CreateCast("name")

‎src/main/java/org/truffleruby/core/module/ModuleNodes.java

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -965,12 +965,14 @@ protected boolean isConstDefined(RubyModule module, String fullName, boolean inh
965965
@Primitive(name = "module_const_get")
966966
@NodeChild(value = "module", type = RubyNode.class)
967967
@NodeChild(value = "name", type = RubyBaseNodeWithExecute.class)
968-
@NodeChild(value = "inherit", type = RubyBaseNodeWithExecute.class)
968+
@NodeChild(value = "inherit", type = RubyNode.class)
969+
@NodeChild(value = "look_in_object", type = RubyNode.class)
969970
@NodeChild(value = "check_name", type = RubyNode.class)
970971
@ImportStatic({ StringCachingGuards.class, StringOperations.class })
971972
public abstract static class ConstGetNode extends PrimitiveNode {
972973

973-
@Child private LookupConstantNode lookupConstantNode = LookupConstantNode.create(true, true, true);
974+
@Child private LookupConstantNode lookupConstantLookInObjectNode = LookupConstantNode.create(true, true);
975+
@Child private LookupConstantNode lookupConstantNode = LookupConstantNode.create(true, false);
974976
@Child private GetConstantNode getConstantNode = GetConstantNode.create();
975977

976978
@CreateCast("name")
@@ -979,20 +981,17 @@ protected RubyBaseNodeWithExecute coerceToSymbolOrString(RubyBaseNodeWithExecute
979981
return ToStringOrSymbolNodeGen.create(name);
980982
}
981983

982-
@CreateCast("inherit")
983-
protected RubyBaseNodeWithExecute coerceToBoolean(RubyBaseNodeWithExecute inherit) {
984-
return BooleanCastWithDefaultNode.create(true, inherit);
985-
}
986-
987984
// Symbol
988985

989986
@Specialization(guards = "inherit")
990-
protected Object getConstant(RubyModule module, RubySymbol name, boolean inherit, boolean checkName) {
991-
return getConstant(module, name.getString(), checkName);
987+
protected Object getConstant(
988+
RubyModule module, RubySymbol name, boolean inherit, boolean lookInObject, boolean checkName) {
989+
return getConstant(module, name.getString(), checkName, lookInObject);
992990
}
993991

994992
@Specialization(guards = "!inherit")
995-
protected Object getConstantNoInherit(RubyModule module, RubySymbol name, boolean inherit, boolean checkName) {
993+
protected Object getConstantNoInherit(
994+
RubyModule module, RubySymbol name, boolean inherit, boolean lookInObject, boolean checkName) {
996995
return getConstantNoInherit(module, name.getString(), checkName);
997996
}
998997

@@ -1006,41 +1005,53 @@ protected Object getConstantNoInherit(RubyModule module, RubySymbol name, boolea
10061005
"!scoped",
10071006
"checkName == cachedCheckName" },
10081007
limit = "getLimit()")
1009-
protected Object getConstantStringCached(RubyModule module, Object name, boolean inherit, boolean checkName,
1008+
protected Object getConstantStringCached(
1009+
RubyModule module, Object name, boolean inherit, boolean lookInObject, boolean checkName,
10101010
@CachedLibrary(limit = "LIBSTRING_CACHE") RubyStringLibrary stringsName,
10111011
@Cached("stringsName.getRope(name)") Rope cachedRope,
10121012
@Cached("stringsName.getJavaString(name)") String cachedString,
10131013
@Cached("checkName") boolean cachedCheckName,
10141014
@Cached RopeNodes.EqualNode equalNode,
10151015
@Cached("isScoped(cachedString)") boolean scoped) {
1016-
return getConstant(module, cachedString, checkName);
1016+
return getConstant(module, cachedString, checkName, lookInObject);
10171017
}
10181018

10191019
@Specialization(
10201020
guards = { "stringsName.isRubyString(name)", "inherit", "!isScoped(stringsName.getRope(name))" },
10211021
replaces = "getConstantStringCached")
1022-
protected Object getConstantString(RubyModule module, Object name, boolean inherit, boolean checkName,
1022+
protected Object getConstantString(
1023+
RubyModule module, Object name, boolean inherit, boolean lookInObject, boolean checkName,
10231024
@CachedLibrary(limit = "LIBSTRING_CACHE") RubyStringLibrary stringsName) {
1024-
return getConstant(module, stringsName.getJavaString(name), checkName);
1025+
return getConstant(module, stringsName.getJavaString(name), checkName, lookInObject);
10251026
}
10261027

10271028
@Specialization(
10281029
guards = { "stringsName.isRubyString(name)", "!inherit", "!isScoped(stringsName.getRope(name))" })
1029-
protected Object getConstantNoInheritString(RubyModule module, Object name, boolean inherit, boolean checkName,
1030+
protected Object getConstantNoInheritString(
1031+
RubyModule module, Object name, boolean inherit, boolean lookInObject, boolean checkName,
10301032
@CachedLibrary(limit = "LIBSTRING_CACHE") RubyStringLibrary stringsName) {
10311033
return getConstantNoInherit(module, stringsName.getJavaString(name), checkName);
10321034
}
10331035

10341036
// Scoped String
10351037
@Specialization(guards = { "stringsName.isRubyString(name)", "isScoped(stringsName.getRope(name))" })
1036-
protected Object getConstantScoped(RubyModule module, Object name, boolean inherit, boolean checkName,
1038+
protected Object getConstantScoped(
1039+
RubyModule module, Object name, boolean inherit, boolean lookInObject, boolean checkName,
10371040
@CachedLibrary(limit = "LIBSTRING_CACHE") RubyStringLibrary stringsName) {
10381041
return FAILURE;
10391042
}
10401043

1041-
private Object getConstant(RubyModule module, String name, boolean checkName) {
1042-
return getConstantNode
1043-
.lookupAndResolveConstant(LexicalScope.IGNORE, module, name, checkName, lookupConstantNode);
1044+
private Object getConstant(RubyModule module, String name, boolean checkName, boolean lookInObject) {
1045+
CompilerAsserts.partialEvaluationConstant(lookInObject);
1046+
if (lookInObject) {
1047+
return getConstantNode
1048+
.lookupAndResolveConstant(LexicalScope.IGNORE, module, name, checkName,
1049+
lookupConstantLookInObjectNode);
1050+
} else {
1051+
return getConstantNode
1052+
.lookupAndResolveConstant(LexicalScope.IGNORE, module, name, checkName,
1053+
lookupConstantNode);
1054+
}
10441055
}
10451056

10461057
private Object getConstantNoInherit(RubyModule module, String name, boolean checkName) {

‎src/main/java/org/truffleruby/language/constants/LookupConstantNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public abstract class LookupConstantNode extends LookupConstantBaseNode implemen
3131
private final boolean ignoreVisibility;
3232
private final boolean lookInObject;
3333

34-
public static LookupConstantNode create(boolean ignoreVisibility, boolean checkName, boolean lookInObject) {
34+
public static LookupConstantNode create(boolean ignoreVisibility, boolean lookInObject) {
3535
return LookupConstantNodeGen.create(ignoreVisibility, lookInObject);
3636
}
3737

‎src/main/java/org/truffleruby/language/constants/ReadConstantNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ private GetConstantNode getGetConstantNode() {
6666
private LookupConstantNode getLookupConstantNode() {
6767
if (lookupConstantNode == null) {
6868
CompilerDirectives.transferToInterpreterAndInvalidate();
69-
lookupConstantNode = insert(LookupConstantNode.create(false, true, false));
69+
lookupConstantNode = insert(LookupConstantNode.create(false, false));
7070
}
7171
return lookupConstantNode;
7272
}

‎src/main/ruby/truffleruby/core/module.rb

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -104,20 +104,36 @@ def const_defined?(name, inherit = true)
104104
end
105105

106106
def const_get(name, inherit = true)
107-
value = Primitive.module_const_get self, name, inherit, true
107+
inherit = Primitive.as_boolean(inherit)
108+
value = Primitive.module_const_get self, name, inherit, true, true
108109
unless Primitive.undefined?(value)
109110
return value
110111
end
111112

112113
names = name.split('::') # name is always String
113-
unless names.empty?
114-
names.shift if '' == names.first
115-
end
114+
top_level = if !names.empty? && '' == names.first
115+
names.shift
116+
true
117+
else
118+
false
119+
end
116120
raise NameError, "wrong constant name #{name}" if names.empty? || names.include?('')
117-
res = self
118-
names.each do |s|
121+
122+
res = if top_level
123+
Object
124+
else
125+
self
126+
end
127+
128+
names.each_with_index do |s, i|
119129
if Primitive.object_kind_of?(res, Module)
120-
res = res.const_get(s, inherit)
130+
res = if !inherit
131+
Primitive.module_const_get res, s, false, false, true
132+
elsif i == 0
133+
Primitive.module_const_get res, s, true, true, true
134+
else
135+
Primitive.module_const_get res, s, true, false, true
136+
end
121137
else
122138
raise TypeError, "#{name} does not refer to a class/module"
123139
end

0 commit comments

Comments
 (0)