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

Make {Enumerable,Iterator}#flat_map work with mixed element types #10329

Merged
merged 2 commits into from
Feb 18, 2021

Conversation

HertzDevil
Copy link
Contributor

Currently, the following code fails:

[[1, 2, 3], ['a', 'b']].flat_map &.itself
# Error: expected block to return Array(U) | Iterator(U) | U, not (Array(Char) | Array(Int32))

There is no reason that the block must return Arrays and Iterators of the same element type; Array#concat and #push, which this method relies on, already work with element types that differ from self's.

The Iterator form will yield the correct elements, but it uses the wrong generic type argument, breaking certain methods like #to_a:

[[1, 2, 3], ['a', 'b']].each.flat_map(&.itself).each { |x| print x } # => 123ab

[[1, 2, 3], ['a', 'b']].each.flat_map(&.itself).to_a
# Error: no overload matches 'Array(Char)#<<' with type (Char | Int32)
#
# Overloads are:
#  - Array(T)#<<(value : T)
# Couldn't find overloads for these types:
#  - Array(Char)#<<(value : Int32)

The type restriction Array(U) | Iterator(U) | U forall U is used in the block's return type. This restriction is unneeded, since whether a value should be flattened or not depends on its dynamic type, not static type. This PR fixes both methods by replacing the restrictions with typeof expressions that deduce the common element type. Fixes #5803:

array = [1, 2, [[3]]]
result = array.flat_map do |item|
  if item.is_a?(Array)
    item
  else
    [item] of typeof(array) | typeof(array.first)
  end
end
result         # => [1, 2, [3]]
typeof(result) # => Array(Array(Array(Array(Int32)) | Int32) | Array(Array(Int32)) | Array(Int32) | Int32)

@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:collection labels Jan 29, 2021
@Sija
Copy link
Contributor

Sija commented Jan 29, 2021

[...] Fixes #5803:

@HertzDevil actually it doesn't, it just changes the code so it doesn't reproduce anymore, but the underlying issue is still there, that's different.

@bcardiff
Copy link
Member

FTR The snippet of #5803 works with this patch. I agree there are cases where the module validation could still appear, but the changes proposed here make sense either way.

@bcardiff bcardiff added this to the 1.0.0 milestone Feb 18, 2021
@asterite asterite merged commit 85da642 into crystal-lang:master Feb 18, 2021
@HertzDevil HertzDevil deleted the bug/flat-map-mixed-types branch February 19, 2021 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LLVM validation error in flat_map
5 participants