From c59201078b545d4168e3ea8ba882d849b922d460 Mon Sep 17 00:00:00 2001
From: Ary Borenszweig <aborenszweig@manas.com.ar>
Date: Sun, 3 Aug 2014 12:05:48 -0300
Subject: [PATCH] Simplified how defs are stored

---
 spec/compiler/codegen/fun_spec.cr             |  14 ++
 spec/compiler/type_inference/class_spec.cr    |  22 +++
 src/compiler/crystal/primitives.cr            |   2 +-
 src/compiler/crystal/type_inference/call.cr   |   2 +-
 .../crystal/type_inference/restrictions.cr    |  10 +-
 src/compiler/crystal/types.cr                 | 132 ++++++++----------
 6 files changed, 99 insertions(+), 83 deletions(-)

diff --git a/spec/compiler/codegen/fun_spec.cr b/spec/compiler/codegen/fun_spec.cr
index 9df89542dd..bf2bd400e1 100644
--- a/spec/compiler/codegen/fun_spec.cr
+++ b/spec/compiler/codegen/fun_spec.cr
@@ -447,4 +447,18 @@ describe "Code gen: fun" do
       f.call Bar.new
       )).to_i.should eq(2)
   end
+
+  it "allows redefining fun" do
+    run(%(
+      fun foo : Int32
+        1
+      end
+
+      fun foo : Int32
+        2
+      end
+
+      foo
+      )).to_i.should eq(2)
+  end
 end
diff --git a/spec/compiler/type_inference/class_spec.cr b/spec/compiler/type_inference/class_spec.cr
index f8ceab44a8..ce5068f0b2 100755
--- a/spec/compiler/type_inference/class_spec.cr
+++ b/spec/compiler/type_inference/class_spec.cr
@@ -661,4 +661,26 @@ describe "Type inference: class" do
       ),
       "can't instantiate abstract class Foo"
   end
+
+  it "doesn't mix types of instance vars with initialize and new" do
+    assert_type(%(
+      class Foo
+        def initialize(x = 1)
+          @x = x
+        end
+
+        def self.new(x : String)
+          new(1)
+        end
+
+        def x
+          @x
+        end
+      end
+
+      Foo.new
+      Foo.new(1)
+      Foo.new("hello").x
+      )) { int32 }
+  end
 end
diff --git a/src/compiler/crystal/primitives.cr b/src/compiler/crystal/primitives.cr
index 910614cf5a..528dfb4d01 100644
--- a/src/compiler/crystal/primitives.cr
+++ b/src/compiler/crystal/primitives.cr
@@ -52,7 +52,7 @@ module Crystal
         end
       end
 
-      %w(to_i to_i8 to_i32 to_i16 to_i32 to_i64 to_u to_u8 to_u16 to_u32 to_u64 to_f to_f32 to_f64).each do |op|
+      %w(to_i to_i8 to_i16 to_i32 to_i64 to_u to_u8 to_u16 to_u32 to_u64 to_f to_f32 to_f64).each do |op|
         number.add_def Def.new(op, [] of Arg, cast)
       end
 
diff --git a/src/compiler/crystal/type_inference/call.cr b/src/compiler/crystal/type_inference/call.cr
index 7493868a55..8a6bff410b 100644
--- a/src/compiler/crystal/type_inference/call.cr
+++ b/src/compiler/crystal/type_inference/call.cr
@@ -339,7 +339,7 @@ module Crystal
       end
 
       match = Match.new(previous, args.map(&.type), MatchContext.new(scope, scope))
-      matches = Matches.new([match], true)
+      matches = Matches.new([match] of Match, true)
       typed_defs = instantiate matches, scope
       typed_defs.each do |typed_def|
         typed_def.next = parent_visitor.typed_def
diff --git a/src/compiler/crystal/type_inference/restrictions.cr b/src/compiler/crystal/type_inference/restrictions.cr
index 55d6eaf060..106e397ad5 100644
--- a/src/compiler/crystal/type_inference/restrictions.cr
+++ b/src/compiler/crystal/type_inference/restrictions.cr
@@ -26,9 +26,12 @@ module Crystal
     end
   end
 
-  class Def
-    def is_restriction_of?(other : Def, owner)
-      args.zip(other.args) do |self_arg, other_arg|
+  struct DefWithMetadata
+    def is_restriction_of?(other : DefWithMetadata, owner)
+      return false unless length == other.length
+      return false unless yields == other.yields
+
+      self.def.args.zip(other.def.args) do |self_arg, other_arg|
         self_type = self_arg.type? || self_arg.restriction
         other_type = other_arg.type? || other_arg.restriction
         return false if self_type == nil && other_type != nil
@@ -40,7 +43,6 @@ module Crystal
     end
   end
 
-
   class Path
     def is_restriction_of?(other : Path, owner)
       return true if self == other
diff --git a/src/compiler/crystal/types.cr b/src/compiler/crystal/types.cr
index d758906b67..c1f9528bba 100644
--- a/src/compiler/crystal/types.cr
+++ b/src/compiler/crystal/types.cr
@@ -233,10 +233,6 @@ module Crystal
       raise "Bug: #{self} doesn't implement defs"
     end
 
-    def sorted_defs
-      raise "Bug: #{self} doesn't implement sorted_defs"
-    end
-
     def add_def(a_def)
       raise "Bug: #{self} doesn't implement add_def"
     end
@@ -478,24 +474,29 @@ module Crystal
     end
 
     def lookup_matches_without_parents(name, arg_types, block, owner = self, type_lookup = self, matches_array = nil)
-      if sorted_defs = self.sorted_defs()
-        if defs = sorted_defs[DefContainer::SortedDefKey.new(name, arg_types.length, !!block)]?
+      if all_defs = self.defs()
+        if defs = all_defs[name]?
           found_defs = true
+          args_length = arg_types.length
+          yields = !!block
           context = MatchContext.new(owner, type_lookup)
 
-          defs.each do |a_def|
-            match = match_def_args(arg_types, a_def, context)
-
-            if match
-              matches_array ||= [] of Match
-              matches_array.push match
-
-              # If the argument types are compatible with the match's argument types,
-              # we are done. We don't just compare types with ==, there is a special case:
-              # a function type with return T can be transpass a restriction of a function
-              # with with the same arguments but which returns Void.
-              if arg_types.equals?(match.arg_types) { |x, y| x.compatible_with?(y) }
-                return Matches.new(matches_array, true, owner)
+          defs.each do |item|
+            if item.length == args_length && item.yields == yields
+              a_def = item.def
+              match = match_def_args(arg_types, a_def, context)
+
+              if match
+                matches_array ||= [] of Match
+                matches_array.push match
+
+                # If the argument types are compatible with the match's argument types,
+                # we are done. We don't just compare types with ==, there is a special case:
+                # a function type with return T can be transpass a restriction of a function
+                # with with the same arguments but which returns Void.
+                if arg_types.equals?(match.arg_types) { |x, y| x.compatible_with?(y) }
+                  return Matches.new(matches_array, true, owner)
+                end
               end
             end
           end
@@ -543,14 +544,15 @@ module Crystal
 
     def lookup_first_def(name, block)
       block = !!block
-      if (defs = self.defs) && (hash = defs[name]?)
-        hash.values.find { |a_def| !!a_def.yields == block }
+      if (defs = self.defs) && (list = defs[name]?)
+        value = list.find { |item| item.yields == block }
+        value.try &.def
       end
     end
 
     def lookup_defs(name)
-      if (defs = self.defs) && (hash = defs[name]?)
-        return hash.values unless hash.empty?
+      if (defs = self.defs) && (list = defs[name]?)
+        return list.map(&.def) unless list.empty?
       end
 
       parents.try &.each do |parent|
@@ -562,8 +564,8 @@ module Crystal
     end
 
     def lookup_defs_with_modules(name)
-      if (defs = self.defs) && (hash = defs[name]?)
-        return hash.values unless hash.empty?
+      if (defs = self.defs) && (list = defs[name]?)
+        return list.map(&.def) unless list.empty?
       end
 
       parents.try &.each do |parent|
@@ -588,7 +590,7 @@ module Crystal
         defs.each do |def_name, hash|
           if def_name =~ SuggestableName
             hash.each do |filter, overload|
-              if filter.restrictions.length == args_length && filter.yields == !!block
+              if filter.length == args_length && filter.yields == !!block
                 if levenshtein(def_name, name) <= tolerance
                   candidates << def_name
                 end
@@ -689,69 +691,48 @@ module Crystal
     end
   end
 
+  make_named_tuple DefWithMetadata, [length, yields, :def]
+
   module DefContainer
     include MatchesLookup
 
-    make_named_tuple DefKey, [restrictions, yields]
-    make_named_tuple SortedDefKey, [name, length, yields]
     make_named_tuple Hook, [kind, :macro]
 
     getter defs
-    getter sorted_defs
     getter macros
     getter hooks
 
     def add_def(a_def)
       a_def.owner = self
-      restrictions = Array(Type | ASTNode | Nil).new(a_def.args.length)
-      a_def.args.each { |arg| restrictions.push(arg.type? || arg.restriction) }
-      key = DefKey.new(restrictions, !!a_def.yields)
-
-      defs = (@defs ||= {} of String => Hash(DefKey, Def))
-      hash = (defs[a_def.name] ||= {} of DefKey => Def)
-      old_def = hash[key]?
-      hash[key] = a_def
-
-      add_sorted_def(a_def)
-
-      a_def.previous = old_def
-      old_def
-    end
-
-    def add_sorted_def(a_def)
-      sorted_defs = (@sorted_defs ||= {} of SortedDefKey => Array(Def))
-      key = SortedDefKey.new(a_def.name, a_def.args.length, !!a_def.yields)
-      list = (sorted_defs[key] ||= [] of Def)
-      list.each_with_index do |ex_def, i|
-        if a_def.is_restriction_of?(ex_def, self)
-          list.insert(i, a_def)
-          return
+      item = DefWithMetadata.new(a_def.args.length, !!a_def.yields, a_def)
+
+      defs = (@defs ||= {} of String => Array(DefWithMetadata))
+      list = defs[a_def.name] ||= [] of DefWithMetadata
+      list.each_with_index do |ex_item, i|
+        if item.is_restriction_of?(ex_item, self)
+          if ex_item.is_restriction_of?(item, self)
+            list[i] = item
+            a_def.previous = ex_item.def
+            return ex_item.def
+          else
+            list.insert(i, item)
+            return nil
+          end
         end
       end
-      list << a_def
+      list << item
+      nil
     end
 
     def undef(def_name)
-      found_def = @defs.try &.delete def_name
-      return false unless found_def
+      found_list = @defs.try &.delete def_name
+      return false unless found_list
 
-      found_def.each do |key, a_def|
+      found_list.each do |item|
+        a_def = item.def
         a_def.dead = true if a_def.is_a?(External)
       end
 
-      if sorted_defs = @sorted_defs
-        keys_to_remove = [] of SortedDefKey
-        sorted_defs.each do |key, defs|
-          if key.name == def_name
-            keys_to_remove << key
-          end
-        end
-
-        keys_to_remove.each do |key|
-          sorted_defs.delete(key)
-        end
-      end
-
       true
     end
 
@@ -1116,9 +1097,9 @@ module Crystal
 
     def transfer_instance_vars_of_mod(mod)
       if (defs = mod.defs)
-        defs.each do |def_name, hash|
-          hash.each do |restrictions, a_def|
-            transfer_instance_vars a_def
+        defs.each do |def_name, list|
+          list.each do |item|
+            transfer_instance_vars item.def
           end
         end
       end
@@ -1617,7 +1598,6 @@ module Crystal
 
     delegate depth, @generic_class
     delegate defs, @generic_class
-    delegate sorted_defs, @generic_class
     delegate superclass, @generic_class
     delegate owned_instance_vars, @generic_class
     delegate instance_vars_in_initialize, @generic_class
@@ -1967,9 +1947,9 @@ module Crystal
     def add_def(a_def : External)
       if defs = self.defs
         if existing_defs = defs[a_def.name]?
-          existing = existing_defs.first_value?
+          existing = existing_defs.first?
           if existing
-            existing = existing as External
+            existing = existing.def as External
             unless existing.compatible_with?(a_def)
               raise "fun redefinition with different signature (was #{existing})"
             end
@@ -2022,7 +2002,6 @@ module Crystal
 
     delegate pointer?, typedef
     delegate defs, typedef
-    delegate sorted_defs, typedef
     delegate macros, typedef
 
     def lookup_type(names : Array, already_looked_up = TypeIdSet.new, lookup_in_container = true)
@@ -2327,7 +2306,6 @@ module Crystal
     end
 
     delegate defs, :"instance_type.generic_class.metaclass"
-    delegate sorted_defs, :"instance_type.generic_class.metaclass"
     delegate macros, :"instance_type.generic_class.metaclass"
     delegate type_vars, instance_type
     delegate :abstract, instance_type
-- 
GitLab