Turn class variable warnings into exceptions

This changes the following warnings:

* warning: class variable access from toplevel
* warning: class variable @foo of D is overtaken by C

into RuntimeErrors.  Handle defined?(@@foo) at toplevel
by returning nil instead of raising an exception (the previous
behavior warned before returning nil when defined? was used).

Refactor the specs to avoid the warnings even in older versions.
The specs were checking for the warnings, but the purpose of
the related specs as evidenced from their description is to
test for behavior, not for warnings.

Fixes [Bug #14541]

9 files changed, 37 insertions, 34 deletions

diff --git a/bootstraptest/test_eval.rb b/bootstraptest/test_eval.rb
index fa04323b7f..5d2593c306 100644
--- a/bootstraptest/test_eval.rb
+++ b/bootstraptest/test_eval.rb
@@ -250,7 +250,9 @@ assert_equal 'ok', %q{
 
 assert_equal 'ok', %q{
   begin
-    12.instance_eval { @@a }
+    class A
+      12.instance_eval { @@a }
+    end
   rescue NameError
     :ok
   end
@@ -258,7 +260,9 @@ assert_equal 'ok', %q{
 
 assert_equal 'ok', %q{
   begin
-    12.instance_exec { @@a }
+    class A
+      12.instance_exec { @@a }
+    end
   rescue NameError
     :ok
   end
diff --git a/bootstraptest/test_insns.rb b/bootstraptest/test_insns.rb
index 1269d7d013..cb54d0d3f7 100644
--- a/bootstraptest/test_insns.rb
+++ b/bootstraptest/test_insns.rb
@@ -64,8 +64,8 @@ tests = [
   [ 'setinstancevariable', %q{ @x = true }, ],
   [ 'getinstancevariable', %q{ @x = true; @x }, ],
 
-  [ 'setclassvariable', %q{ @@x = true }, ],
-  [ 'getclassvariable', %q{ @@x = true; @@x }, ],
+  [ 'setclassvariable', %q{ class A; @@x = true; end }, ],
+  [ 'getclassvariable', %q{ class A; @@x = true; @@x end }, ],
 
   [ 'setconstant', %q{ X = true }, ],
   [ 'setconstant', %q{ Object::X = true }, ],
diff --git a/bootstraptest/test_syntax.rb b/bootstraptest/test_syntax.rb
index a111990a1f..fa27bf2aeb 100644
--- a/bootstraptest/test_syntax.rb
+++ b/bootstraptest/test_syntax.rb
@@ -268,8 +268,10 @@ assert_equal %q{}, %q{
   defined?(@@a)
 }
 assert_equal %q{class variable}, %q{
-  @@a = 1
-  defined?(@@a)
+  class A
+    @@a = 1
+    defined?(@@a)
+  end
 }
 assert_equal %q{}, %q{
   defined?($a)
diff --git a/insns.def b/insns.def
index aab5cca065..58349b0ccc 100644
--- a/insns.def
+++ b/insns.def
@@ -236,7 +236,7 @@ getclassvariable
 /* "class variable access from toplevel" warning can be hooked. */
 // attr bool leaf = false; /* has rb_warning() */
 {
-    val = rb_cvar_get(vm_get_cvar_base(vm_get_cref(GET_EP()), GET_CFP()), id);
+    val = rb_cvar_get(vm_get_cvar_base(vm_get_cref(GET_EP()), GET_CFP(), 1), id);
 }
 
 /* Set value of class variable id of klass as val. */
@@ -249,7 +249,7 @@ setclassvariable
 // attr bool leaf = false; /* has rb_warning() */
 {
     vm_ensure_not_refinement_module(GET_SELF());
-    rb_cvar_set(vm_get_cvar_base(vm_get_cref(GET_EP()), GET_CFP()), id, val);
+    rb_cvar_set(vm_get_cvar_base(vm_get_cref(GET_EP()), GET_CFP(), 1), id, val);
 }
 
 /* Get constant variable id. If klass is Qnil and allow_nil is Qtrue, constants
diff --git a/spec/ruby/core/exception/name_spec.rb b/spec/ruby/core/exception/name_spec.rb
index d1def51f24..c8a49b40e2 100644
--- a/spec/ruby/core/exception/name_spec.rb
+++ b/spec/ruby/core/exception/name_spec.rb
@@ -21,9 +21,7 @@ describe "NameError#name" do
 
   it "returns a class variable name as a symbol" do
     -> {
-      -> {
-        @@doesnt_exist
-      }.should complain(/class variable access from toplevel/)
+      eval("class singleton_class::A; @@doesnt_exist end", binding, __FILE__, __LINE__)
     }.should raise_error(NameError) { |e| e.name.should == :@@doesnt_exist }
   end
 
diff --git a/spec/ruby/core/exception/receiver_spec.rb b/spec/ruby/core/exception/receiver_spec.rb
index 7c57d63c3e..d1c23b67be 100644
--- a/spec/ruby/core/exception/receiver_spec.rb
+++ b/spec/ruby/core/exception/receiver_spec.rb
@@ -28,10 +28,8 @@ describe "NameError#receiver" do
 
   it "returns the Object class when an undefined class variable is called" do
     -> {
-      -> {
-        @@doesnt_exist
-      }.should complain(/class variable access from toplevel/)
-    }.should raise_error(NameError) {|e| e.receiver.should equal(Object) }
+      eval("class singleton_class::A; @@doesnt_exist end", binding, __FILE__, __LINE__)
+    }.should raise_error(NameError) {|e| e.receiver.should equal(singleton_class::A) }
   end
 
   it "returns a class when an undefined class variable is called in a subclass' namespace" do
diff --git a/spec/ruby/language/defined_spec.rb b/spec/ruby/language/defined_spec.rb
index ed61ed3b53..f2da8a9293 100644
--- a/spec/ruby/language/defined_spec.rb
+++ b/spec/ruby/language/defined_spec.rb
@@ -275,9 +275,7 @@ describe "The defined? keyword for an expression" do
     end
 
     it "returns nil for an expression with '!' and an unset class variable" do
-      -> {
-        @result = defined?(!@@defined_specs_undefined_class_variable)
-      }.should complain(/class variable access from toplevel/)
+      @result = eval("class singleton_class::A; defined?(!@@doesnt_exist) end", binding, __FILE__, __LINE__)
       @result.should be_nil
     end
 
@@ -286,9 +284,7 @@ describe "The defined? keyword for an expression" do
     end
 
     it "returns nil for an expression with 'not' and an unset class variable" do
-      -> {
-        @result = defined?(not @@defined_specs_undefined_class_variable)
-      }.should complain(/class variable access from toplevel/)
+      @result = eval("class singleton_class::A; defined?(not @@doesnt_exist) end", binding, __FILE__, __LINE__)
       @result.should be_nil
     end
 
@@ -897,17 +893,21 @@ describe "The defined? keyword for a variable scoped constant" do
   end
 
   it "returns nil if the class scoped constant is not defined" do
-    -> {
-      @@defined_specs_obj = DefinedSpecs::Basic
-      defined?(@@defined_specs_obj::Undefined).should be_nil
-    }.should complain(/class variable access from toplevel/)
+    eval(<<-END, binding, __FILE__, __LINE__)
+      class singleton_class::A
+        @@defined_specs_obj = DefinedSpecs::Basic
+        defined?(@@defined_specs_obj::Undefined).should be_nil
+      end
+    END
   end
 
   it "returns 'constant' if the constant is defined in the scope of the class variable" do
-    -> {
-      @@defined_specs_obj = DefinedSpecs::Basic
-      defined?(@@defined_specs_obj::A).should == "constant"
-    }.should complain(/class variable access from toplevel/)
+    eval(<<-END, binding, __FILE__, __LINE__)
+      class singleton_class::A
+        @@defined_specs_obj = DefinedSpecs::Basic
+        defined?(@@defined_specs_obj::A).should == "constant"
+      end
+    END
   end
 
   it "returns nil if the local scoped constant is not defined" do
diff --git a/variable.c b/variable.c
index 9ff0326a14..3bcee434da 100644
--- a/variable.c
+++ b/variable.c
@@ -3064,7 +3064,8 @@ cvar_overtaken(VALUE front, VALUE target, ID id)
 	st_data_t did = (st_data_t)id;
 
         if (RTEST(ruby_verbose) && original_module(front) != original_module(target)) {
-	    rb_warning("class variable % "PRIsVALUE" of %"PRIsVALUE" is overtaken by %"PRIsVALUE"",
+            rb_raise(rb_eRuntimeError,
+                     "class variable % "PRIsVALUE" of %"PRIsVALUE" is overtaken by %"PRIsVALUE"",
 		       ID2SYM(id), rb_class_name(original_module(front)),
 		       rb_class_name(original_module(target)));
 	}
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index a253a222de..2fca2bdd9d 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -989,7 +989,7 @@ vm_get_ev_const(rb_execution_context_t *ec, VALUE orig_klass, ID id, bool allow_
 }
 
 static inline VALUE
-vm_get_cvar_base(const rb_cref_t *cref, rb_control_frame_t *cfp)
+vm_get_cvar_base(const rb_cref_t *cref, rb_control_frame_t *cfp, int top_level_raise)
 {
     VALUE klass;
 
@@ -1002,8 +1002,8 @@ vm_get_cvar_base(const rb_cref_t *cref, rb_control_frame_t *cfp)
 	    CREF_PUSHED_BY_EVAL(cref))) {
 	cref = CREF_NEXT(cref);
     }
-    if (!CREF_NEXT(cref)) {
-	rb_warn("class variable access from toplevel");
+    if (top_level_raise && !CREF_NEXT(cref)) {
+        rb_raise(rb_eRuntimeError, "class variable access from toplevel");
     }
 
     klass = vm_get_iclass(cfp, CREF_CLASS(cref));
@@ -3567,7 +3567,7 @@ vm_defined(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, rb_num_t op_
 	break;
       case DEFINED_CVAR: {
         const rb_cref_t *cref = vm_get_cref(GET_EP());
-	klass = vm_get_cvar_base(cref, GET_CFP());
+        klass = vm_get_cvar_base(cref, GET_CFP(), 0);
 	if (rb_cvar_defined(klass, SYM2ID(obj))) {
 	    expr_type = DEFINED_CVAR;
 	}