Skip to content

Commit 1c888c9

Browse files
bjfishbjfish
committed
[GR-18163] Fix Module#const_get to raise a NameError when nested modules do not exist
PullRequest: truffleruby/3220
2 parents 44a5bf5 + 0a70aa4 commit 1c888c9

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
@@ -19,6 +19,7 @@ Bug fixes:
1919
* Fix `Array#pack` with `x*` to not output null characters (#2614, @bjfish).
2020
* Fix `Random#rand` not returning random floats when given float ranges (#2612, @bjfish).
2121
* Fix `Array#sample` for `[]` when called without `n` and a `Random` is given (#2612, @bjfish).
22+
* Fix `Module#const_get` to raise a `NameError` when nested modules do not exist (#2610, @bjfish).
2223

2324
Compatibility:
2425

‎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
@@ -793,7 +793,7 @@ protected ImmutableRubyString rbStrUnlockTmpImmutable(ImmutableRubyString string
793793
@NodeChild(value = "name", type = RubyNode.class)
794794
public abstract static class RbConstGetNode extends CoreMethodNode {
795795

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

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

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

820820
@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
@@ -968,12 +968,14 @@ protected boolean isConstDefined(RubyModule module, String fullName, boolean inh
968968
@Primitive(name = "module_const_get")
969969
@NodeChild(value = "module", type = RubyNode.class)
970970
@NodeChild(value = "name", type = RubyBaseNodeWithExecute.class)
971-
@NodeChild(value = "inherit", type = RubyBaseNodeWithExecute.class)
971+
@NodeChild(value = "inherit", type = RubyNode.class)
972+
@NodeChild(value = "look_in_object", type = RubyNode.class)
972973
@NodeChild(value = "check_name", type = RubyNode.class)
973974
@ImportStatic({ StringCachingGuards.class, StringOperations.class })
974975
public abstract static class ConstGetNode extends PrimitiveNode {
975976

976-
@Child private LookupConstantNode lookupConstantNode = LookupConstantNode.create(true, true, true);
977+
@Child private LookupConstantNode lookupConstantLookInObjectNode = LookupConstantNode.create(true, true);
978+
@Child private LookupConstantNode lookupConstantNode = LookupConstantNode.create(true, false);
977979
@Child private GetConstantNode getConstantNode = GetConstantNode.create();
978980

979981
@CreateCast("name")
@@ -982,20 +984,17 @@ protected RubyBaseNodeWithExecute coerceToSymbolOrString(RubyBaseNodeWithExecute
982984
return ToStringOrSymbolNodeGen.create(name);
983985
}
984986

985-
@CreateCast("inherit")
986-
protected RubyBaseNodeWithExecute coerceToBoolean(RubyBaseNodeWithExecute inherit) {
987-
return BooleanCastWithDefaultNode.create(true, inherit);
988-
}
989-
990987
// Symbol
991988

992989
@Specialization(guards = "inherit")
993-
protected Object getConstant(RubyModule module, RubySymbol name, boolean inherit, boolean checkName) {
994-
return getConstant(module, name.getString(), checkName);
990+
protected Object getConstant(
991+
RubyModule module, RubySymbol name, boolean inherit, boolean lookInObject, boolean checkName) {
992+
return getConstant(module, name.getString(), checkName, lookInObject);
995993
}
996994

997995
@Specialization(guards = "!inherit")
998-
protected Object getConstantNoInherit(RubyModule module, RubySymbol name, boolean inherit, boolean checkName) {
996+
protected Object getConstantNoInherit(
997+
RubyModule module, RubySymbol name, boolean inherit, boolean lookInObject, boolean checkName) {
999998
return getConstantNoInherit(module, name.getString(), checkName);
1000999
}
10011000

@@ -1009,41 +1008,53 @@ protected Object getConstantNoInherit(RubyModule module, RubySymbol name, boolea
10091008
"!scoped",
10101009
"checkName == cachedCheckName" },
10111010
limit = "getLimit()")
1012-
protected Object getConstantStringCached(RubyModule module, Object name, boolean inherit, boolean checkName,
1011+
protected Object getConstantStringCached(
1012+
RubyModule module, Object name, boolean inherit, boolean lookInObject, boolean checkName,
10131013
@CachedLibrary(limit = "LIBSTRING_CACHE") RubyStringLibrary stringsName,
10141014
@Cached("stringsName.getRope(name)") Rope cachedRope,
10151015
@Cached("stringsName.getJavaString(name)") String cachedString,
10161016
@Cached("checkName") boolean cachedCheckName,
10171017
@Cached RopeNodes.EqualNode equalNode,
10181018
@Cached("isScoped(cachedString)") boolean scoped) {
1019-
return getConstant(module, cachedString, checkName);
1019+
return getConstant(module, cachedString, checkName, lookInObject);
10201020
}
10211021

10221022
@Specialization(
10231023
guards = { "stringsName.isRubyString(name)", "inherit", "!isScoped(stringsName.getRope(name))" },
10241024
replaces = "getConstantStringCached")
1025-
protected Object getConstantString(RubyModule module, Object name, boolean inherit, boolean checkName,
1025+
protected Object getConstantString(
1026+
RubyModule module, Object name, boolean inherit, boolean lookInObject, boolean checkName,
10261027
@CachedLibrary(limit = "LIBSTRING_CACHE") RubyStringLibrary stringsName) {
1027-
return getConstant(module, stringsName.getJavaString(name), checkName);
1028+
return getConstant(module, stringsName.getJavaString(name), checkName, lookInObject);
10281029
}
10291030

10301031
@Specialization(
10311032
guards = { "stringsName.isRubyString(name)", "!inherit", "!isScoped(stringsName.getRope(name))" })
1032-
protected Object getConstantNoInheritString(RubyModule module, Object name, boolean inherit, boolean checkName,
1033+
protected Object getConstantNoInheritString(
1034+
RubyModule module, Object name, boolean inherit, boolean lookInObject, boolean checkName,
10331035
@CachedLibrary(limit = "LIBSTRING_CACHE") RubyStringLibrary stringsName) {
10341036
return getConstantNoInherit(module, stringsName.getJavaString(name), checkName);
10351037
}
10361038

10371039
// Scoped String
10381040
@Specialization(guards = { "stringsName.isRubyString(name)", "isScoped(stringsName.getRope(name))" })
1039-
protected Object getConstantScoped(RubyModule module, Object name, boolean inherit, boolean checkName,
1041+
protected Object getConstantScoped(
1042+
RubyModule module, Object name, boolean inherit, boolean lookInObject, boolean checkName,
10401043
@CachedLibrary(limit = "LIBSTRING_CACHE") RubyStringLibrary stringsName) {
10411044
return FAILURE;
10421045
}
10431046

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

10491060
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)