diff --git a/CHANGELOG.md b/CHANGELOG.md index a20f920c5930..4a98a182b7cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,7 @@ Compatibility: * Emit warning when `Kernel#format` called with excessive arguments (@andrykonchin). * Fix `Integer#ceil` when self is 0 (@andrykonchin). * Fix `Module#remove_const` and emit warning when constant is deprecated (@andrykonchin). +* Add `Module#set_temporary_name` (#3681, @andrykonchin). Performance: diff --git a/spec/ruby/core/module/fixtures/set_temporary_name.rb b/spec/ruby/core/module/fixtures/set_temporary_name.rb new file mode 100644 index 000000000000..901b3b94d122 --- /dev/null +++ b/spec/ruby/core/module/fixtures/set_temporary_name.rb @@ -0,0 +1,4 @@ +module ModuleSpecs + module SetTemporaryNameSpec + end +end diff --git a/spec/ruby/core/module/set_temporary_name_spec.rb b/spec/ruby/core/module/set_temporary_name_spec.rb index f5886a33988e..12541374aec7 100644 --- a/spec/ruby/core/module/set_temporary_name_spec.rb +++ b/spec/ruby/core/module/set_temporary_name_spec.rb @@ -1,4 +1,5 @@ require_relative '../../spec_helper' +require_relative 'fixtures/set_temporary_name' ruby_version_is "3.3" do describe "Module#set_temporary_name" do @@ -13,13 +14,34 @@ m.name.should be_nil end + it "returns self" do + m = Module.new + m.set_temporary_name("fake_name").should.equal? m + end + it "can assign a temporary name which is not a valid constant path" do m = Module.new - m.set_temporary_name("a::B") - m.name.should == "a::B" + + m.set_temporary_name("name") + m.name.should == "name" m.set_temporary_name("Template['foo.rb']") m.name.should == "Template['foo.rb']" + + m.set_temporary_name("a::B") + m.name.should == "a::B" + + m.set_temporary_name("A::b") + m.name.should == "A::b" + + m.set_temporary_name("A::B::") + m.name.should == "A::B::" + + m.set_temporary_name("A::::B") + m.name.should == "A::::B" + + m.set_temporary_name("A=") + m.name.should == "A=" end it "can't assign empty string as name" do @@ -43,7 +65,7 @@ -> { Object.set_temporary_name("fake_name") }.should raise_error(RuntimeError, "can't change permanent name") end - it "can assign a temporary name to a nested module" do + it "can assign a temporary name to a module nested into an anonymous module" do m = Module.new module m::N; end m::N.name.should =~ /\A#::N\z/ @@ -55,6 +77,17 @@ module m::N; end m::N.name.should be_nil end + it "discards a temporary name when an outer anonymous module gets a permanent name" do + m = Module.new + module m::N; end + + m::N.set_temporary_name("fake_name") + m::N.name.should == "fake_name" + + ModuleSpecs::SetTemporaryNameSpec::M = m + m::N.name.should == "ModuleSpecs::SetTemporaryNameSpec::M::N" + end + it "can update the name when assigned to a constant" do m = Module.new m::N = Module.new @@ -64,5 +97,45 @@ module m::N; end m::M = m::N m::M.name.should =~ /\A#::M\z/m end + + it "can reassign a temporary name repeatedly" do + m = Module.new + + m.set_temporary_name("fake_name") + m.name.should == "fake_name" + + m.set_temporary_name("fake_name_2") + m.name.should == "fake_name_2" + end + + it "does not affect a name of a module nested into an anonymous module with a temporary name" do + m = Module.new + m::N = Module.new + m::N.name.should =~ /\A#::N\z/ + + m.set_temporary_name("foo") + m::N.name.should =~ /\A#::N\z/ + end + + it "keeps temporary name when assigned in an anonymous module" do + outer = Module.new + m = Module.new + m.set_temporary_name "m" + m.name.should == "m" + outer::M = m + m.name.should == "m" + m.inspect.should == "m" + end + + it "keeps temporary name when assigned in an anonymous module and nested before" do + outer = Module.new + m = Module.new + outer::A = m + m.set_temporary_name "m" + m.name.should == "m" + outer::M = m + m.name.should == "m" + m.inspect.should == "m" + end end end diff --git a/spec/tags/core/module/set_temporary_name_tags.txt b/spec/tags/core/module/set_temporary_name_tags.txt index 538ad142bcc4..81b108d98e3c 100644 --- a/spec/tags/core/module/set_temporary_name_tags.txt +++ b/spec/tags/core/module/set_temporary_name_tags.txt @@ -1,8 +1,2 @@ -fails:Module#set_temporary_name can assign a temporary name -fails:Module#set_temporary_name can assign a temporary name which is not a valid constant path -fails:Module#set_temporary_name can't assign empty string as name -fails:Module#set_temporary_name can't assign a constant name as a temporary name -fails:Module#set_temporary_name can't assign a constant path as a temporary name -fails:Module#set_temporary_name can't assign name to permanent module -fails:Module#set_temporary_name can assign a temporary name to a nested module fails:Module#set_temporary_name can update the name when assigned to a constant +fails:Module#set_temporary_name does not affect a name of a module nested into an anonymous module with a temporary name diff --git a/src/main/java/org/truffleruby/core/module/ModuleFields.java b/src/main/java/org/truffleruby/core/module/ModuleFields.java index b580605b41f0..9db9a674d9ee 100644 --- a/src/main/java/org/truffleruby/core/module/ModuleFields.java +++ b/src/main/java/org/truffleruby/core/module/ModuleFields.java @@ -90,15 +90,39 @@ public static void debugModuleChain(RubyModule module) { private final PrependMarker start; + /** + * 
+     * Naming modules is complicated, we try to summarize the transitions here.
+     * fully named (simple): module M; end -> module with givenBaseName="M" hasFullName=true
+     * fully named (nested): module N; module M; end; end -> module with name="N::M" hasFullName=true
+     * anonymous: Module.new -> #name is nil, #inspect is #
+     * nested-anonymous: m = Module.new; module m::N; end -> givenBaseName="N" hasFullName=false hasPartialName=true #name is "#::N"
+     *
+     * Transitions:
+     * * anonymous to nested-anonymous, by constant assignment
+     * * anonymous/nested-anonymous to full, by constant assignment (for anonymous), or lexical parent gets fully named (for nested-anonymous)
+     * * temporary to full, by constant assignment or lexical parent gets fully named
+     * * anonymous/nested-anonymous to temporary, by set_temporary_name("...") or set_temporary_name(nil)
+     * * temporary to temporary, by set_temporary_name("...") or set_temporary_name(nil)
+     * They form a lattice/total order, as it's never possible to go back from a transition
+     *
+     * Note there is no temporary to anonymous/nested-anonymous, it just stays temporary in that case.
+     *
+ */ + private final RubyModule lexicalParent; - /** Module (or class) own explicitly specified name */ + /** Module (or class) own explicitly specified name: either `class Foo` or `module Foo` or a core module/class */ public final String givenBaseName; - /** Means whether a fully qualified name (with parent lexical scope names) is already set */ private boolean hasFullName = false; - /** Either fully qualified name or lazily evaluated anonymous name */ + /** Either fully qualified name or lazily evaluated anonymous name, or temporary name */ private String name = null; + /** Same as {@link #name} except null if {@link #hasPartialName()} */ private ImmutableRubyString rubyStringName; + /** A temporary name assigned to an anonymous module */ + private String temporaryName = null; + /** Whether a temporary name is assigned. Is needed to distinguish a case when nil value is assigned. */ + private boolean isTemporaryNameAssigned = false; /** Whether this is a refinement module (R), created by #refine */ private boolean isRefinement = false; @@ -180,7 +204,7 @@ private void nameModule( nameChildrenModules(context); } else { // Our lexicalParent is also an anonymous module - // and will name us when it gets named via updateAnonymousChildrenModules(), + // and will name us when it gets named via nameChildrenModules(), // or we'll compute an anonymous name on #getName() if needed } } @@ -717,11 +741,12 @@ public boolean undefineConstantIfStillAutoload(RubyConstant autoloadConstant) { } public String getName() { - final String name = this.name; + // Lazily compute the anonymous name because it is expensive if (name == null) { - // Lazily compute the anonymous name because it is expensive - return getAnonymousName(); + recomputeAnonymousOrTemporaryName(); } + + assert name != null; return name; } @@ -737,25 +762,48 @@ public String getSimpleName() { } @TruffleBoundary - private String getAnonymousName() { - final String anonymousName = createAnonymousName(); - setName(anonymousName); - return anonymousName; + private void recomputeAnonymousOrTemporaryName() { + if (!isAnonymousOrTemporary()) { + return; + } + + final String newName; + if (temporaryName != null) { + newName = temporaryName; + } else { + newName = createFullyQualifiedAnonymousName(); + } + + setName(newName); } public void setFullName(String name) { assert name != null; hasFullName = true; setName(name); + resetTemporaryName(); } private void setName(String name) { this.name = name; if (hasPartialName()) { this.rubyStringName = language.getFrozenStringLiteral(TStringUtils.utf8TString(name), Encodings.UTF_8); + } else { + this.rubyStringName = null; } } + public void setTemporaryName(String name) { + this.temporaryName = name; + this.isTemporaryNameAssigned = true; + recomputeAnonymousOrTemporaryName(); + } + + private void resetTemporaryName() { + temporaryName = null; + isTemporaryNameAssigned = false; + } + public Object getRubyStringName() { if (hasPartialName()) { if (rubyStringName == null) { @@ -769,7 +817,7 @@ public Object getRubyStringName() { } @TruffleBoundary - private String createAnonymousName() { + private String createFullyQualifiedAnonymousName() { if (givenBaseName != null) { if (lexicalParent == getObjectClass()) { return givenBaseName; @@ -804,11 +852,15 @@ public boolean hasFullName() { return hasFullName; } + /** Whether Module#name Ruby method returns a value other than nil */ public boolean hasPartialName() { + if (isTemporaryNameAssigned) { + return temporaryName != null; + } return hasFullName() || givenBaseName != null; } - public boolean isAnonymous() { + public boolean isAnonymousOrTemporary() { return !this.hasFullName; } diff --git a/src/main/java/org/truffleruby/core/module/ModuleNodes.java b/src/main/java/org/truffleruby/core/module/ModuleNodes.java index af3927d07e20..159d5e549765 100644 --- a/src/main/java/org/truffleruby/core/module/ModuleNodes.java +++ b/src/main/java/org/truffleruby/core/module/ModuleNodes.java @@ -645,7 +645,7 @@ public abstract static class IsAnonymousNode extends PrimitiveArrayArgumentsNode @Specialization boolean isAnonymous(RubyModule module) { - return module.fields.isAnonymous(); + return module.fields.isAnonymousOrTemporary(); } } @@ -2296,6 +2296,48 @@ private RubyModule newRefinementModule(RubyModule namespace, RubyModule moduleTo } + @CoreMethod(names = "set_temporary_name", required = 1) + public abstract static class SetTemporaryNameNode extends CoreMethodArrayArgumentsNode { + + @TruffleBoundary + @Specialization + RubyModule setTemporaryName(RubyModule self, Object name, + @Cached ToJavaStringNode toJavaStringNode) { + if (name == nil) { + self.fields.setTemporaryName(null); + return self; + } + + final String string = toJavaStringNode.execute(this, name); + validateName(string, self); + self.fields.setTemporaryName(string); + + return self; + } + + private void validateName(String name, RubyModule self) { + if (name.isEmpty()) { + throw new RaiseException( + getContext(this), + coreExceptions(this).argumentError("empty class/module name", this)); + } + + if (self.fields.hasFullName()) { + throw new RaiseException( + getContext(this), + coreExceptions(this).runtimeError("can't change permanent name", this)); + } + + if (Identifiers.isValidConstantPath(name)) { + throw new RaiseException( + getContext(this), + coreExceptions(this).argumentError( + "the temporary name must not be a constant path to avoid confusion", this)); + } + } + + } + @GenerateUncached @CoreMethod(names = "using", required = 1, visibility = Visibility.PRIVATE, alwaysInlined = true) public abstract static class ModuleUsingNode extends UsingNode { diff --git a/src/main/java/org/truffleruby/parser/Identifiers.java b/src/main/java/org/truffleruby/parser/Identifiers.java index 1207c8889045..c991cb4c1d6e 100644 --- a/src/main/java/org/truffleruby/parser/Identifiers.java +++ b/src/main/java/org/truffleruby/parser/Identifiers.java @@ -44,6 +44,29 @@ public static boolean isValidConstantName(String id) { return isConstantFirstCodePoint(first) && isNameString(id, Character.charCount(first)); } + /** Check if String is a valid constant path, e.g. A::B or ::A */ + @TruffleBoundary + public static boolean isValidConstantPath(String id) { + boolean isConstantPath = true; + + final String[] pathSections = id.split("::", -1); + for (int i = 0; i < pathSections.length; i++) { + String section = pathSections[i]; + + // is a constant path prefixed with '::' (e.g. '::A::B')? + if (section.isEmpty() && i == 0) { + continue; + } + + if (!isValidConstantName(section)) { + isConstantPath = false; + break; + } + } + + return isConstantPath; + } + @TruffleBoundary public static boolean isValidLocalVariableName(String id) { return isValidIdentifier(id, 0);