Skip to content

Commit

Permalink
[ruby/prism] Change inspect from recursive to a queue
Browse files Browse the repository at this point in the history
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.

ruby/prism@0f21f5bfe1
  • Loading branch information
kddnewton committed Apr 24, 2024
1 parent cf24a04 commit 6d9ba1e
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 146 deletions.
2 changes: 1 addition & 1 deletion lib/prism.rb
Expand Up @@ -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"
Expand Down
68 changes: 0 additions & 68 deletions lib/prism/node_inspector.rb

This file was deleted.

74 changes: 38 additions & 36 deletions lib/prism/prism.gemspec
Expand Up @@ -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",
Expand All @@ -101,11 +101,46 @@ Gem::Specification.new do |spec|
"lib/prism/translation/ripper/shim.rb",
"lib/prism/translation/ruby_parser.rb",
"lib/prism/visitor.rb",
"prism.gemspec",
"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",
"rbi/prism/parse_result.rbi",
"rbi/prism/reflection.rbi",
"rbi/prism/translation/parser.rbi",
"rbi/prism/translation/parser/compiler.rbi",
"rbi/prism/translation/parser33.rbi",
"rbi/prism/translation/parser34.rbi",
"rbi/prism/translation/ripper.rbi",
"rbi/prism/translation/ripper/ripper_compiler.rbi",
"rbi/prism/translation/ruby_parser.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",
Expand All @@ -117,43 +152,10 @@ Gem::Specification.new do |spec|
"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_string.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/mutation_compiler.rbi",
"rbi/prism/node_ext.rbi",
"rbi/prism/node.rbi",
"rbi/prism/parse_result.rbi",
"rbi/prism/reflection.rbi",
"rbi/prism/translation/parser.rbi",
"rbi/prism/translation/parser/compiler.rbi",
"rbi/prism/translation/parser33.rbi",
"rbi/prism/translation/parser34.rbi",
"rbi/prism/translation/ripper.rbi",
"rbi/prism/translation/ripper/ripper_compiler.rbi",
"rbi/prism/translation/ruby_parser.rbi",
"rbi/prism/visitor.rbi"
"src/util/pm_strpbrk.c"
]

spec.extensions = ["ext/prism/extconf.rb"]
Expand Down
137 changes: 137 additions & 0 deletions 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
44 changes: 4 additions & 40 deletions prism/templates/lib/prism/node.rb.erb
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion prism/templates/lib/prism/reflection.rb.erb
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions prism/templates/template.rb
Expand Up @@ -629,6 +629,7 @@ def locals
"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",
Expand Down

0 comments on commit 6d9ba1e

Please sign in to comment.