[ruby-core:113554] [Ruby master Bug#19681] The final classpath of partially named modules is sometimes inconsistent once permanently named

Issue #19681 has been updated by Eregon (Benoit Daloze).


Eregon (Benoit Daloze) wrote in #note-8:
> The final/permanent name of a Module should reflect how to access it, and this should hold for e.g. A::B as long as no one does `A.remove_const :B` or `A::B = ...` (or `const_set` obviously).
> Which is AFAIK the only 2 ways to break that (the first is private to discourage using it, the second warns, so it's clear these two should not be used lightly).

@fxn gave me a counter-example:
```ruby
M = Module.new
N = M
Object.send :remove_const, :M
p N.name # => "M"
N::C = Class.new
p N::C.name # => "M::C"
```

So N.name is broken, and that matches what I said.
But N::C.name is broken too, it "inherits" the broken name from its lexical parent.
So the actual semantics are a bit more complex and harder to express.

It could be nice to change the name of a Module once it's no longer a valid way to access it (so do that on remove_const/const_set). Like N.name => `<invalid, was: M>` or so.
But that's a separate issue, although it would help to clarify such edge cases and avoid confusion.

---

For this issue, I think we have 3 alternatives:
1) Intended behavior, not a bug. Pro: more correct for https://bugs.ruby-lang.org/issues/19681#note-8, has been like that since a long time, the example in the description is very edge case. Cons: non-deterministic for such code.
2) Use the basename if, at this point in time, it is a valid way (i.e., does `mod.const_get(basename) == module_being_named`) to refer to that module/class being named.
3) Use the basename, even if the module being named is no longer reachable that way. Cons: incompatible for https://bugs.ruby-lang.org/issues/19681#note-8, it gives an invalid name when there is a valid one.

----------------------------------------
Bug #19681: The final classpath of partially named modules is sometimes inconsistent once permanently named
https://bugs.ruby-lang.org/issues/19681#change-103184

* Author: byroot (Jean Boussier)
* Status: Open
* Priority: Normal
* Backport: 3.0: WONTFIX, 3.1: REQUIRED, 3.2: REQUIRED
----------------------------------------
Reported to me by @fxn 

```ruby
m = Module.new

class m::C; end
p m::C.name # => "#<Module:0x000000010789fbe0>::C"

m::D = m::C
p m::D.name # => "#<Module:0x000000010789fbe0>::C"

M = m
p M::C.name # => "M::D"
```

Expected behavior:

```ruby
p M::C.name # => "M::C"
```

### Reason

When the parent is assigned its permanent classpath, we iterate over its `const_table` to recursively give a permanent name to all the constant it owns.

However, `const_table` is an `id_table` so it doesn't retain the insertion order, which means that if the constant was aliased,
we can no longer distinguish between the original name and its aliases, and whichever comes first in the `const_table` will be used as the permanent name.

### Potential solution

I have a tentative fix for it in https://github.com/ruby/ruby/pull/7829. Instead of relying on the `const_table` key, it extract the original name from the temporary classpath.
It does feel a bit wrong to do a string search in such a place, but it does work.

 



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- [e-mail]
 To unsubscribe send an email to [e-mail]
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/