Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow unions of same-type containers for certain container methods #10510

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

HertzDevil
Copy link
Contributor

The following fails to compile, even though it should have a well-defined result:

def foo
  true ? [1] : ['a']
end

foo + foo # Error: no overload matches 'Array(Float64)#+' with type (Array(Float64) | Array(Int32))

This happens because Array(T)#+(other : Array(U)) forall U ensures only that self is never a union of arrays, but other in general could be one; thus, following the discussion from #8973, it is correct to issue a compiler error here (although U shouldn't even be substitutable in the first place). The solution is to remove the unnecessary free var in the other's restriction, which works for most methods affected by this PR. They are:

In the example above, this makes typeof(foo + foo) always equal to Array(Int32 | Char), instead of Array(Int32 | Char) | Array(Int32) | Array(Char). However this trick doesn't work with Tuple#+ and NamedTuple#merge, because even though these types have built-in covariance, the types with different arities / names still cannot be merged. Those two methods are fixed instead by dispatching over the other argument's type. In other words:

def foo
  true ? {1} : {'a'}
end

typeof(foo + foo) # => Tuple(Char | Int32, Char | Int32)

def bar
  true ? {1} : {'a', 'a'}
end

typeof(bar + bar) # => (Tuple(Char | Int32, Char, Char | Int32)
                  #   | Tuple(Char, Char, Char, Char)
                  #   | Tuple(Int32, Int32))

Note that #10429 disallows unions inside splats, so a + b supports more types than {*a, *b} (it already does since a can already be a union of Tuples).

spec/std/array_spec.cr Outdated Show resolved Hide resolved
spec/std/array_spec.cr Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Array#| doesn't work with union types
2 participants