From 1f83679f06ead4f4489c0e0f43abdd6027605d2b Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Fri, 11 Sep 2020 16:03:45 -0400 Subject: [PATCH] Optimize some `NodePattern`s by using `Set`s --- CHANGELOG.md | 1 + lib/rubocop/ast.rb | 1 + lib/rubocop/ast/node_pattern/builder.rb | 7 +++- .../node_pattern/compiler/atom_subcompiler.rb | 5 +++ lib/rubocop/ast/node_pattern/node.rb | 9 +++++ lib/rubocop/ast/node_pattern/sets.rb | 37 +++++++++++++++++++ spec/rubocop/ast/node_pattern/parser_spec.rb | 10 +++++ spec/rubocop/ast/node_pattern/sets_spec.rb | 17 +++++++++ 8 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 lib/rubocop/ast/node_pattern/sets.rb create mode 100644 spec/rubocop/ast/node_pattern/sets_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 242731104..9c067e2ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * [#105](https://github.com/rubocop-hq/rubocop-ast/pull/105): `NodePattern` compiler [complete rewrite](https://docs.rubocop.org/rubocop-ast/node_pattern_compiler.html). Add support for multiple variadic terms. ([@marcandre][]) * [#109](https://github.com/rubocop-hq/rubocop-ast/pull/109): Add `NodePattern` debugging rake tasks: `test_pattern`, `compile`, `parse`. See also [this app](https://nodepattern.herokuapp.com) ([@marcandre][]) * [#110](https://github.com/rubocop-hq/rubocop-ast/pull/110): Add `NodePattern` support for multiple terms unions. ([@marcandre][]) +* [#111](https://github.com/rubocop-hq/rubocop-ast/pull/111): Optimize some `NodePattern`s by using `Set`s. ([@marcandre][]) ## 0.6.0 (2020-09-26) diff --git a/lib/rubocop/ast.rb b/lib/rubocop/ast.rb index 2cb68611c..9b8c8be33 100644 --- a/lib/rubocop/ast.rb +++ b/lib/rubocop/ast.rb @@ -20,6 +20,7 @@ require_relative 'ast/node_pattern/lexer' require_relative 'ast/node_pattern/node' require_relative 'ast/node_pattern/parser' +require_relative 'ast/node_pattern/sets' require_relative 'ast/sexp' require_relative 'ast/node' require_relative 'ast/node/mixin/method_identifier_predicates' diff --git a/lib/rubocop/ast/node_pattern/builder.rb b/lib/rubocop/ast/node_pattern/builder.rb index 13fdba392..71fc6f5fe 100644 --- a/lib/rubocop/ast/node_pattern/builder.rb +++ b/lib/rubocop/ast/node_pattern/builder.rb @@ -34,7 +34,8 @@ def emit_call(type, selector, args = nil) def emit_union(begin_t, pattern_lists, end_t) children = union_children(pattern_lists) - emit_list(:union, begin_t, children, end_t) + type = optimizable_as_set?(children) ? :set : :union + emit_list(type, begin_t, children, end_t) end def emit_subsequence(node_list) @@ -45,6 +46,10 @@ def emit_subsequence(node_list) private + def optimizable_as_set?(children) + children.all?(&:matches_within_set?) + end + def n(type, *args) Node::MAP[type].new(type, *args) end diff --git a/lib/rubocop/ast/node_pattern/compiler/atom_subcompiler.rb b/lib/rubocop/ast/node_pattern/compiler/atom_subcompiler.rb index bca1875fb..027827d4f 100644 --- a/lib/rubocop/ast/node_pattern/compiler/atom_subcompiler.rb +++ b/lib/rubocop/ast/node_pattern/compiler/atom_subcompiler.rb @@ -36,6 +36,11 @@ def visit_positional_parameter compiler.positional_parameter(node.child) end + def visit_set + set = node.children.map(&:child).to_set.freeze + NodePattern::Sets[set] + end + # Assumes other types are node patterns. def visit_other_type compiler.with_temp_variables do |compare| diff --git a/lib/rubocop/ast/node_pattern/node.rb b/lib/rubocop/ast/node_pattern/node.rb index 4d105ed4e..2795a80ba 100644 --- a/lib/rubocop/ast/node_pattern/node.rb +++ b/lib/rubocop/ast/node_pattern/node.rb @@ -8,6 +8,9 @@ class Node < ::Parser::AST::Node extend Forwardable include ::RuboCop::AST::Descendence + MATCHES_WITHIN_SET = %i[symbol number string].to_set.freeze + private_constant :MATCHES_WITHIN_SET + ### # To be overriden by subclasses ### @@ -55,6 +58,12 @@ def variadic? arity.is_a?(Range) end + # @return [Boolean] returns true for nodes having a Ruby literal equivalent + # that matches withing a Set (e.g. `42`, `:sym` but not `/regexp/`) + def matches_within_set? + MATCHES_WITHIN_SET.include?(type) + end + # @return [Range] arity as a Range def arity_range a = arity diff --git a/lib/rubocop/ast/node_pattern/sets.rb b/lib/rubocop/ast/node_pattern/sets.rb new file mode 100644 index 000000000..809b58217 --- /dev/null +++ b/lib/rubocop/ast/node_pattern/sets.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module RuboCop + module AST + class NodePattern + # Utility to assign a set of values to a constant + module Sets + REGISTRY = Hash.new do |h, set| + name = Sets.name(set).freeze + Sets.const_set(name, set) + h[set] = "::RuboCop::AST::NodePattern::Sets::#{name}" + end + + MAX = 4 + def self.name(set) + elements = set + elements = set.first(MAX - 1) << :etc if set.size > MAX + name = elements.to_a.join('_').upcase.gsub(/[^A-Z0-9_]/, '') + uniq("SET_#{name}") + end + + def self.uniq(name) + return name unless Sets.const_defined?(name) + + (2..Float::INFINITY).each do |i| + uniq = "#{name}_#{i}" + return uniq unless Sets.const_defined?(uniq) + end + end + + def self.[](set) + REGISTRY[set] + end + end + end + end +end diff --git a/spec/rubocop/ast/node_pattern/parser_spec.rb b/spec/rubocop/ast/node_pattern/parser_spec.rb index 05877d154..a95bdd11e 100644 --- a/spec/rubocop/ast/node_pattern/parser_spec.rb +++ b/spec/rubocop/ast/node_pattern/parser_spec.rb @@ -61,6 +61,16 @@ ) end + it 'parses unions of literals as a set' do + expect_parsing( + s(:sequence, s(:set, s(:symbol, :a), s(:number, 42), s(:string, 'hello'))), + '({:a 42 "hello"})', + ' ^ begin (set) + | ^ end (set) + | ~~ expression (set/1.number)' + ) + end + it 'generates specialized nodes' do source_file = Parser::Source::Buffer.new('(spec)', source: '($_)') ast = parser.parse(source_file) diff --git a/spec/rubocop/ast/node_pattern/sets_spec.rb b/spec/rubocop/ast/node_pattern/sets_spec.rb new file mode 100644 index 000000000..888b2de5f --- /dev/null +++ b/spec/rubocop/ast/node_pattern/sets_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::AST::NodePattern::Sets do + subject(:name) { described_class[set] } + + let(:set) { Set[1, 2, 3, 4, 5, 6] } + + it { is_expected.to eq '::RuboCop::AST::NodePattern::Sets::SET_1_2_3_ETC' } + + it { is_expected.to eq described_class[Set[6, 5, 4, 3, 2, 1]] } + + it { is_expected.not_to eq described_class[Set[1, 2, 3, 4, 5, 6, 7]] } + + it 'creates a constant with the right value' do + expect(eval(name)).to eq set # rubocop:disable Security/Eval + end +end