[GR-20288] Don't omit backtrace when rescue $ERROR_INFO (#1660).

PullRequest: truffleruby/1207

Author: eregon

CHANGELOG.md

@@ -54,6 +54,7 @@ Bug fixes:
 * Make top-level methods available in `Context#getBindings()` (#1838).
 * Made `Kernel#caller_locations` accept a range argument, and return `nil` when appropriate.
 * Made `rb_respond_to` work with primitives (#1869, @chrisseaton).
+* Fixed issue with missing backtrace for `rescue $ERROR_INFO` (#1660).
 
 Compatibility:
 

spec/ruby/language/alias_spec.rb

@@ -255,4 +255,9 @@ def test_with_check(*args)
     code = '$a = 1; $b = 2; alias $b $a; p [$a, $b]; $b = 3; p [$a, $b]'
     ruby_exe(code).should == "[1, 1]\n[3, 3]\n"
   end
+
+  it "supports aliasing twice the same global variables" do
+    code = '$a = 1; alias $b $a; alias $b $a; p [$a, $b]'
+    ruby_exe(code).should == "[1, 1]\n"
+  end
 end

spec/ruby/library/English/alias_spec.rb

@@ -0,0 +1,14 @@
+require_relative '../../spec_helper'
+require 'English'
+
+describe "English" do
+  it "aliases $! to $ERROR_INFO and $ERROR_INFO still returns an Exception with a backtrace" do
+    exception = (1 / 0 rescue $ERROR_INFO)
+    exception.should be_kind_of(Exception)
+    exception.backtrace.should be_kind_of(Array)
+  end
+
+  it "aliases $@ to $ERROR_POSITION and $ERROR_POSITION still returns a backtrace" do
+    (1 / 0 rescue $ERROR_POSITION).should be_kind_of(Array)
+  end
+end

spec/tags/language/alias_tags.txt

@@ -1,3 +1,4 @@
 slow:The alias keyword on top level defines the alias on Object
 slow:The alias keyword can create a new global variable, synonym of the original
 slow:The alias keyword can override an existing global variable and make them synonyms
+slow:The alias keyword supports aliasing twice the same global variables

src/main/java/org/truffleruby/language/globals/GlobalVariables.java

@@ -72,7 +72,8 @@ public void alias(String oldName, String newName) {
         final GlobalVariableStorage storage = getStorage(oldName);
 
         final GlobalVariableStorage previousStorage = variables.put(newName, storage);
-        if (previousStorage != null) {
+        // If previousStorage == storage, we already have that alias and should not invalidate
+        if (previousStorage != null && previousStorage != storage) {
             previousStorage.getValidAssumption().invalidate();
         }
     }

src/main/java/org/truffleruby/parser/BodyTranslator.java

@@ -2840,10 +2840,7 @@ public RubyNode visitRescueNode(RescueParseNode node) {
         boolean canOmitBacktrace = false;
 
         if (context.getOptions().BACKTRACES_OMIT_UNUSED && rescueBody != null &&
-                rescueBody.getBodyNode() instanceof SideEffectFree /* allow `expression rescue $!` pattern */ &&
-                (!(rescueBody.getBodyNode() instanceof GlobalVarParseNode) ||
-                        !((GlobalVarParseNode) rescueBody.getBodyNode()).getName().equals("$!")) &&
-                rescueBody.getOptRescueNode() == null) {
+                rescueBody.getBodyNode() instanceof SideEffectFree && rescueBody.getOptRescueNode() == null) {
             canOmitBacktrace = true;
         }
 

src/main/java/org/truffleruby/parser/ast/GlobalVarParseNode.java

@@ -40,7 +40,7 @@
 /**
  * access to a global variable.
  */
-public class GlobalVarParseNode extends ParseNode implements INameNode, SideEffectFree {
+public class GlobalVarParseNode extends ParseNode implements INameNode {
     private String name;
 
     public GlobalVarParseNode(SourceIndexLength position, String name) {