summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlan Wu <XrXr@users.noreply.github.com>2021-06-17 17:47:11 -0400
committerNobuyoshi Nakada <nobu@ruby-lang.org>2021-06-30 10:49:27 +0900
commit3dd3ea092acead6179033f2c95525ffc5b8bb6ff (patch)
treed46ba269e49deb686ff0069755f514dcbe2035e8
parentdcd1eedba7af06cdb7f81f1fc4866088665c9d99 (diff)
Use Module#ancestors order in recursive constant lookup
Before this commit, const_get with inherit=true and constant lookup expressions searched the ancestors of the starting point in an order different from `starting_point.ancestors`. Items in the ancestry list introduced through prepend were searched after searching the module they were prepended into. This oddity allowed for situations where constant lookups gave different results even though `starting_point.ancestors` is the same. Do the lookup in the same order as `starting_point.ancestors` by skipping classes and modules that have an origin iclass. The origin iclass is in the super chain after the prepended modules. Note that just like before this commit, the starting point of the constant lookup is always the first item that we search, regardless of the presence of any prepended modules. [Bug #17887]
Notes
Notes: Merged: https://github.com/ruby/ruby/pull/4585
-rw-r--r--test/ruby/test_module.rb55
-rw-r--r--variable.c24
2 files changed, 74 insertions, 5 deletions
diff --git a/test/ruby/test_module.rb b/test/ruby/test_module.rb
index 3411c3d701..2d7bdb47fc 100644
--- a/test/ruby/test_module.rb
+++ b/test/ruby/test_module.rb
@@ -2890,6 +2890,61 @@ class TestModule < Test::Unit::TestCase
}
end
+ def test_prepend_constant_lookup
+ m = Module.new do
+ const_set(:C, :m)
+ end
+ c = Class.new do
+ const_set(:C, :c)
+ prepend m
+ end
+ sc = Class.new(c)
+ # Situation from [Bug #17887]
+ assert_equal(sc.ancestors.take(3), [sc, m, c])
+ assert_equal(:m, sc.const_get(:C))
+ assert_equal(:m, sc::C)
+
+ assert_equal(:c, c::C)
+
+ m.send(:remove_const, :C)
+ assert_equal(:c, sc.const_get(:C))
+ assert_equal(:c, sc::C)
+
+ # Same ancestors, built with include instead of prepend
+ m = Module.new do
+ const_set(:C, :m)
+ end
+ c = Class.new do
+ const_set(:C, :c)
+ end
+ sc = Class.new(c) do
+ include m
+ end
+
+ assert_equal(sc.ancestors.take(3), [sc, m, c])
+ assert_equal(:m, sc.const_get(:C))
+ assert_equal(:m, sc::C)
+
+ m.send(:remove_const, :C)
+ assert_equal(:c, sc.const_get(:C))
+ assert_equal(:c, sc::C)
+
+ # Situation from [Bug #17887], but with modules
+ m = Module.new do
+ const_set(:C, :m)
+ end
+ m2 = Module.new do
+ const_set(:C, :m2)
+ prepend m
+ end
+ c = Class.new do
+ include m2
+ end
+ assert_equal(c.ancestors.take(3), [c, m, m2])
+ assert_equal(:m, c.const_get(:C))
+ assert_equal(:m, c::C)
+ end
+
def test_inspect_segfault
bug_10282 = '[ruby-core:65214] [Bug #10282]'
assert_separately [], "#{<<~"begin;"}\n#{<<~'end;'}"
diff --git a/variable.c b/variable.c
index eef19ec237..0e442241ea 100644
--- a/variable.c
+++ b/variable.c
@@ -2569,16 +2569,31 @@ rb_const_get_0(VALUE klass, ID id, int exclude, int recurse, int visibility)
static VALUE
rb_const_search_from(VALUE klass, ID id, int exclude, int recurse, int visibility)
{
- VALUE value, tmp;
+ VALUE value, current;
+ bool first_iteration = true;
- tmp = klass;
- while (RTEST(tmp)) {
+ for (current = klass;
+ RTEST(current);
+ current = RCLASS_SUPER(current), first_iteration = false) {
+ VALUE tmp;
VALUE am = 0;
rb_const_entry_t *ce;
+ if (!first_iteration && RCLASS_ORIGIN(current) != current) {
+ // This item in the super chain has an origin iclass
+ // that comes later in the chain. Skip this item so
+ // prepended modules take precedence.
+ continue;
+ }
+
+ // Do lookup in original class or module in case we are at an origin
+ // iclass in the chain.
+ tmp = current;
+ if (BUILTIN_TYPE(tmp) == T_ICLASS) tmp = RBASIC(tmp)->klass;
+
+ // Do the lookup. Loop in case of autoload.
while ((ce = rb_const_lookup(tmp, id))) {
if (visibility && RB_CONST_PRIVATE_P(ce)) {
- if (BUILTIN_TYPE(tmp) == T_ICLASS) tmp = RBASIC(tmp)->klass;
GET_EC()->private_const_reference = tmp;
return Qundef;
}
@@ -2599,7 +2614,6 @@ rb_const_search_from(VALUE klass, ID id, int exclude, int recurse, int visibilit
return value;
}
if (!recurse) break;
- tmp = RCLASS_SUPER(tmp);
}
not_found: