[ruby/prism] Change inspect from recursive to a queue

We would previously cause a stack overflow if we parsed a file that was too deeply nested when we were calling inspect. Instead, we now use a queue of commands to do it linearly so we don't. https://github.com/ruby/prism/commit/0f21f5bfe1
2024-04-24 11:58:24 -04:00 · 2024-04-24 11:58:24 -04:00 · 6d9ba1e014
commit 6d9ba1e014
parent cf24a0483e
7 changed files with 184 additions and 148 deletions
--- a/lib/prism.rb
+++ b/lib/prism.rb
@ -18,10 +18,10 @@ module Prism
  autoload :Dispatcher, "prism/dispatcher"
  autoload :DotVisitor, "prism/dot_visitor"
  autoload :DSL, "prism/dsl"
+  autoload :InspectVisitor, "prism/inspect_visitor"
  autoload :LexCompat, "prism/lex_compat"
  autoload :LexRipper, "prism/lex_compat"
  autoload :MutationCompiler, "prism/mutation_compiler"
-  autoload :NodeInspector, "prism/node_inspector"
  autoload :Pack, "prism/pack"
  autoload :Pattern, "prism/pattern"
  autoload :Reflection, "prism/reflection"
--- a/lib/prism/node_inspector.rb
+++ b/lib/prism/node_inspector.rb
@ -1,68 +0,0 @@
-# frozen_string_literal: true
-
-module Prism
-  # This object is responsible for generating the output for the inspect method
-  # implementations of child nodes.
-  class NodeInspector # :nodoc:
-    attr_reader :prefix, :output
-
-    def initialize(prefix = "")
-      @prefix = prefix
-      @output = +""
-    end
-
-    # Appends a line to the output with the current prefix.
-    def <<(line)
-      output << "#{prefix}#{line}"
-    end
-
-    # This generates a string that is used as the header of the inspect output
-    # for any given node.
-    def header(node)
-      output = +"@ #{node.class.name.split("::").last} ("
-      output << "location: (#{node.location.start_line},#{node.location.start_column})-(#{node.location.end_line},#{node.location.end_column})"
-      output << ", newline: true" if node.newline?
-      output << ")\n"
-      output
-    end
-
-    # Generates a string that represents a list of nodes. It handles properly
-    # using the box drawing characters to make the output look nice.
-    def list(prefix, nodes)
-      output = +"(length: #{nodes.length})\n"
-      last_index = nodes.length - 1
-
-      nodes.each_with_index do |node, index|
-        pointer, preadd = (index == last_index) ? ["└── ", "    "] : ["├── ", "│   "]
-        node_prefix = "#{prefix}#{preadd}"
-        output << node.inspect(NodeInspector.new(node_prefix)).sub(node_prefix, "#{prefix}#{pointer}")
-      end
-
-      output
-    end
-
-    # Generates a string that represents a location field on a node.
-    def location(value)
-      if value
-        "(#{value.start_line},#{value.start_column})-(#{value.end_line},#{value.end_column}) = #{value.slice.inspect}"
-      else
-        "∅"
-      end
-    end
-
-    # Generates a string that represents a child node.
-    def child_node(node, append)
-      node.inspect(child_inspector(append)).delete_prefix(prefix)
-    end
-
-    # Returns a new inspector that can be used to inspect a child node.
-    def child_inspector(append)
-      NodeInspector.new("#{prefix}#{append}")
-    end
-
-    # Returns the output as a string.
-    def to_str
-      output
-    end
-  end
-end
--- a/lib/prism/prism.gemspec
+++ b/lib/prism/prism.gemspec
@ -76,10 +76,10 @@ Gem::Specification.new do |spec|
    "lib/prism/dot_visitor.rb",
    "lib/prism/dsl.rb",
    "lib/prism/ffi.rb",
+    "lib/prism/inspect_visitor.rb",
    "lib/prism/lex_compat.rb",
    "lib/prism/mutation_compiler.rb",
    "lib/prism/node_ext.rb",
-    "lib/prism/node_inspector.rb",
    "lib/prism/node.rb",
    "lib/prism/pack.rb",
    "lib/prism/parse_result.rb",
@ -101,46 +101,11 @@ Gem::Specification.new do |spec|
    "lib/prism/translation/ripper/shim.rb",
    "lib/prism/translation/ruby_parser.rb",
    "lib/prism/visitor.rb",
-    "src/diagnostic.c",
-    "src/encoding.c",
-    "src/node.c",
-    "src/pack.c",
-    "src/prettyprint.c",
-    "src/regexp.c",
-    "src/serialize.c",
-    "src/static_literals.c",
-    "src/token_type.c",
-    "src/util/pm_buffer.c",
-    "src/util/pm_char.c",
-    "src/util/pm_constant_pool.c",
-    "src/util/pm_integer.c",
-    "src/util/pm_list.c",
-    "src/util/pm_memchr.c",
-    "src/util/pm_newline_list.c",
-    "src/util/pm_string.c",
-    "src/util/pm_string_list.c",
-    "src/util/pm_strncasecmp.c",
-    "src/util/pm_strpbrk.c",
-    "src/options.c",
-    "src/prism.c",
    "prism.gemspec",
-    "sig/prism.rbs",
-    "sig/prism/compiler.rbs",
-    "sig/prism/dispatcher.rbs",
-    "sig/prism/dot_visitor.rbs",
-    "sig/prism/dsl.rbs",
-    "sig/prism/mutation_compiler.rbs",
-    "sig/prism/node.rbs",
-    "sig/prism/node_ext.rbs",
-    "sig/prism/pack.rbs",
-    "sig/prism/parse_result.rbs",
-    "sig/prism/pattern.rbs",
-    "sig/prism/reflection.rbs",
-    "sig/prism/serialize.rbs",
-    "sig/prism/visitor.rbs",
    "rbi/prism.rbi",
    "rbi/prism/compiler.rbi",
    "rbi/prism/desugar_compiler.rbi",
+    "rbi/prism/inspect_visitor.rbi",
    "rbi/prism/mutation_compiler.rbi",
    "rbi/prism/node_ext.rbi",
    "rbi/prism/node.rbi",
@ -153,7 +118,44 @@ Gem::Specification.new do |spec|
    "rbi/prism/translation/ripper.rbi",
    "rbi/prism/translation/ripper/ripper_compiler.rbi",
    "rbi/prism/translation/ruby_parser.rbi",
-    "rbi/prism/visitor.rbi"
+    "rbi/prism/visitor.rbi",
+    "sig/prism.rbs",
+    "sig/prism/compiler.rbs",
+    "sig/prism/dispatcher.rbs",
+    "sig/prism/dot_visitor.rbs",
+    "sig/prism/dsl.rbs",
+    "sig/prism/inspect_visitor.rbs",
+    "sig/prism/mutation_compiler.rbs",
+    "sig/prism/node_ext.rbs",
+    "sig/prism/node.rbs",
+    "sig/prism/pack.rbs",
+    "sig/prism/parse_result.rbs",
+    "sig/prism/pattern.rbs",
+    "sig/prism/reflection.rbs",
+    "sig/prism/serialize.rbs",
+    "sig/prism/visitor.rbs",
+    "src/diagnostic.c",
+    "src/encoding.c",
+    "src/node.c",
+    "src/options.c",
+    "src/pack.c",
+    "src/prettyprint.c",
+    "src/prism.c",
+    "src/regexp.c",
+    "src/serialize.c",
+    "src/static_literals.c",
+    "src/token_type.c",
+    "src/util/pm_buffer.c",
+    "src/util/pm_char.c",
+    "src/util/pm_constant_pool.c",
+    "src/util/pm_integer.c",
+    "src/util/pm_list.c",
+    "src/util/pm_memchr.c",
+    "src/util/pm_newline_list.c",
+    "src/util/pm_string_list.c",
+    "src/util/pm_string.c",
+    "src/util/pm_strncasecmp.c",
+    "src/util/pm_strpbrk.c"
  ]

  spec.extensions = ["ext/prism/extconf.rb"]
--- a/prism/templates/lib/prism/inspect_visitor.rb.erb
+++ b/prism/templates/lib/prism/inspect_visitor.rb.erb
@ -0,0 +1,137 @@
+module Prism
+  # This visitor is responsible for composing the strings that get returned by
+  # the various #inspect methods defined on each of the nodes.
+  class InspectVisitor < Visitor
+    # Most of the time, we can simply pass down the indent to the next node.
+    # However, when we are inside a list we want some extra special formatting
+    # when we hit an element in that list. In this case, we have a special
+    # command that replaces the subsequent indent with the given value.
+    class Replace # :nodoc:
+      attr_reader :value
+
+      def initialize(value)
+        @value = value
+      end
+    end
+
+    private_constant :Replace
+
+    # The current prefix string.
+    attr_reader :indent
+
+    # The list of commands that we need to execute in order to compose the
+    # final string.
+    attr_reader :commands
+
+    # Initializes a new instance of the InspectVisitor.
+    def initialize(indent = +"")
+      @indent = indent
+      @commands = []
+    end
+
+    # Compose an inspect string for the given node.
+    def self.compose(node)
+      visitor = new
+      node.accept(visitor)
+      visitor.compose
+    end
+
+    # Compose the final string.
+    def compose
+      buffer = +""
+      replace = nil
+
+      until commands.empty?
+        # @type var command: String | node | Replace
+        # @type var indent: String
+        command, indent = *commands.shift
+
+        case command
+        when String
+          buffer << (replace || indent)
+          buffer << command
+          replace = nil
+        when Node
+          visitor = InspectVisitor.new(indent)
+          command.accept(visitor)
+          @commands = [*visitor.commands, *@commands]
+        when Replace
+          replace = command.value
+        else
+          raise "Unknown command: #{command.inspect}"
+        end
+      end
+
+      buffer
+    end
+    <%- nodes.each do |node| -%>
+
+    # Inspect a <%= node.name %> node.
+    def visit_<%= node.human %>(node)
+      commands << [inspect_node(<%= node.name.inspect %>, node), indent]
+      <%- node.fields.each_with_index do |field, index| -%>
+      <%- pointer = index == node.fields.length - 1 ? "└── " : "├── " -%>
+      <%- preadd = index == node.fields.length - 1 ? "    " : "│   " -%>
+      <%- case field -%>
+      <%- when Prism::Template::NodeListField -%>
+      commands << ["<%= pointer %><%= field.name %>: (length: #{(<%= field.name %> = node.<%= field.name %>).length})\n", indent]
+      if <%= field.name %>.any?
+        <%= field.name %>[0...-1].each do |child|
+          commands << [Replace.new("#{indent}<%= preadd %>├── "), indent]
+          commands << [child, "#{indent}<%= preadd %>│   "]
+        end
+        commands << [Replace.new("#{indent}<%= preadd %>└── "), indent]
+        commands << [<%= field.name %>[-1], "#{indent}<%= preadd %>    "]
+      end
+      <%- when Prism::Template::NodeField -%>
+      commands << ["<%= pointer %><%= field.name %>:\n", indent]
+      commands << [node.<%= field.name %>, "#{indent}<%= preadd %>"]
+      <%- when Prism::Template::OptionalNodeField -%>
+      if (<%= field.name %> = node.<%= field.name %>).nil?
+        commands << ["<%= pointer %><%= field.name %>: ∅\n", indent]
+      else
+        commands << ["<%= pointer %><%= field.name %>:\n", indent]
+        commands << [<%= field.name %>, "#{indent}<%= preadd %>"]
+      end
+      <%- when Prism::Template::ConstantField, Prism::Template::ConstantListField, Prism::Template::StringField, Prism::Template::UInt8Field, Prism::Template::UInt32Field, Prism::Template::IntegerField, Prism::Template::DoubleField -%>
+      commands << ["<%= pointer %><%= field.name %>: #{node.<%= field.name %>.inspect}\n", indent]
+      <%- when Prism::Template::OptionalConstantField -%>
+      if (<%= field.name %> = node.<%= field.name %>).nil?
+        commands << ["<%= pointer %><%= field.name %>: ∅\n", indent]
+      else
+        commands << ["<%= pointer %><%= field.name %>: #{<%= field.name %>.inspect}\n", indent]
+      end
+      <%- when Prism::Template::FlagsField -%>
+      <%- flag = flags.find { |flag| flag.name == field.kind }.tap { |flag| raise unless flag } -%>
+      flags = [<%= flag.values.map { |value| "(\"#{value.name.downcase}\" if node.#{value.name.downcase}?)" }.join(", ") %>].compact
+      commands << ["<%= pointer %><%= field.name %>: #{flags.empty? ? "∅" : flags.join(", ")}\n", indent]
+      <%- when Prism::Template::LocationField, Prism::Template::OptionalLocationField -%>
+      commands << ["<%= pointer %><%= field.name %>: #{inspect_location(node.<%= field.name %>)}\n", indent]
+      <%- end -%>
+      <%- end -%>
+    end
+    <%- end -%>
+
+    private
+
+    # Compose a header for the given node.
+    def inspect_node(name, node)
+      result = +"@ #{name} ("
+
+      location = node.location
+      result << "location: (#{location.start_line},#{location.start_column})-(#{location.end_line},#{location.end_column})"
+      result << ", newline: true" if node.newline?
+
+      result << ")\n"
+    end
+
+    # Compose a string representing the given inner location field.
+    def inspect_location(location)
+      if location
+        "(#{location.start_line},#{location.start_column})-(#{location.end_line},#{location.end_column}) = #{location.slice.inspect}"
+      else
+        "∅"
+      end
+    end
+  end
+end
--- a/prism/templates/lib/prism/node.rb.erb
+++ b/prism/templates/lib/prism/node.rb.erb
@ -110,7 +110,7 @@ module Prism
    end

    # Returns a string representation of the node.
-    def inspect(inspector = NodeInspector.new)
+    def inspect
      raise NoMethodError, "undefined method `inspect' for #{inspect}"
    end

@ -280,45 +280,9 @@ module Prism
    <%- end -%>
    <%- end -%>

-    # def inspect(NodeInspector inspector) -> String
-    def inspect(inspector = NodeInspector.new)
-      inspector << inspector.header(self)
-      <%- node.fields.each_with_index do |field, index| -%>
-      <%- pointer, preadd = index == node.fields.length - 1 ? ["└── ", "    "] : ["├── ", "│   "] -%>
-      <%- case field -%>
-      <%- when Prism::Template::NodeListField -%>
-      inspector << "<%= pointer %><%= field.name %>: #{inspector.list("#{inspector.prefix}<%= preadd %>", <%= field.name %>)}"
-      <%- when Prism::Template::ConstantListField -%>
-      inspector << "<%= pointer %><%= field.name %>: #{<%= field.name %>.inspect}\n"
-      <%- when Prism::Template::NodeField -%>
-      inspector << "<%= pointer %><%= field.name %>:\n"
-      inspector << inspector.child_node(<%= field.name %>, "<%= preadd %>")
-      <%- when Prism::Template::OptionalNodeField -%>
-      if (<%= field.name %> = self.<%= field.name %>).nil?
-        inspector << "<%= pointer %><%= field.name %>: ∅\n"
-      else
-        inspector << "<%= pointer %><%= field.name %>:\n"
-        inspector << <%= field.name %>.inspect(inspector.child_inspector("<%= preadd %>")).delete_prefix(inspector.prefix)
-      end
-      <%- when Prism::Template::ConstantField, Prism::Template::StringField, Prism::Template::UInt8Field, Prism::Template::UInt32Field, Prism::Template::IntegerField, Prism::Template::DoubleField -%>
-      inspector << "<%= pointer %><%= field.name %>: #{<%= field.name %>.inspect}\n"
-      <%- when Prism::Template::OptionalConstantField -%>
-      if (<%= field.name %> = self.<%= field.name %>).nil?
-        inspector << "<%= pointer %><%= field.name %>: ∅\n"
-      else
-        inspector << "<%= pointer %><%= field.name %>: #{<%= field.name %>.inspect}\n"
-      end
-      <%- when Prism::Template::FlagsField -%>
-      <%- flag = flags.find { |flag| flag.name == field.kind }.tap { |flag| raise unless flag } -%>
-      flags = [<%= flag.values.map { |value| "(\"#{value.name.downcase}\" if #{value.name.downcase}?)" }.join(", ") %>].compact
-      inspector << "<%= pointer %><%= field.name %>: #{flags.empty? ? "∅" : flags.join(", ")}\n"
-      <%- when Prism::Template::LocationField, Prism::Template::OptionalLocationField -%>
-      inspector << "<%= pointer %><%= field.name %>: #{inspector.location(<%= field.name %>)}\n"
-      <%- else -%>
-      <%- raise -%>
-      <%- end -%>
-      <%- end -%>
-      inspector.to_str
+    # def inspect -> String
+    def inspect
+      InspectVisitor.compose(self)
    end

    # Sometimes you want to check an instance of a node against a list of
--- a/prism/templates/lib/prism/reflection.rb.erb
+++ b/prism/templates/lib/prism/reflection.rb.erb
@ -118,7 +118,7 @@ module Prism
          when Prism::Template::OptionalLocationField
            "OptionalLocationField.new(:#{field.name})"
          when Prism::Template::UInt8Field, Prism::Template::UInt32Field, Prism::Template::IntegerField
-            "Integer.new(:#{field.name})"
+            "IntegerField.new(:#{field.name})"
          when Prism::Template::DoubleField
            "FloatField.new(:#{field.name})"
          when Prism::Template::FlagsField
--- a/prism/templates/template.rb
+++ b/prism/templates/template.rb
@ -629,6 +629,7 @@ module Prism
      "lib/prism/dispatcher.rb",
      "lib/prism/dot_visitor.rb",
      "lib/prism/dsl.rb",
+      "lib/prism/inspect_visitor.rb",
      "lib/prism/mutation_compiler.rb",
      "lib/prism/node.rb",
      "lib/prism/reflection.rb",