diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c546cd5a0ad..b01906c76ef8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ New features: Bug fixes: +* Fix `Module#name` called inside the `Module#const_added` callback when the module is defined in the top-level scope (#3683, @andrykonchin). + Compatibility: * Fix `Module#include` so a module included into a reopened nested module is added into an ancestors chain (#3570, @andrykonchin). diff --git a/spec/ruby/core/module/fixtures/name.rb b/spec/ruby/core/module/fixtures/name.rb index fb9e66c309e3..25c74d3944a9 100644 --- a/spec/ruby/core/module/fixtures/name.rb +++ b/spec/ruby/core/module/fixtures/name.rb @@ -7,4 +7,7 @@ def name Cß.name end end + + module NameSpecs + end end diff --git a/spec/ruby/core/module/name_spec.rb b/spec/ruby/core/module/name_spec.rb index 0d1f4e24d587..2d946a13a9bd 100644 --- a/spec/ruby/core/module/name_spec.rb +++ b/spec/ruby/core/module/name_spec.rb @@ -140,6 +140,47 @@ module ModuleSpecs::Anonymous valid_names.should include(m::N.name) # You get one of the two, but you don't know which one. end + ruby_version_is "3.2" do + it "is set in #const_added callback when a module defined in the top-level scope" do + ruby_exe(<<~RUBY, args: "2>&1").chomp.should == "TEST1\nTEST2" + class Module + def const_added(name) + puts const_get(name).name + end + end + + # module with name + module TEST1 + end + + # anonymous module + TEST2 = Module.new + RUBY + end + + it "is set in #const_added callback for a nested module when an outer module defined in the top-level scope" do + ScratchPad.record [] + + ModuleSpecs::NameSpecs::NamedModule = Module.new do + def self.const_added(name) + ScratchPad << const_get(name).name + end + + module self::A + def self.const_added(name) + ScratchPad << const_get(name).name + end + + module self::B + end + end + end + + ScratchPad.recorded.should.one?(/#::A$/) + ScratchPad.recorded.should.one?(/#::A::B$/) + end + end + it "returns a frozen String" do ModuleSpecs.name.should.frozen? end diff --git a/spec/tags/core/module/name_tags.txt b/spec/tags/core/module/name_tags.txt index 6d3da7fc7f6e..96af9f248fd8 100644 --- a/spec/tags/core/module/name_tags.txt +++ b/spec/tags/core/module/name_tags.txt @@ -1 +1,3 @@ fails:Module#name is not nil when assigned to a constant in an anonymous module +fails:Module#name is set in #const_added callback for a nested module when an outer module defined in the top-level scope +slow:Module#name is set in #const_added callback when a module defined in the top-level scope diff --git a/src/main/java/org/truffleruby/core/module/ModuleFields.java b/src/main/java/org/truffleruby/core/module/ModuleFields.java index 458e37b68b96..ddeb79c76b5a 100644 --- a/src/main/java/org/truffleruby/core/module/ModuleFields.java +++ b/src/main/java/org/truffleruby/core/module/ModuleFields.java @@ -172,19 +172,8 @@ public RubyConstant getAdoptedByLexicalParent( Node currentNode) { assert name != null; - RubyConstant previous = lexicalParent.fields.setConstantInternal( - context, - currentNode, - name, - rubyModule, - false); - if (!hasFullName()) { - // Tricky, we need to compare with the Object class, but we only have a Class at hand. - final RubyClass classClass = getLogicalClass().getLogicalClass(); - final RubyClass objectClass = (RubyClass) ((RubyClass) classClass.superclass).superclass; - - if (lexicalParent == objectClass) { + if (lexicalParent == getObjectClass()) { this.setFullName(name); updateAnonymousChildrenModules(context); } else if (lexicalParent.fields.hasFullName()) { @@ -196,6 +185,17 @@ public RubyConstant getAdoptedByLexicalParent( // or we'll compute an anonymous name on #getName() if needed } } + + // A module correct final name should be assigned by this time (by #setFullName() method call). + // So a temporary anonymous module name isn't visible in the Module#const_added callback, that + // is called inside #setConstantInternal(). + RubyConstant previous = lexicalParent.fields.setConstantInternal( + context, + currentNode, + name, + rubyModule, + false); + return previous; } @@ -792,7 +792,11 @@ public Object getRubyStringName() { @TruffleBoundary private String createAnonymousName() { if (givenBaseName != null) { - return lexicalParent.fields.getName() + "::" + givenBaseName; + if (lexicalParent == getObjectClass()) { + return givenBaseName; + } else { + return lexicalParent.fields.getName() + "::" + givenBaseName; + } } else if (getLogicalClass() == rubyModule) { // For the case of class Class during initialization return "#"; } else if (RubyGuards.isSingletonClass(rubyModule)) { @@ -1181,4 +1185,12 @@ private void refinedMethod(RubyContext context, String methodName, Node currentN } } + private RubyClass getObjectClass() { + // Tricky, we need to compare with the Object class, but we only have a Module at hand. + final RubyClass classClass = getLogicalClass().getLogicalClass(); + final RubyClass objectClass = (RubyClass) ((RubyClass) classClass.superclass).superclass; + assert objectClass.fields.givenBaseName == "Object"; + return objectClass; + } + }