crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.34k stars 1.61k forks source link

Overridden == being ignored under particular circumstances #7105

Open ezrast opened 5 years ago

ezrast commented 5 years ago

Crystal is using Reference#== instead of Derived#== in the last line of the following code:

class Base
  getter foo : Base?  # One

  def ==(other : self)
    puts "Using overridden =="
    {% for ivar in @type.instance_vars %}   #
      {{ ivar.id }} == other.{{ ivar.id }}  # Two
    {% end %}                               #
    return true
  end
end

class Derived < Base
end

if false
  {Derived.new => nil}  # Three
end

p Derived.new == Derived.new # => false, should be true

Commenting out any of the sections marked One, Two, Three will cause the override for == to be used, which I believe is the correct behavior.

carc.in link: https://carc.in/#/r/5lvi

$  crystal --version
Crystal 0.27.0 (2018-11-02)

LLVM: 6.0.1
Default target: x86_64-pc-linux-gnu
konovod commented 5 years ago

I think the == here has infinite recursion - to compare Base you have to compare Base first (in a foo field). This doesn't answers why operator isn't used but can be linked. Also, section Three can be replaced with Base.new == Base.new (equality is used inside Hash implementation)

Blacksmoke16 commented 5 years ago

Another option here would be to include Comparable(Base) and implement a <=> method.

https://crystal-lang.org/api/0.27.0/Comparable.html

asterite commented 5 years ago

The self restriction here causes problems, coupled with the macro method. It's a known issue (to me), I don't know why it happens. It happened in Struct too. You can use this as a workaround.

HertzDevil commented 3 years ago

Might be related to #6760

HertzDevil commented 3 years ago

Reduced:

class A
  def bar(other)
    1
  end
end

class B
  def bar(other : self)
    2
  end

  def bar(other)
    3
  end
end

class C < B
  def foo
    A.new.as(A | C)
  end

  def bar(other : self)
    {% @type %}
    foo.bar other.foo
    4
  end
end

class D < C
end

def test
  {% D.methods %} # => []
  D.new.bar(D.new) # => 4
  {% D.methods %} # => [def bar(other : self); 2; end, def bar(other); 3; end]
  D.new.bar(D.new) # => 2
end

test

Non-macro defs should never have to be copied to subtypes.