Skip to content

Commit

Permalink
Compiler: fix new/initialize lookup regarding modules
Browse files Browse the repository at this point in the history
  • Loading branch information
asterite committed May 24, 2019
1 parent cd50377 commit 3f03066
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 6 deletions.
16 changes: 16 additions & 0 deletions spec/compiler/semantic/module_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -1352,4 +1352,20 @@ describe "Semantic: module" do
Gen(Int32).foo
)) { int32.metaclass }
end

it "doesn't look up initialize past module that defines initialize (#7007)" do
assert_error %(
module Moo
def initialize(x)
end
end
class Foo
include Moo
end
Foo.new
),
"wrong number of arguments"
end
end
4 changes: 2 additions & 2 deletions src/compiler/crystal/semantic/call_error.cr
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ class Crystal::Call
end
end

# Another special case: initialize is only looked up one level,
# Another special case: `new` and `initialize` are only looked up one level,
# so we must find the first one defined.
new_owner = owner
while defs.empty? && def_name == "initialize"
while defs.empty? && (def_name == "initialize" || def_name == "new")
new_owner = new_owner.superclass
if new_owner
defs = new_owner.lookup_defs(def_name)
Expand Down
7 changes: 6 additions & 1 deletion src/compiler/crystal/semantic/method_lookup.cr
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,19 @@ module Crystal
# `new` must only be searched in ancestors if this type itself doesn't define
# an `initialize` or `self.new` method. This was already computed in `new.cr`
# and can be known by invoking `lookup_new_in_ancestors?`
if my_parents && !(!lookup_new_in_ancestors? && is_new)
if my_parents && !(is_new && !lookup_new_in_ancestors?)
my_parents.each do |parent|
matches = parent.lookup_matches(signature, owner, parent, matches_array)
if matches.cover_all?
return matches
else
matches_array = matches.matches
end

# If this is a `new` method, once a parent defines an `initialize`
# method and we couldn't find any matches we must not go up in the
# hierarchy.
break if is_new && parent.has_def_without_parents?(signature.name)
end
end

Expand Down
21 changes: 18 additions & 3 deletions src/compiler/crystal/types.cr
Original file line number Diff line number Diff line change
Expand Up @@ -393,10 +393,25 @@ module Crystal
all_defs << item.def unless all_defs.find(&.same?(item.def))
end

if lookup_ancestors_for_new || self.lookup_new_in_ancestors? ||
!(name == "new" || name == "initialize")
parents.try &.each do |parent|
is_new = name == "new"
is_new_or_initialize = is_new || name == "initialize"
return if is_new_or_initialize && !all_defs.empty?

if !is_new_or_initialize || (lookup_ancestors_for_new || self.lookup_new_in_ancestors?)
if is_new
# For a `new` method we need to do this in case a `new` is defined
# in a module type
my_parents = instance_type.parents.try &.map(&.metaclass)
else
my_parents = parents
end

my_parents.try &.each do |parent|
old_size = all_defs.size
parent.lookup_defs(name, all_defs, lookup_ancestors_for_new)

# Don't lookup new or initialize in parents once we found some defs
break if is_new_or_initialize && all_defs.size > old_size
end
end
end
Expand Down

0 comments on commit 3f03066

Please sign in to comment.