[GR-20180] Implement Thread/Exception #backtrace_locations.

Nicolas Laurent · Nicolas Laurent · commit a174c1b471b8 · 2019-12-27T16:09:03.000Z
PullRequest: truffleruby/1218
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -52,6 +52,7 @@ Bug fixes:
 * Make `GC.start` work with keyword arguments.
 * Fixed `Kernel#clone` for `nil`, `true`, `false`, `Integer`, and `Symbol`.
 * Make top-level methods available in `Context#getBindings()` (#1838).
+* Made `Kernel#caller_locations` accept a range argument, and return `nil` when appropriate.
 
 Compatibility:
 
@@ -73,6 +74,7 @@ Compatibility:
 * Implemented `rb_io_set_nonblock` (#1741).
 * Include the major kernel version in `RUBY_PLATFORM` on macOS like MRI (#1860, @eightbitraptor).
 * Implemented `Enumerator::Chain`, `Enumerator#+`, and `Enumerable#chain` (#1859, #1858).
+* Implemented `Thread#backtrace_locations` and `Exception#backtrace_locations` (#1556).
 
 Performance:
 
diff --git a/spec/ruby/core/kernel/caller_locations_spec.rb b/spec/ruby/core/kernel/caller_locations_spec.rb
@@ -22,11 +22,39 @@
     locations.length.should == 1
   end
 
+  it "can be called with a range" do
+    locations1 = caller_locations(0)
+    locations2 = caller_locations(2..4)
+    locations1[2..4].map(&:to_s).should == locations2.map(&:to_s)
+  end
+
+  it "can be called with a range whose end is negative" do
+    locations1 = caller_locations(0)
+    locations2 = caller_locations(2..-1)
+    locations3 = caller_locations(2..-2)
+    locations1[2..-1].map(&:to_s).should == locations2.map(&:to_s)
+    locations1[2..-2].map(&:to_s).should == locations3.map(&:to_s)
+  end
+
+  it "must return nil if omitting more locations than available" do
+    caller_locations(100).should == nil
+    caller_locations(100..-1).should == nil
+  end
+
+  it "must return [] if omitting exactly the number of locations available" do
+    omit = caller_locations(0).length
+    caller_locations(omit).should == []
+  end
+
   it 'returns the locations as Thread::Backtrace::Location instances' do
     locations = KernelSpecs::CallerLocationsTest.locations
 
     locations.each do |location|
       location.kind_of?(Thread::Backtrace::Location).should == true
     end
   end
+
+  it "must return the same locations when called with 1..-1 and when called with no arguments" do
+    caller_locations.map(&:to_s).should == caller_locations(1..-1).map(&:to_s)
+  end
 end
diff --git a/spec/ruby/core/kernel/warn_spec.rb b/spec/ruby/core/kernel/warn_spec.rb
@@ -111,7 +111,7 @@
         -> { w.f4(obj, 2) }.should output(nil, %r|core/kernel/fixtures/classes.rb:#{w.f2_call_lineno}: warning: to_s called|)
       end
 
-      it "does not prepend caller information if line number is too big" do
+      it "does not prepend caller information if the uplevel argument is too large" do
         w = KernelSpecs::WarnInNestedCall.new
         -> { w.f4("foo", 100) }.should output(nil, "warning: foo\n")
       end
diff --git a/spec/ruby/core/thread/backtrace_locations_spec.rb b/spec/ruby/core/thread/backtrace_locations_spec.rb
@@ -19,6 +19,43 @@
     locations.each { |loc| loc.should be_an_instance_of(Thread::Backtrace::Location) }
   end
 
+  it "can be called with a number of locations to omit" do
+    locations1 = Thread.current.backtrace_locations
+    locations2 = Thread.current.backtrace_locations(2)
+    locations1[2..-1].length.should == locations2.length
+    locations1[2..-1].map(&:to_s).should == locations2.map(&:to_s)
+  end
+
+  it "can be called with a maximum number of locations to return as second parameter" do
+    locations1 = Thread.current.backtrace_locations
+    locations2 = Thread.current.backtrace_locations(2, 3)
+    locations1[2..4].map(&:to_s).should == locations2.map(&:to_s)
+  end
+
+  it "can be called with a range" do
+    locations1 = Thread.current.backtrace_locations
+    locations2 = Thread.current.backtrace_locations(2..4)
+    locations1[2..4].map(&:to_s).should == locations2.map(&:to_s)
+  end
+
+  it "can be called with a range whose end is negative" do
+    locations1 = Thread.current.backtrace_locations
+    locations2 = Thread.current.backtrace_locations(2..-1)
+    locations3 = Thread.current.backtrace_locations(2..-2)
+    locations1[2..-1].map(&:to_s).should == locations2.map(&:to_s)
+    locations1[2..-2].map(&:to_s).should == locations3.map(&:to_s)
+  end
+
+  it "returns nil if omitting more locations than available" do
+    Thread.current.backtrace_locations(100).should == nil
+    Thread.current.backtrace_locations(100..-1).should == nil
+  end
+
+  it "returns [] if omitting exactly the number of locations available" do
+    omit = Thread.current.backtrace_locations.length
+    Thread.current.backtrace_locations(omit).should == []
+  end
+
   it "without argument is the same as showing all locations with 0..-1" do
     Thread.current.backtrace_locations.map(&:to_s).should == Thread.current.backtrace_locations(0..-1).map(&:to_s)
   end
diff --git a/spec/tags/core/exception/backtrace_locations_tags.txt b/spec/tags/core/exception/backtrace_locations_tags.txt
diff --git a/spec/tags/core/thread/backtrace_locations_tags.txt b/spec/tags/core/thread/backtrace_locations_tags.txt
@@ -1,6 +1 @@
-fails:Thread#backtrace_locations returns an Array
-fails:Thread#backtrace_locations sets each element to a Thread::Backtrace::Location
-fails:Thread#backtrace_locations can be called on any Thread
-fails:Thread#backtrace_locations without argument is the same as showing all locations with 0..-1
-fails:Thread#backtrace_locations the first location reports the call to #backtrace_locations
-fails:Thread#backtrace_locations [1..-1] is the same as #caller_locations(0..-1) for Thread.current
+fails:Thread#backtrace_locations the first location reports the call to #backtrace_locations
diff --git a/src/main/java/org/truffleruby/core/exception/CoreExceptions.java b/src/main/java/org/truffleruby/core/exception/CoreExceptions.java
@@ -729,6 +729,8 @@ public DynamicObject nameError(String message, Object receiver, String name, Nod
                 null,
                 backtrace,
                 cause,
+                null,
+                null,
                 receiver,
                 context.getSymbolTable().getSymbol(name)));
     }
@@ -744,6 +746,8 @@ public DynamicObject nameErrorFromMethodMissing(DynamicObject formatter, Object
                         formatter,
                         backtrace,
                         cause,
+                        null,
+                        null,
                         receiver,
                         context.getSymbolTable().getSymbol(name)));
         showExceptionIfDebug(exception, backtrace);
@@ -766,6 +770,8 @@ public DynamicObject noMethodError(String message, Object receiver, String name,
                         null,
                         backtrace,
                         cause,
+                        null,
+                        null,
                         receiver,
                         context.getSymbolTable().getSymbol(name),
                         argsArray));
@@ -784,6 +790,8 @@ public DynamicObject noMethodErrorFromMethodMissing(DynamicObject formatter, Obj
                         formatter,
                         backtrace,
                         cause,
+                        null,
+                        null,
                         receiver,
                         context.getSymbolTable().getSymbol(name),
                         argsArray));
@@ -807,6 +815,8 @@ public DynamicObject noSuperMethodOutsideMethodError(Node currentNode) {
                 backtrace,
                 cause,
                 null,
+                null,
+                null,
                 // FIXME: the name of the method is not known in this case currently
                 context.getSymbolTable().getSymbol("<unknown>")));
     }
diff --git a/src/main/java/org/truffleruby/core/exception/ExceptionLayout.java b/src/main/java/org/truffleruby/core/exception/ExceptionLayout.java
@@ -28,7 +28,9 @@ Object[] build(
             @Nullable Object message,
             @Nullable DynamicObject formatter,
             @Nullable Backtrace backtrace,
-            DynamicObject cause);
+            DynamicObject cause,
+            @Nullable DynamicObject backtraceStringArray,
+            @Nullable DynamicObject backtraceLocations);
 
     boolean isException(DynamicObject object);
 
@@ -48,4 +50,11 @@ Object[] build(
 
     void setCause(DynamicObject object, DynamicObject value);
 
+    DynamicObject getBacktraceStringArray(DynamicObject object);
+
+    void setBacktraceStringArray(DynamicObject object, DynamicObject value);
+
+    DynamicObject getBacktraceLocations(DynamicObject object);
+
+    void setBacktraceLocations(DynamicObject object, DynamicObject value);
 }
diff --git a/src/main/java/org/truffleruby/core/exception/ExceptionNodes.java b/src/main/java/org/truffleruby/core/exception/ExceptionNodes.java
@@ -44,7 +44,8 @@ public abstract static class AllocateNode extends CoreMethodArrayArgumentsNode {
 
         @Specialization
         protected DynamicObject allocateNameError(DynamicObject rubyClass) {
-            return allocateObjectNode.allocate(rubyClass, Layouts.EXCEPTION.build(nil(), null, null, nil()));
+            return allocateObjectNode
+                    .allocate(rubyClass, Layouts.EXCEPTION.build(nil(), null, null, nil(), null, null));
         }
 
     }
@@ -151,14 +152,14 @@ protected Object backtrace(
             if (hasCustomBacktraceProfile.profile(customBacktrace != null)) {
                 return customBacktrace;
             } else if (hasBacktraceProfile.profile(Layouts.EXCEPTION.getBacktrace(exception) != null)) {
-                final Backtrace backtrace = Layouts.EXCEPTION.getBacktrace(exception);
-                if (backtrace.getBacktraceStringArray() == null) {
-                    backtrace.setBacktraceStringArray(
-                            getContext().getUserBacktraceFormatter().formatBacktraceAsRubyStringArray(
-                                    exception,
-                                    Layouts.EXCEPTION.getBacktrace(exception)));
+                DynamicObject backtraceStringArray = Layouts.EXCEPTION.getBacktraceStringArray(exception);
+                if (backtraceStringArray == null) {
+                    backtraceStringArray = getContext().getUserBacktraceFormatter().formatBacktraceAsRubyStringArray(
+                            exception,
+                            Layouts.EXCEPTION.getBacktrace(exception));
+                    Layouts.EXCEPTION.setBacktraceStringArray(exception, backtraceStringArray);
                 }
-                return backtrace.getBacktraceStringArray();
+                return backtraceStringArray;
             } else {
                 return nil();
             }
@@ -175,6 +176,27 @@ private ReadObjectFieldNode getReadCustomBacktraceNode() {
 
     }
 
+    @CoreMethod(names = "backtrace_locations")
+    public abstract static class BacktraceLocationsNode extends CoreMethodArrayArgumentsNode {
+
+        @Specialization
+        protected Object backtraceLocations(DynamicObject exception,
+                @Cached("createBinaryProfile()") ConditionProfile hasBacktraceProfile,
+                @Cached("createBinaryProfile()") ConditionProfile hasLocationsProfile) {
+            if (hasBacktraceProfile.profile(Layouts.EXCEPTION.getBacktrace(exception) != null)) {
+                DynamicObject backtraceLocations = Layouts.EXCEPTION.getBacktraceLocations(exception);
+                if (hasLocationsProfile.profile(backtraceLocations == null)) {
+                    Backtrace backtrace = Layouts.EXCEPTION.getBacktrace(exception);
+                    backtraceLocations = backtrace.getBacktraceLocations(GetBacktraceException.UNLIMITED);
+                    Layouts.EXCEPTION.setBacktraceLocations(exception, backtraceLocations);
+                }
+                return backtraceLocations;
+            } else {
+                return nil();
+            }
+        }
+    }
+
     @Primitive(name = "exception_backtrace?")
     public abstract static class BacktraceQueryPrimitiveNode extends PrimitiveArrayArgumentsNode {
 
diff --git a/src/main/java/org/truffleruby/core/exception/ExceptionOperations.java b/src/main/java/org/truffleruby/core/exception/ExceptionOperations.java
@@ -77,7 +77,7 @@ public static DynamicObject createRubyException(RubyContext context, DynamicObje
         context.getCoreExceptions().showExceptionIfDebug(rubyClass, message, backtrace);
         return Layouts.CLASS
                 .getInstanceFactory(rubyClass)
-                .newInstance(Layouts.EXCEPTION.build(message, null, backtrace, cause));
+                .newInstance(Layouts.EXCEPTION.build(message, null, backtrace, cause, null, null));
     }
 
     // because the factory is not constant
@@ -88,7 +88,7 @@ public static DynamicObject createSystemCallError(RubyContext context, DynamicOb
         final DynamicObject cause = ThreadGetExceptionNode.getLastException(context);
         context.getCoreExceptions().showExceptionIfDebug(rubyClass, message, backtrace);
         return Layouts.CLASS.getInstanceFactory(rubyClass).newInstance(
-                Layouts.SYSTEM_CALL_ERROR.build(message, null, backtrace, cause, errno));
+                Layouts.SYSTEM_CALL_ERROR.build(message, null, backtrace, cause, null, null, errno));
     }
 
     public static DynamicObject getFormatter(String name, RubyContext context) {
diff --git a/src/main/java/org/truffleruby/core/exception/NameErrorLayout.java b/src/main/java/org/truffleruby/core/exception/NameErrorLayout.java
@@ -28,6 +28,8 @@ Object[] build(
             @Nullable DynamicObject formatter,
             @Nullable Backtrace backtrace,
             DynamicObject cause,
+            @Nullable DynamicObject backtraceStringArray,
+            @Nullable DynamicObject backtraceLocations,
             @Nullable Object receiver,
             Object name);
 
diff --git a/src/main/java/org/truffleruby/core/exception/NameErrorNodes.java b/src/main/java/org/truffleruby/core/exception/NameErrorNodes.java
@@ -33,7 +33,9 @@ public abstract static class AllocateNode extends CoreMethodArrayArgumentsNode {
         @Specialization
         protected DynamicObject allocateNameError(DynamicObject rubyClass) {
             return allocateObjectNode
-                    .allocate(rubyClass, Layouts.NAME_ERROR.build(nil(), null, null, nil(), null, nil()));
+                    .allocate(
+                            rubyClass,
+                            Layouts.NAME_ERROR.build(nil(), null, null, nil(), null, null, null, nil()));
         }
 
     }
diff --git a/src/main/java/org/truffleruby/core/exception/NoMethodErrorLayout.java b/src/main/java/org/truffleruby/core/exception/NoMethodErrorLayout.java
@@ -28,6 +28,8 @@ Object[] build(
             @Nullable DynamicObject formatter,
             @Nullable Backtrace backtrace,
             DynamicObject cause,
+            @Nullable DynamicObject backtraceStringArray,
+            @Nullable DynamicObject backtraceLocations,
             @Nullable Object receiver,
             Object name,
             Object args);
diff --git a/src/main/java/org/truffleruby/core/exception/NoMethodErrorNodes.java b/src/main/java/org/truffleruby/core/exception/NoMethodErrorNodes.java
@@ -32,7 +32,9 @@ public abstract static class AllocateNode extends CoreMethodArrayArgumentsNode {
         @Specialization
         protected DynamicObject allocateNoMethodError(DynamicObject rubyClass) {
             return allocateObjectNode
-                    .allocate(rubyClass, Layouts.NO_METHOD_ERROR.build(nil(), null, null, nil(), null, nil(), nil()));
+                    .allocate(
+                            rubyClass,
+                            Layouts.NO_METHOD_ERROR.build(nil(), null, null, nil(), null, null, null, nil(), nil()));
         }
 
     }
diff --git a/src/main/java/org/truffleruby/core/exception/SystemCallErrorLayout.java b/src/main/java/org/truffleruby/core/exception/SystemCallErrorLayout.java
@@ -28,6 +28,8 @@ Object[] build(
             @Nullable DynamicObject formatter,
             @Nullable Backtrace backtrace,
             DynamicObject cause,
+            @Nullable DynamicObject backtraceStringArray,
+            @Nullable DynamicObject backtraceLocations,
             Object errno);
 
     boolean isSystemCallError(DynamicObject object);
diff --git a/src/main/java/org/truffleruby/core/exception/SystemCallErrorNodes.java b/src/main/java/org/truffleruby/core/exception/SystemCallErrorNodes.java
@@ -32,7 +32,7 @@ public abstract static class AllocateNode extends CoreMethodArrayArgumentsNode {
         @Specialization
         protected DynamicObject allocateNameError(DynamicObject rubyClass) {
             return allocateObjectNode
-                    .allocate(rubyClass, Layouts.SYSTEM_CALL_ERROR.build(nil(), null, null, nil(), nil()));
+                    .allocate(rubyClass, Layouts.SYSTEM_CALL_ERROR.build(nil(), null, null, nil(), null, null, nil()));
         }
 
     }
diff --git a/src/main/java/org/truffleruby/core/kernel/KernelNodes.java b/src/main/java/org/truffleruby/core/kernel/KernelNodes.java
@@ -301,47 +301,33 @@ protected DynamicObject calleeName() {
         }
     }
 
-    @CoreMethod(names = "caller_locations", isModuleFunction = true, optional = 2, lowerFixnum = { 1, 2 })
+    @Primitive(name = "kernel_caller_locations", lowerFixnum = { 0, 1 })
     public abstract static class CallerLocationsNode extends CoreMethodArrayArgumentsNode {
 
-        @Specialization
-        protected DynamicObject callerLocations(NotProvided omit, NotProvided length) {
-            return innerCallerLocations(1, GetBacktraceException.UNLIMITED);
-        }
-
         @Specialization
         protected DynamicObject callerLocations(int omit, NotProvided length) {
             return innerCallerLocations(omit, GetBacktraceException.UNLIMITED);
         }
 
-        @Specialization(guards = "length >= 0")
+        @Specialization
         protected DynamicObject callerLocations(int omit, int length) {
             return innerCallerLocations(omit, length);
         }
 
         private DynamicObject innerCallerLocations(int omit, int length) {
-            final int omitted = 1 /* always skip #caller_locations */ + omit;
+            // Always skip #caller_locations.
+            final int omitted = omit + 1;
             final Backtrace backtrace = getContext().getCallStack().getBacktrace(this, omitted);
-            final int limit = (length == GetBacktraceException.UNLIMITED)
+
+            // We can't set an effective limit when dealing with negative range endings.
+            final int limit = length < 0
                     ? GetBacktraceException.UNLIMITED
                     : omitted + length;
 
-            int locationsCount = backtrace.getActivations(new GetBacktraceException(this, limit)).length;
-
-            if (length != GetBacktraceException.UNLIMITED && length < locationsCount) {
-                locationsCount = length;
-            }
-
-            final Object[] locations = new Object[locationsCount];
-
-            for (int n = 0; n < locationsCount; n++) {
-                locations[n] = Layouts.THREAD_BACKTRACE_LOCATION.createThreadBacktraceLocation(
-                        coreLibrary().threadBacktraceLocationFactory,
-                        backtrace,
-                        n);
-            }
+            // Fill in the stack trace.
+            backtrace.getActivations(new GetBacktraceException(this, limit));
 
-            return createArray(locations, locations.length);
+            return backtrace.getBacktraceLocations(length);
         }
     }
 
diff --git a/src/main/java/org/truffleruby/core/thread/ThreadNodes.java b/src/main/java/org/truffleruby/core/thread/ThreadNodes.java
diff --git a/src/main/java/org/truffleruby/language/backtrace/Backtrace.java b/src/main/java/org/truffleruby/language/backtrace/Backtrace.java
diff --git a/src/main/ruby/truffleruby/core/kernel.rb b/src/main/ruby/truffleruby/core/kernel.rb
diff --git a/src/main/ruby/truffleruby/core/thread.rb b/src/main/ruby/truffleruby/core/thread.rb
