Error: undefined method 'to_unsafe' for Foo (compile-time type is MyModule)

[Blacksmoke16]

Noticed a regression in nightly Athena CI. Was able to reduce it to the following:

require "spec"

module MyModule; end

class Foo
  include MyModule
end

it do
  a = Foo.new
  a.as(MyModule).should be a.as(MyModule)
end

Seems to be a regression from #14728 in that a in this case is typed as an interface module so doesn't actually respond to #object_id or #to_unsafe. At least that's what I'm getting from the full trace:

In test.cr:11:18

 11 | a.as(MyModule).should be a.as(MyModule)
                     ^-----
Error: instantiating 'MyModule#should(Spec::BeExpectation(MyModule))'


In src/spec/expectations.cr:467:41

 467 | failure_message ||= expectation.failure_message(self)
                                       ^--------------
Error: instantiating 'Spec::BeExpectation(MyModule)#failure_message(Foo)'


In src/spec/expectations.cr:68:55

 68 | "Expected: #{@expected_value.pretty_inspect} (#{identify(@expected_value)})\n     got: #{actual_value.pretty_inspect} (#{identify(actual_value)})"
                                                      ^-------
Error: instantiating 'identify(MyModule)'


In src/spec/expectations.cr:79:15

 79 | value.to_unsafe
            ^--------
Error: undefined method 'to_unsafe' for Foo (compile-time type is MyModule)

[straight-shoota]

Reduced further:

module MyModule; end

class Foo
  include MyModule
end

a = Foo.new.as(MyModule)

if a.responds_to?(:object_id)
else
  a.to_unsafe
end

The only implementation of MyModule is Foo which clearly implements object_id. So the to_unsafe code path should not be reachable.

Before #14728, the if statement would be replaced by a.object_id. If there was a chance that object_id is not implemented for MyModule, this should fail to compile as well.

The if condition restricts a to Foo in the then branch. In the else branch, it's MyModule, even though all implementing types are already covered in the then branch, so it should actually be unreachable / typed as NoReturn.

[HertzDevil]

Modules are similar to abstract classes in this aspect so I don't think they can be exhausted like that.

[straight-shoota]

Ah, right. That's unfortunate.

The change from 1dac1b0 (#14728) is really just about adding an alternative code path for types that don't implement object_id:

+if value.responds_to?(:object_id)
   "object_id: #{value.object_id}"
+else
+  value.to_unsafe
+end

But now it crashes on types that actually do implement it. 🤷

[straight-shoota]

Trying to refactor the control flow as a workaround causes an ICE:

module MyModule; end

class Foo
  include MyModule
end

a = Foo.new.as(MyModule)

if !a.responds_to?(:object_id) && a.responds_to?(:to_unsafe)
  a.to_unsafe
else
  a.object_id
end # BUG: trying to downcast (Foo | MyModule) (Crystal::MixedUnionType) <- Foo (Crystal::NonGenericClassType) (Exception)