Virtual types causing unexpected behaviour

I’m currently writing a library, and have run into some issues with virtual
types. Additionally, there seem to be inconsistencies between #as and
#unsafe_as that I want to understand better. If anyone could help me out, I’d really appreciate it!

This code does not compile:

# I have control only over the code in MyLibrary.
module MyLibrary::Readable
  abstract def get_chunk : Readable
end

class MyLibrary::ReadableIterator
  include Iterator(Readable)

  def initialize(@src : Readable)
  end

  def next
    return stop if Random.rand < 0.1
    @src.get_chunk
  end
end


# This is some hypothetical code a user might write using the library.
class FooBase
end

class ReadonlyFoo < FooBase
  include MyLibrary::Readable

  def get_chunk : self
    ReadonlyFoo.new
  end
end

class ReadWriteFoo < FooBase
  include MyLibrary::Readable

  def get_chunk : self
    ReadWriteFoo.new
  end
end

readonly = ReadonlyFoo.new
rw = ReadWriteFoo.new
MyLibrary::ReadableIterator.new(readonly).to_a

To restate the above - my library provides an interface called Readable,
and an Iterator over Readables.

The user creates a base class, FooBase (which contains boilerplate Foo code),
then creates different subclasses with different read/write abilities. (You might
wonder why the user does not just include Readable from FooBase. In the real
version of this code, there is a WriteonlyFoo that must inherit from FooBase
but should not include Readable.)

Here’s the compilation error:

 44 | puts MyLibrary::ReadableIterator.new(readonly).to_a
                                                     ^---
Error: instantiating 'MyLibrary::ReadableIterator#to_a()'


In /usr/lib/crystal/enumerable.cr:1712:5

 1712 | each { |e| ary << e }
        ^---
Error: instantiating 'each()'


In /usr/lib/crystal/iterator.cr:597:7

 597 | yield value
       ^
Error: argument #1 of yield expected to be MyLibrary::Readable, not FooBase+

I think that the compiler is turning both ReadonlyFoo and ReadWriteFoo
into the virtual type FooBase+, which is causing the return type of
MyLibrary::ReadableIterator#next to be FooBase+. Because FooBase is not
Readable in general, the compiler refuses to continue from this point.

Now, onto my first question - is this (and should it be) expected behaviour? The user’s
code seems perfectly sound to me, and it’s only due to the virtual type system
that it’s not compiling.

My second question concerns the fix I’m currently using:

# inside MyLibrary::ReadableIterator:
def next
  return stop if Random.rand < 0.1
  @src.get_chunk.unsafe_as(Readable)
end

This works, but is it a sound thing to do? I’m not familiar with how crystal handles
modules-as-types, so I’m not confident in this solution.

Finally, my third question - how come only #unsafe_as fixes this problem?
If #as is used instead, the FooBase+ error persists.

The main issue is that @src.get_chunk resolves to all the concrete definitions of get_chunk, and there are two: one returns ReadonlyFoo and the other returns ReadWriteFoo (the abstract method definition signature is usually ignored in the compiler except when checking that it’s implemented, but it’s not used to cast types down.) Then the result of combining those two types is "FooBase or any of its subtypes", which isn’t Readable.

It seems that if you do as(Readable) it doesn’t work but it should, so I would report that as a bug.

Using unsafe_as works because it completely bypasses the type-system, but I wouldn’t advise doing that.

Is there a reason FooBase can’t be abstract and Readable? That way it works.

In any case, I would report the bug above.

Thank you!

2 Likes

Ah, I see. Thanks for helping me understand where in the process the types were actually getting agglomerated. I’ll go ahead and open up a bug with the as / unsafe_as disparity.

FooBase can be abstract, but it cannot be Readable. In the actual implementation, there is a WriteonlyFoo which requires the functionality of FooBase but should not be Readable.

As a quick follow up - I’m well aware that using unsafe_as in the way detailed above is a poor design choice. However, I want to safeguard against this bug in the future by leaving the unsafe_as where it is, at least until as is fixed.

Ignoring code quality, is unsafe_as correct to use here? Readable::get_chunk and ReadableIterator#@src have the type annotation Readable, so I’d imagine it is.

Thanks again :)

unsafe_as just reinterprets bytes. If you have a Readable struct I’m sure that will result in a segfault or undefined behavior. It’s almost pure chance that unsafe_as works here.

That was my main concern - I suppose I’ll have to pursue a different solution, then! Thanks a ton for your help :)

In the issue you created HertzDevil proposed a solution: use as(Readable) when returning from each method.