search

crystal-lang / crystal

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

Error: type must be xxx not yyy #8812

Open Blacksmoke16 opened 4 years ago

Blacksmoke16 commented 4 years ago

https://play.crystal-lang.org/#/r/8kjf

require "http/request"

abstract struct Param; end

abstract struct Parameter(T) < Param
  getter name : String
  getter default : T?
  getter type : T.class

  def initialize(@name : String, @default : T? = nil, @type : T.class = T); end

  abstract def extract(request : HTTP::Request) : String?

  def nilable? : Bool
    @type.nilable?
  end

  def required? : Bool
    !nilable?
  end
end

struct MockParameter(T) < Parameter(T)
  def initialize(name : String, @value : String? = nil, default : T? = nil, type : T.class = T)
    super name, default, type
  end

  def extract(request : HTTP::Request) : String?
    @value
  end
end

struct ArgumentResolver
  def resolve(request : HTTP::Request, route : Action)
    route.parameters.map do |param|
      value = param.extract request

      if param.required? && value.nil?
        param.default.try do |default_value|
          next default_value
        end

        raise "Missing required parameter"
      else
        next param.default if value.nil?
      end

      value
    end
  end
end

class Action
  getter parameters : Array(Param)

  def initialize(@parameters : Array(Param) = [] of Param)
  end
end

request = HTTP::Request.new "GET", "FOO"

route = Action.new [MockParameter(Int32?).new("id")] of Param
pp ArgumentResolver.new.resolve(request, route)

route = Action.new [MockParameter(Bool).new("id", "true")] of Param
pp ArgumentResolver.new.resolve(request, route)
In test.cr:65:20

 65 | route = Action.new [MockParameter(Bool).new("id", "true")] of Param
                         ^----
Error: instantiating 'Array(Param).class#build(Int32)'

In test.cr:65:20

 65 | route = Action.new [MockParameter(Bool).new("id", "true")] of Param
                         ^----
Error: instantiating 'Array(Param).class#build(Int32)'

In test.cr:65:21

 65 | route = Action.new [MockParameter(Bool).new("id", "true")] of Param
                          ^
Error: type must be (Int32 | String | Nil), not (Bool | Int32 | String | Nil)

In test.cr:35:26

 35 | route.parameters.map do |param|
                           ^
Error: type must be (Int32 | String | Nil), not (Bool | Int32 | String | Nil)

Related, If I change Param and Parameter(T) types to modules, it complies but then the Int32? type is used twice within the map.

https://play.crystal-lang.org/#/r/8kjg

@asterite mentioned in Gitter

compiler bug related to caching, probably generics I think it's a bug related to subclasses of modules or generic types affecting methods that were already typed

Blacksmoke16 commented 4 years ago

https://github.com/athena-framework/athena/runs/476846691?check_suite_focus=true

Causing CI to fail on GH actions, however I can't seem to reproduce locally :/

Blacksmoke16 commented 4 years ago

Reduced:

https://play.crystal-lang.org/#/r/8nf1

abstract struct Param; end

struct Parameter(T) < Param
  getter default : T?

  def initialize(@default : T? = nil); end
end

parameters = [Parameter(Int32?).new] of Param

parameters.map do |param|
  1 || param.default
end

parameters = [Parameter(Bool).new(default: true)] of Param
crystal test.cr --error-trace
In test.cr:15:14

 15 | parameters = [Parameter(Bool).new(default: true)] of Param
                   ^----
Error: instantiating 'Array(Param).class#build(Int32)'

In test.cr:15:14

 15 | parameters = [Parameter(Bool).new(default: true)] of Param
                   ^----
Error: instantiating 'Array(Param).class#build(Int32)'

In test.cr:15:15

 15 | parameters = [Parameter(Bool).new(default: true)] of Param
                    ^
Error: type must be (Int32 | Nil), not (Bool | Int32 | Nil)

In test.cr:11:16

 11 | parameters.map do |param|
                     ^
Error: type must be (Int32 | Nil), not (Bool | Int32 | Nil)

EDIT: Here's the callstack

["src/compiler/crystal/semantic/bindings.cr:70:7 in 'raise_frozen_type'",
 "src/compiler/crystal/semantic/bindings.cr:40:9 in 'set_type'",
 "src/compiler/crystal/semantic/bindings.cr:50:7 in 'set_type_from'",
 "src/compiler/crystal/semantic/bindings.cr:199:9 in 'update'",
 "src/compiler/crystal/semantic/bindings.cr:184:31 in 'notify_observers'",
 "src/compiler/crystal/semantic/bindings.cr:212:9 in 'propagate'",
 "src/compiler/crystal/semantic/bindings.cr:186:31 in 'notify_observers'",
 "src/compiler/crystal/semantic/bindings.cr:212:9 in 'propagate'",
 "src/compiler/crystal/semantic/bindings.cr:186:31 in 'notify_observers'",
 "src/compiler/crystal/semantic/bindings.cr:212:9 in 'propagate'",
 "src/compiler/crystal/semantic/bindings.cr:143:7 in 'bind_to'",
 "src/compiler/crystal/semantic/call.cr:100:5 in 'recalculate'",
 "src/compiler/crystal/semantic/call.cr:691:5 in 'on_new_subclass'",
 "src/compiler/crystal/types.cr:1015:44 in 'notify_subclass_added'",
 "src/compiler/crystal/types.cr:1202:7 in 'add_subclass'",
 "src/compiler/crystal/types.cr:2002:7 in 'after_initialize'",
 "src/compiler/crystal/types.cr:1545:7 in 'instantiate'",
 "src/compiler/crystal/semantic/bindings.cr:549:26 in 'update'",
 "src/compiler/crystal/semantic/bindings.cr:465:5 in 'update'",
 "src/compiler/crystal/semantic/main_visitor.cr:295:7 in 'visit'",
 "src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept'",
 "src/compiler/crystal/semantic/main_visitor.cr:1299:11 in 'visit'",
 "src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept'",
 "src/compiler/crystal/semantic/main_visitor.cr:1308:21 in 'visit'",
 "src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept'",
 "src/compiler/crystal/syntax/ast.cr:169:27 in 'accept_children'",
 "src/compiler/crystal/syntax/visitor.cr:28:11 in 'accept'",
 "src/compiler/crystal/semantic/main_visitor.cr:1121:7 in 'visit'",
 "src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept'",
 "src/compiler/crystal/semantic/main_visitor.cr:1042:13 in 'visit'",
 "src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept'",
 "src/compiler/crystal/syntax/ast.cr:169:27 in 'accept_children'",
 "src/compiler/crystal/syntax/visitor.cr:28:11 in 'accept'",
 "src/compiler/crystal/semantic/main_visitor.cr:1299:11 in 'visit'",
 "src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept'",
 "src/compiler/crystal/semantic/main_visitor.cr:1308:21 in 'visit'",
 "src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept'",
 "src/compiler/crystal/syntax/ast.cr:169:27 in 'accept_children'",
 "src/compiler/crystal/syntax/visitor.cr:28:11 in 'accept'",
 "src/compiler/crystal/semantic/call.cr:422:13 in 'instantiate'",
 "src/compiler/crystal/semantic/call.cr:311:5 in 'lookup_matches_in_type'",
 "src/compiler/crystal/semantic/call.cr:260:3 in 'lookup_matches_in_type:search_in_parents:with_literals'",
 "src/compiler/crystal/semantic/call.cr:239:5 in 'lookup_matches_in'",
 "src/compiler/crystal/semantic/call.cr:238:3 in 'lookup_matches_in:with_literals'",
 "src/compiler/crystal/semantic/call.cr:195:7 in 'lookup_matches_without_splat'",
 "src/compiler/crystal/semantic/call.cr:130:7 in 'lookup_matches:with_literals'",
 "src/compiler/crystal/semantic/call.cr:119:5 in 'lookup_matches'",
 "src/compiler/crystal/semantic/call.cr:91:5 in 'recalculate'",
 "src/compiler/crystal/semantic/main_visitor.cr:1349:7 in 'visit'",
 "src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept'",
 "src/compiler/crystal/semantic/main_visitor.cr:3043:7 in 'expand'",
 "src/compiler/crystal/semantic/main_visitor.cr:2966:9 in 'visit'",
 "src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept'",
 "src/compiler/crystal/semantic/main_visitor.cr:752:7 in 'type_assign'",
 "src/compiler/crystal/semantic/main_visitor.cr:751:5 in 'type_assign'",
 "src/compiler/crystal/semantic/main_visitor.cr:740:7 in 'visit'",
 "src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept'",
 "src/compiler/crystal/syntax/ast.cr:169:27 in 'accept_children'",
 "src/compiler/crystal/syntax/visitor.cr:28:11 in 'accept'",
 "src/compiler/crystal/semantic/main_visitor.cr:6:7 in 'visit_main'",
 "src/compiler/crystal/semantic/main_visitor.cr:5:5 in 'visit_main:process_finished_hooks:cleanup'",
 "src/compiler/crystal/semantic.cr:22:7 in 'semantic'",
 "src/compiler/crystal/compiler.cr:169:7 in 'compile'",
 "src/compiler/crystal/command.cr:275:3 in 'compile'",
 "src/compiler/crystal/command.cr:191:14 in 'run_command'",
 "src/compiler/crystal/command.cr:106:7 in 'run'",
 "src/compiler/crystal/command.cr:49:5 in 'run'",
 "src/compiler/crystal/command.cr:48:3 in 'run'",
 "src/compiler/crystal.cr:8:1 in '__crystal_main'",
 "src/crystal/main.cr:106:5 in 'main_user_code'",
 "src/crystal/main.cr:92:7 in 'main'",
 "src/crystal/main.cr:115:3 in 'main'",
 "__libc_start_main",
 "_start",
 "???"]

Seems to be related to the Block? Added some pp before raise_frozen_type

pp freeze_type, typeof(freeze_type), type, typeof(type), self, typeof(self)

(Int32 | Nil)
Crystal::Type
(Bool | Int32 | Nil)
Crystal::Type
do |param|
  1 || param.default
end
Crystal::ASTNode

Anything obvious come to mind @asterite?

Blacksmoke16 commented 4 years ago

@asterite One thing I noticed is this only happens with #map. #compact_map, #each, etc work fine.

abstract struct Param; end

struct Parameter(T) < Param
  getter default : T

  def initialize(@default : T); end
end

parameters = [Parameter(Int32).new(1)] of Param

pp parameters.map &.default

parameters = [Parameter(Bool).new(default: true)] of Param

This seems to indicate this is some issue with free variables given the #map is defined as 

def map(&block : T -> U) forall U
  Array(U).new(size) { |i| yield @buffer[i] }
end

While #compact_map uses the type of the first element in the array to define the type of the array; ary = [] of typeof((yield first).not_nil!).

SO, a workaround for this issue would be to define a custom iterator method to use instead of #map.

Something like:

class Array
  def map_first
    ary = [] of typeof((yield first).not_nil!)
    each do |e|
      ary << yield e
    end
    ary
  end
end

abstract struct Param; end

struct Parameter(T) < Param
  getter default : T

  def initialize(@default : T); end
end

parameters = [Parameter(Int32).new(1)] of Param

pp parameters.map_first &.default # => [1]

parameters = [Parameter(Bool).new(default: true)] of Param

pp parameters.map_first &.default # => [true]
asterite commented 4 years ago

@Blacksmoke16 Yes, that's exactly the issue.

The problem is that U is computed once and is not recomputed once T changes, and that's why you get the error. There are some bindings between nodes missing, but I have to figure out where they go and why when I tried to fix it it didn't work.

Blacksmoke16 commented 3 years ago

Reduced more:

module SomeInterface
  abstract def foo(& : _ -> Nil) : Nil
end

class Example
  include SomeInterface

  def foo(& : -> Nil) : Nil
    yield 1 || "foo"
  end
end

Example.new.foo do |v|
  pp v
end
In test.cr:13:17

 13 | Example.new.foo do |v|
                      ^
Error: type must be Nil, not (Int32 | String)
Blacksmoke16 commented 2 years ago

Looks like https://github.com/crystal-lang/crystal/issues/8812#issuecomment-731864946 context was fixed at some point as it works fine as of 1.1.1. The other two reproductions do not however.