Merge pull request #6769 from hlindberg/PUP-8610_speedup-get-top-of-p…

…uppet-stack

(PUP-8610) Speedup PuppetStack by adding top_of_stack and use that

lib/puppet/error.rb

@@ -72,13 +72,8 @@ def to_s
     end
 
     def self.from_issue_and_stack(issue, args = {})
-      stacktrace = Puppet::Pops::PuppetStack.stacktrace()
-      if stacktrace.size > 0
-        filename, line = stacktrace[0]
-      else
-        filename = nil
-        line = nil
-      end
+      filename, line = Puppet::Pops::PuppetStack.top_of_stack
+
       self.new(
             issue.format(args),
             filename,

lib/puppet/functions/break.rb

@@ -36,13 +36,9 @@
   end
 
   def break_impl()
-    stacktrace = Puppet::Pops::PuppetStack.stacktrace()
-    if stacktrace.size > 0
-      file, line = stacktrace[0]
-    else
-      file = nil
-      line = nil
-    end
+    # get file, line if available, else they are set to nil
+    file, line = Puppet::Pops::PuppetStack.top_of_stack
+
     # PuppetStopIteration contains file and line and is a StopIteration exception
     # so it can break a Ruby Kernel#loop or enumeration
     #

lib/puppet/functions/empty.rb

@@ -71,8 +71,7 @@ def undef_empty(x)
   end
 
   def deprecation_warning_for(arg_type)
-    stacktrace = Puppet::Pops::PuppetStack.stacktrace()
-    file, line = stacktrace[0] # defaults to nil
+    file, line = Puppet::Pops::PuppetStack.top_of_stack
     msg = _("Calling function empty() with %{arg_type} value is deprecated.") % { arg_type: arg_type }
     Puppet.warn_once('deprecations', "empty-from-#{file}-#{line}", msg, file, line)
   end

lib/puppet/functions/next.rb

@@ -9,14 +9,7 @@
   end
 
   def next_impl(value = nil)
-    stacktrace = Puppet::Pops::PuppetStack.stacktrace()
-    if stacktrace.size > 0
-      file, line = stacktrace[0]
-    else
-      file = nil
-      line = nil
-    end
-
+    file, line = Puppet::Pops::PuppetStack.top_of_stack
     exc = Puppet::Pops::Evaluator::Next.new(value, file, line)
     raise exc
   end

lib/puppet/functions/return.rb

@@ -9,14 +9,7 @@
   end
 
   def return_impl(value = nil)
-    stacktrace = Puppet::Pops::PuppetStack.stacktrace()
-    if stacktrace.size > 0
-      file, line = stacktrace[0]
-    else
-      file = nil
-      line = nil
-    end
-
+    file, line = Puppet::Pops::PuppetStack.top_of_stack
     raise Puppet::Pops::Evaluator::Return.new(value, file, line)
   end
 end

lib/puppet/functions/strftime.rb

@@ -202,13 +202,7 @@ def format_timestamp(time_object, format, timezone = nil)
   end
 
   def legacy_strftime(format, timezone = nil)
-    stacktrace = Puppet::Pops::PuppetStack.stacktrace()
-    if stacktrace.size > 0
-      file, line = stacktrace[0]
-    else
-      file = nil
-      line = nil
-    end
+    file, line = Puppet::Pops::PuppetStack.top_of_stack
     Puppet.warn_once('deprecations', 'legacy#strftime',
       _('The argument signature (String format, [String timezone]) is deprecated for #strftime. See #strftime documentation and Timespan type for more info'),
       file, line)

lib/puppet/parser/functions/create_resources.rb

@@ -67,13 +67,7 @@
   # If relayed via other puppet functions in ruby that do not nest their calls, the source position
   # will be in the original puppet source.
   #
-  stacktrace = Puppet::Pops::PuppetStack.stacktrace()
-  if stacktrace.size > 0
-    file, line = stacktrace[0]
-  else
-    file = nil
-    line = nil
-  end
+  file, line = Puppet::Pops::PuppetStack.top_of_stack
 
   if type.start_with? '@@'
     exported = true

lib/puppet/parser/scope.rb

@@ -875,8 +875,8 @@ def to_s
     return "Scope(#{@resource})" unless @resource.nil?
 
     # For logging of function-scope - it is now showing the file and line.
-    detail = Puppet::Pops::PuppetStack.stacktrace[0]
-    return "Scope()" unless detail.is_a?(Array)
+    detail = Puppet::Pops::PuppetStack.top_of_stack
+    return "Scope()" if detail.empty?
 
     # shorten the path if possible
     path = detail[0]

lib/puppet/pops/evaluator/runtime3_support.rb

@@ -55,13 +55,8 @@ def optionally_fail(issue, semantic, options={}, except=nil)
   #
   def runtime_issue(issue, options={})
     # Get position from puppet runtime stack
-    stacktrace = Puppet::Pops::PuppetStack.stacktrace()
-    if stacktrace.size > 0
-      file, line = stacktrace[0]
-    else
-      file = nil
-      line = nil
-    end
+    file, line = Puppet::Pops::PuppetStack.top_of_stack
+
     # Use a SemanticError as the sourcepos
     semantic = Puppet::Pops::SemanticError.new(issue, nil, options.merge({:file => file, :line => line}))
     optionally_fail(issue,  semantic)

lib/puppet/pops/puppet_stack.rb

@@ -17,6 +17,9 @@ module Puppet::Pops
 # will be represented with the text `unknown` and `0´ respectively.
 #
 module PuppetStack
+  # Pattern matching an entry in the ruby stack that is a puppet entry
+  PP_ENTRY_PATTERN = /^(.*\.pp)?:([0-9]+):in (`stack'|`block in call_function')/
+
   # Sends a message to an obj such that it appears to come from
   # file, line when calling stacktrace.
   #
@@ -33,11 +36,22 @@ def self.stack(file, line, obj, message, args, &block)
 
   def self.stacktrace
     caller().reduce([]) do |memo, loc|
-      if loc =~ /^(.*\.pp)?:([0-9]+):in (`stack'|`block in call_function')/
+      if loc =~ PP_ENTRY_PATTERN
         memo << [$1.nil? ? 'unknown' : $1, $2.to_i]
       end
       memo
     end
   end
+
+  # Returns an Array with the top of the puppet stack, or an empty Array if there was no such entry.
+  #
+  def self.top_of_stack
+    caller.each do |loc|
+      if loc =~ PP_ENTRY_PATTERN
+        return [$1.nil? ? 'unknown' : $1, $2.to_i]
+      end
+    end
+    []
+  end
 end
 end

lib/puppet/pops/serialization/to_data_converter.rb

@@ -290,12 +290,12 @@ def pcore_type_to_data(pcore_type)
     def serialization_issue(issue, options = EMPTY_HASH)
       semantic = @semantic
       if semantic.nil?
-        stacktrace = Puppet::Pops::PuppetStack.stacktrace()
-        if stacktrace.size > 0
-          file, line = stacktrace[0]
-          semantic = Puppet::Pops::SemanticError.new(issue, nil, {:file => file, :line => line})
-        else
+        tos = Puppet::Pops::PuppetStack.top_of_stack
+        if tos.empty?
           semantic = Puppet::Pops::SemanticError.new(issue, nil, EMPTY_HASH)
+        else
+          file, line = stacktrace
+          semantic = Puppet::Pops::SemanticError.new(issue, nil, {:file => file, :line => line})
         end
       end
       optionally_fail(issue,  semantic, options)

spec/unit/parser/functions/create_resources_spec.rb

@@ -77,7 +77,7 @@
     it 'should pick up and pass on file and line information' do
       # mock location as the compile_to_catalog sets Puppet[:code} which does not
       # have file/line support.
-      Puppet::Pops::PuppetStack.expects(:stacktrace).once.returns([['test.pp', 1234]])
+      Puppet::Pops::PuppetStack.expects(:top_of_stack).once.returns(['test.pp', 1234])
       catalog = compile_to_catalog("create_resources('file', {'/etc/foo'=>{'ensure'=>'present'}})")
       r = catalog.resource(:file, "/etc/foo")
       expect(r.file).to eq('test.pp')

spec/unit/pops/puppet_stack_spec.rb

@@ -8,22 +8,34 @@ def get_stacktrace
       Puppet::Pops::PuppetStack.stacktrace
     end
 
-    def one_level
-      Puppet::Pops::PuppetStack.stack("one_level.pp", 1234, self, :get_stacktrace, [])
+    def get_top_of_stack
+      Puppet::Pops::PuppetStack.top_of_stack
     end
 
     def one_level
       Puppet::Pops::PuppetStack.stack("one_level.pp", 1234, self, :get_stacktrace, [])
     end
 
+    def one_level_top
+      Puppet::Pops::PuppetStack.stack("one_level.pp", 1234, self, :get_top_of_stack, [])
+    end
+
     def two_levels
       Puppet::Pops::PuppetStack.stack("two_levels.pp", 1237, self, :level2, [])
     end
 
+    def two_levels_top
+      Puppet::Pops::PuppetStack.stack("two_levels.pp", 1237, self, :level2_top, [])
+    end
+
     def level2
       Puppet::Pops::PuppetStack.stack("level2.pp", 1240, self, :get_stacktrace, [])
     end
 
+    def level2_top
+      Puppet::Pops::PuppetStack.stack("level2.pp", 1240, self, :get_top_of_stack, [])
+    end
+
     def gets_block(a, &block)
       block.call(a)
     end
@@ -45,16 +57,33 @@ def with_empty_string_file
     expect(Puppet::Pops::PuppetStack.stacktrace).to eql([])
   end
 
-  it 'returns a one element array with file, line from stacktrace when there is one entry on the stack' do
-    expect(StackTraceTest.new.one_level).to eql([['one_level.pp', 1234]])
-  end
 
-  it 'returns an array from stacktrace with information about each level with oldest frame last' do
-    expect(StackTraceTest.new.two_levels).to eql([['level2.pp', 1240], ['two_levels.pp', 1237]])
+  context 'full stacktrace' do
+    it 'returns a one element array with file, line from stacktrace when there is one entry on the stack' do
+      expect(StackTraceTest.new.one_level).to eql([['one_level.pp', 1234]])
+    end
+
+    it 'returns an array from stacktrace with information about each level with oldest frame last' do
+      expect(StackTraceTest.new.two_levels).to eql([['level2.pp', 1240], ['two_levels.pp', 1237]])
+    end
+
+    it 'returns an empty array from top_of_stack when there is nothing on the stack' do
+      expect(Puppet::Pops::PuppetStack.top_of_stack).to eql([])
+    end
   end
 
-  it 'accepts file to be nil' do
-    expect(StackTraceTest.new.with_nil_file).to eql([['unknown', 1250]])
+  context 'top of stack' do
+    it 'returns a one element array with file, line from top_of_stack when there is one entry on the stack' do
+      expect(StackTraceTest.new.one_level_top).to eql(['one_level.pp', 1234])
+    end
+
+    it 'returns newest entry from top_of_stack' do
+      expect(StackTraceTest.new.two_levels_top).to eql(['level2.pp', 1240])
+    end
+
+    it 'accepts file to be nil' do
+      expect(StackTraceTest.new.with_nil_file).to eql([['unknown', 1250]])
+    end
   end
 
   it 'accepts file to be empty_string' do