Bug #20948: Constant references incorrectly cached in `module (expr)::Foo`

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: -
• Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN

[mame (Yusuke Endoh)]

module M1
  module Foo
    X = 1
  end
end

module M2
  module Foo
    X = 2
  end
end

[M1, M2].each do
  module it::Foo
    p X #=> expected: prints "1" then "2"; actual: prints "1" twice
  end
end

To be honest, I don't think it's worth slowing down the processor for this, but I create a ticket since I discovered it and since ko1 seemed to have a bit of an idea of how to solve it.

[Eregon (Benoit Daloze)]

There used to be a similar bug a while ago and IIRC some specs were added, @ko1 (Koichi Sasada) maybe you remember it and could link that ticket here?

I think it was constants inside class << expr though, not inside module expr::Foo.

(FWIW, it's already 1\n2 on TruffleRuby which detects this case as non-static constant scope:

$ ruby --experimental-options --ruby.constant-dynamic-lookup-log c.rb
...
[ruby] INFO: start dynamic constant lookup at c.rb:14
[ruby] INFO: dynamic constant lookup at c.rb:15
1
2

)

[jhawthorn (John Hawthorn)]

• Related to Bug #10943: Singleton class expression (class << obj) should make be indivisual namespaces added

[jhawthorn (John Hawthorn)]

• Status changed from Assigned to Closed

Applied in changeset git|91ae69860591f0dfc85571cfbd023dd199e17e73.

---

Use compile-time flag to indicate dynamic CREFs

The inline constant cache previously used RCLASS_SINGLETON_P to detect
"unstable" CREFs that need ic_cref stored and checked on every IC hit.
This caused the class << self pattern to create inline caches which
requires extra checks and can't optimize as well by the JITs.

We can avoid this by replacing the RCLASS_SINGLETON_P check with a
VM_DEFINECLASS_FLAG_DYNAMIC_CREF flag added to the defineclass
instruction at compile time to indicate a dynamic class scope and
specifically avoid setting it for the class << self pattern.

We can apply the same logic to fix dynamic CREF on module (expr)::Foo.
We can say that any class definition which uses only constant references
is stable, or at least as stable as the cref it was declared inside.

[Bug #20948]