Skip to content

Commit

Permalink
Raise RuntimeError for class variable overtaken in nonverbose mode
Browse files Browse the repository at this point in the history
900e83b changed from a warning
to an error in this case, but the warning was only issued in
verbose mode, and therefore the error was only raised in verbose
mode.  That was not intentional, verbose mode should only change
whether warnings are emitted, not other behavior.  This issues
the RuntimeError in all cases.

This change broke a couple tests, as the tests actually issued
the warning and therefore now raise an error.  This wasn't caught
earlier as test_variable suppressed the warning in this case,
effectively setting $VERBOSE = false around the code that warned.
basictest isn't run in verbose mode and therefore didn't expose
the issue previously. Fix these tests.

Fixes [Bug #14541]
  • Loading branch information
jeremyevans committed Jun 18, 2020
1 parent aae8223 commit 95dc9c0
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 8 deletions.
13 changes: 10 additions & 3 deletions basictest/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2137,7 +2137,7 @@ module M003; include M002; end
test_ok($_ == foobar)

class Gods
@@rule = "Uranus" # private to Gods
@@rule = "Uranus"
def ruler0
@@rule
end
Expand All @@ -2160,7 +2160,7 @@ def ruler3
end

class Titans < Gods
@@rule = "Cronus" # do not affect @@rule in Gods
@@rule = "Cronus" # modifies @@rule in Gods
include Olympians
def ruler4
@@rule
Expand All @@ -2175,7 +2175,14 @@ def ruler4
atlas = Titans.new
test_ok(atlas.ruler0 == "Cronus")
test_ok(atlas.ruler3 == "Zeus")
test_ok(atlas.ruler4 == "Cronus")
begin
atlas.ruler4
rescue RuntimeError => e
test_ok(e.message.include?("class variable @@rule of Olympians is overtaken by Gods"))
else
test_ok(false)
end
test_ok(atlas.ruler3 == "Zeus")

test_check "trace"
$x = 1234
Expand Down
6 changes: 2 additions & 4 deletions test/ruby/test_variable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ class Titans < Gods
@@rule = "Cronus" # modifies @@rule in Gods
include Olympians
def ruler4
EnvUtil.suppress_warning {
@@rule
}
@@rule
end
end

Expand Down Expand Up @@ -117,7 +115,7 @@ def test_variable
atlas = Titans.new
assert_equal("Cronus", atlas.ruler0)
assert_equal("Zeus", atlas.ruler3)
assert_equal("Cronus", atlas.ruler4)
assert_raise(RuntimeError) { atlas.ruler4 }
assert_nothing_raised do
class << Gods
defined?(@@rule) && @@rule
Expand Down
2 changes: 1 addition & 1 deletion variable.c
Original file line number Diff line number Diff line change
Expand Up @@ -3105,7 +3105,7 @@ cvar_overtaken(VALUE front, VALUE target, ID id)
if (front && target != front) {
st_data_t did = (st_data_t)id;

if (RTEST(ruby_verbose) && original_module(front) != original_module(target)) {
if (original_module(front) != original_module(target)) {
rb_raise(rb_eRuntimeError,
"class variable % "PRIsVALUE" of %"PRIsVALUE" is overtaken by %"PRIsVALUE"",
ID2SYM(id), rb_class_name(original_module(front)),
Expand Down

0 comments on commit 95dc9c0

Please sign in to comment.