From bfc33dbda41ed4c3590d02bcdd781fcea5368fd3 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 7 Sep 2022 06:50:06 -0300 Subject: [PATCH] Compiler: refactor and slightly optimize merging two types (#12436) * Use early returns in type_merge_two * Don't flip types in type_merge_two * Don't return tuple in type_merge_two * Add some type annotations * Rename type_merge_two to just type_merge * Avoid creating intermediate array in Type.merge! * Add a few more type annotations * Generalize the case of a non-union type merged with a union type * Consider NoReturn when merging two types * Use suffix if and add a few more comments * Don't create intermediate arrays when calling Type.merge! --- .../semantic/type_declaration_processor.cr | 6 +- .../crystal/semantic/type_guess_visitor.cr | 2 +- src/compiler/crystal/semantic/type_merge.cr | 88 +++++++++---------- 3 files changed, 44 insertions(+), 52 deletions(-) diff --git a/src/compiler/crystal/semantic/type_declaration_processor.cr b/src/compiler/crystal/semantic/type_declaration_processor.cr index 1fd1cf5aa36b..79ef150f908d 100644 --- a/src/compiler/crystal/semantic/type_declaration_processor.cr +++ b/src/compiler/crystal/semantic/type_declaration_processor.cr @@ -406,7 +406,7 @@ struct Crystal::TypeDeclarationProcessor when NonGenericModuleType type = type_info.type if nilable_instance_var?(owner, name) - type = Type.merge!([type, @program.nil]) + type = Type.merge!(type, @program.nil) end # Same as above, only Nil makes no sense @@ -423,7 +423,7 @@ struct Crystal::TypeDeclarationProcessor when GenericClassType type = type_info.type if nilable_instance_var?(owner, name) - type = Type.merge!([type, @program.nil]) + type = Type.merge!(type, @program.nil) end # Same as above, only Nil makes no sense @@ -442,7 +442,7 @@ struct Crystal::TypeDeclarationProcessor when GenericModuleType type = type_info.type if nilable_instance_var?(owner, name) - type = Type.merge!([type, @program.nil]) + type = Type.merge!(type, @program.nil) end declare_meta_type_var(owner.instance_vars, owner, name, type, type_info.location, instance_var: true, annotations: type_info.annotations) diff --git a/src/compiler/crystal/semantic/type_guess_visitor.cr b/src/compiler/crystal/semantic/type_guess_visitor.cr index b9af913f93c6..61f0d4480509 100644 --- a/src/compiler/crystal/semantic/type_guess_visitor.cr +++ b/src/compiler/crystal/semantic/type_guess_visitor.cr @@ -419,7 +419,7 @@ module Crystal info.add_annotations(annotations) if annotations vars[name] = info else - info.type = Type.merge!([info.type, type]) + info.type = Type.merge!(info.type, type) info.outside_def = true if @outside_def info.add_annotations(annotations) if annotations vars[name] = info diff --git a/src/compiler/crystal/semantic/type_merge.cr b/src/compiler/crystal/semantic/type_merge.cr index 8096ee9de839..5710d425bf8b 100644 --- a/src/compiler/crystal/semantic/type_merge.cr +++ b/src/compiler/crystal/semantic/type_merge.cr @@ -2,80 +2,72 @@ require "../program" module Crystal class Program - def type_merge(types : Array(Type?)) + def type_merge(types : Array(Type?)) : Type? case types.size when 0 - return nil + nil when 1 - return types.first + types.first when 2 # Merging two types is the most common case, so we optimize it first, second = types - did_merge, merged_type = type_merge_two(first, second) - return merged_type if did_merge + type_merge(first, second) else - # combined_union_of + combined_union_of compact_types(types) end - - combined_union_of compact_types(types) end - def type_merge(nodes : Array(ASTNode)) + def type_merge(nodes : Array(ASTNode)) : Type? case nodes.size when 0 - return nil + nil when 1 - return nodes.first.type? + nodes.first.type? when 2 # Merging two types is the most common case, so we optimize it first, second = nodes - did_merge, merged_type = type_merge_two(first.type?, second.type?) - return merged_type if did_merge + type_merge(first.type?, second.type?) else - # combined_union_of + combined_union_of compact_types(nodes, &.type?) end - - combined_union_of compact_types(nodes, &.type?) end - def type_merge_two(first, second) - if first == second - # Same, so return any of them - {true, first} - elsif first - if second - # first and second not nil and different - if first.opaque_id > second.opaque_id - first, second = second, first - end + def type_merge(first : Type?, second : Type?) : Type? + # Same, so return any of them + return first if first == second - if first.nil_type? - if second.is_a?(UnionType) && second.union_types.includes?(first) - return true, second - end - end + # First is nil, so return second + return second unless first - # puts "#{first} vs. #{second}" - {false, nil} - else - # Second is nil, so return first - {true, first} - end - else - # First is nil, so return second - {true, second} + # Second is nil, so return first + return first unless second + + # NoReturn is removed from unions + return second if first.no_return? + return first if second.no_return? + + # Check if a non-union type is part of a union type + if !first.is_a?(UnionType) && second.is_a?(UnionType) && second.union_types.includes?(first) + return second end + + if !second.is_a?(UnionType) && first.is_a?(UnionType) && first.union_types.includes?(second) + return first + end + + # General case + combined_union_of compact_types({first, second}) end - def type_merge_union_of(types : Array(Type)) + def type_merge_union_of(types : Array(Type)) : Type? union_of compact_types(types) end - def compact_types(types) + def compact_types(types) : Array(Type) compact_types(types) { |type| type } end - def compact_types(objects) + def compact_types(objects) : Array(Type) all_types = Array(Type).new(objects.size) objects.each { |obj| add_type all_types, yield(obj) } all_types.reject! &.no_return? if all_types.size > 1 @@ -170,11 +162,11 @@ module Crystal end class Type - def self.merge(nodes : Array(ASTNode)) + def self.merge(nodes : Array(ASTNode)) : Type? nodes.find(&.type?).try &.type.program.type_merge(nodes) end - def self.merge(types : Array(Type)) + def self.merge(types : Array(Type)) : Type? if types.size == 0 nil else @@ -182,12 +174,12 @@ module Crystal end end - def self.merge!(types_or_nodes) + def self.merge!(types_or_nodes) : Type merge(types_or_nodes).not_nil! end - def self.merge!(type1 : Type, type2 : Type) - merge!([type1, type2]) + def self.merge!(type1 : Type, type2 : Type) : Type + type1.program.type_merge(type1, type2).not_nil! end # Given two non-union types T and U, returns their least common ancestor