Crystal: Module validation failed: Function return type does not match operand type of return inst

[ljuti]

Found this compiler bug when duplicating and modifying test data hashes in my specs.

Code to reproduce

x = {
  "y" => [] of Hash(String, String | Hash(String, String))
}
x["y"].dup.concat [{ "a" => "b" }]

Compiler output

Module validation failed: Function return type does not match operand type of return inst!
  ret i32* %1, !dbg !36
 %"Hash(String, Hash(String, String) | String)"* (Exception)
  from Crystal::CodeGenVisitor#finish:Nil
  from Crystal::Compiler#codegen<Crystal::Program, Crystal::ASTNode+, Array(Crystal::Compiler::Source), String>:(Tuple(Array(Crystal::Compiler::CompilationUnit), Array(String)) | Nil)
  from Crystal::Compiler#compile<Array(Crystal::Compiler::Source), String>:Crystal::Compiler::Result
  from Crystal::Command#run_command<Bool>:Nil
  from Crystal::Command#run:(Bool | Crystal::Compiler::Result | Nil)
  from main
Error: you've found a bug in the Crystal compiler. Please open an issue, including source code that will allow us to reproduce the bug: https://github.com/crystal-lang/crystal/issues

Environment

Crystal 0.26.1 (2018-09-26)

LLVM: 6.0.1
Default target: x86_64-apple-macosx

References

Might be related to #6253

[codenoid]

same here :

Module validation failed: Function return type does not match operand type of return inst!          
  ret i32* %1, !dbg !24                                                                             
 %"Hash(String, Int32 | String)"* (Exception)                                                       
  from ???                                                                                          
  from ???                                                                                          
  from ???                                                                                          
  from ???                                                                                          
  from ???                                                                                          
  from ???                                                                                          
  from ???                                                                                          
  from ???                                                                                          
  from ???                                                                                          

[RX14]

This is the real error, visible when the compiler is downgraded to 0.23.1: https://carc.in/#/r/5mhm

I'll bisect.

[RX14]

I traced the problem down to https://github.com/crystal-lang/crystal/pull/5025

It's fairly easy to revert, which does fix the problem and I posted a revert here: https://github.com/RX14/crystal/commit/afad608ebe5f41e535c3898dc4a015a6bca233b0. If someone wants to figure out the root cause they can.

[Sija]

@RX14 thanks for bisecting.

It seems that calling #dup/#clone somehow breaks things, only for empty arrays though... any idea why's that? /cc @asterite

[asterite]

The code shouldn't compile at all, the types are not compatible.

The code is trying to insert an Hash(String, String) into something that holds Hash(String, String | Hash(String, String)).

[asterite]

This is the code that shouldn't compile:

class Gen(T)
end

p = Pointer(Gen(String | Int32)).malloc(1)
p.value = Gen(String).new

Because it shouldn't compile, but it does, the end result probably is something that the codegen doesn't expect. I'm not sure.

This has to do with covariance/contravariance/invariance and I won't have time to look at it.

[asterite]

This is somehow completely broken... I don't know what happened:

a = [] of Array(String | Int32)
puts typeof(a) # => Array(Array(Int32 | String))
a << [1] # ???
z = a[0]
puts typeof(a) # => Array(Array(Int32 | String))
puts typeof(z) # => (Array(Int32 | String) | Array(Int32)) # Huh???

[asterite]

I can make it give a compile error by hardcoding something in the Pointer logic in the compiler, though the error will be ugly (but it's better than a compile error)

As a first step, I think we need a way to tell whether a type parameter in restriction should be an exact match or an upper bound match. For example:

def strict(x : Array(Int32 | String))
end

def nonstrict(x : Array(<(Int32 | String))
end

strict(Array(Int32).new) # Error
strict(Array(String).new) # Error
strict(Array(Int32 | String).new) # OK

nonstrict(Array(Int32).new) # OK
nonstrict(Array(String).new) # OK
nonstrict(Array(Int32 | String).new) # OK

Then a restriction like def push(value : T) inside Array would mean it's a strict match, and if it's an Array(Array(Int32 | String)) then you can't pass an Array(Int32).

We have stuff like x : Range(Int, Int) that means "a range of any integer type", but that would now stop to work and you'll have to do Range(<Int, <Int).

I say let's make type parameter restrictions be a strict match, and later (much later) we can implement the < operator in type restrictions, if we really need to (you can always omit them and rely on duck typing).

[RX14]

@asterite I'd prefer def nonstrict(x : Array(T)) forall T < Int32 | String, which makes it much clearer that it's generating multiple semantically different methods.

This is a massive change to the language though. I don't know how we missed this :(

[straight-shoota]

Shouldn't this discussion better continue in #3803?

[vladfaust]

I've been implementing event driven architecture and got that bug recently (code reduced) (https://carc.in/#/r/5whz):

abstract struct Event
end

class MyChannel
  def subscribe(event : T.class, &proc : Proc(T, Nil)) forall T
    ary = Array(ProcSubscription(Event)).new
    sub = ProcSubscription(T).new(proc)

    unless ary.includes?(sub)
      ary << sub
    end
  end

  struct ProcSubscription(T)
    def initialize(@proc : Proc(T, Nil))
    end

    def call(event)
      @proc.call(event.as(T))
    end
  end
end

struct FooEvent < Event
  getter foo

  def initialize(@foo : String)
  end
end

channel = MyChannel.new

channel.subscribe(FooEvent) do |event|
  puts "Got FooEvent: #{event.foo}"
end

Module validation failed: Function return type does not match operand type of return inst!
  ret %"(MyChannel::ProcSubscription(Event+) | MyChannel::ProcSubscription(FooEvent))" %1, !dbg !11
 %"MyChannel::ProcSubscription(Event+)" = type { %"->" } (Exception)

I've narrowed the bug to these lines:

    unless ary.includes?(sub)
      ary << sub
    end

Once unless wrapper is removed, it compiles nicely (https://carc.in/#/r/5wi0):

-    unless ary.includes?(sub)
      ary << sub
-    end

Basically, any trial to return bool fails with the same bug:

if ary.includes?(sub)
  return false
end
ary << sub
return true

[decademoon]

Also found this bug, probably related:

lib LibFoo
  struct S
    callback : ->
  end
end

s = LibFoo::S.new
s.callback = nil

Module validation failed: Call parameter type does not match function signature!
%Nil ()* null
 %"->" = type { i8*, i8* }  %71 = call i8* @"~check_proc_is_not_closure"(%Nil ()* null), !dbg !74
 (Exception)
  from Crystal::CodeGenVisitor#finish:Nil
  from Crystal::Compiler#codegen<Crystal::Program, Crystal::ASTNode+, Array(Crystal::Compiler::Source), String>:(Tuple(Array(Crystal::Compiler::CompilationUnit), Array(String)) | Nil)
  from Crystal::Compiler#compile<Array(Crystal::Compiler::Source), String>:Crystal::Compiler::Result
  from Crystal::Command#run_command<Bool>:Nil
  from Crystal::Command#run:(Bool | Crystal::Compiler::Result | Nil)
  from __crystal_main
  from main

[RX14]

@decademoon luckily that's not related! please make another issue :)

[decademoon]

@RX14 #7279

[straight-shoota]

I'm also hitting this compiler error, but it does not reproduce reliably.

Module validation failed: Function return type does not match operand type of return inst!
  ret %"(Array(AnyHash::JSONTypes::Value)+ | Bool | Float32 | Float64 | Hash(String | Symbol, AnyHash::JSONTypes::Value)+ | Int128 | Int16 | Int32 | Int64 | Int8 | JSON::Any | Set(AnyHash::JSONTypes::Value) | String | Symbol | Time | UInt128 | UInt16 | UInt32 | UInt64 | UInt8 | Nil)" %1, !dbg !21
 %"(Array(AnyHash::JSONTypes::Value) | Bool | Float32 | Float64 | Hash(String | Symbol, AnyHash::JSONTypes::Value) | Int128 | Int16 | Int32 | Int64 | Int8 | JSON::Any | Set(AnyHash::JSONTypes::Value) | String | Symbol | Time | UInt128 | UInt16 | UInt32 | UInt64 | UInt8 | Nil)" = type { i32, [3 x i64] } (Exception)

It happened on CI: https://circleci.com/gh/shardbox/shardbox-core/22
In the next build https://circleci.com/gh/shardbox/shardbox-core/24 it did not occur, but there was no change to the Crystal code in between.

[Sija]

@straight-shoota there's also #6253

[straight-shoota]

Thanks @Sija I didn't notice this other issue.