Fixed bug for mapping over empty array

[TheSeamau5]

Mapping a function over an empty array used to somehow create an array
of length 1. Now, mapping a function over an empty array returns an
empty array

[evancz]

Make sure the style of the change matches the surrounding code. More about that here.

I think this case deserves a test case as well so we don't have regressions.

Overall, I see that this fixes the bug, but I don't feel like we actually understood what's going on here. Perhaps @Xashili can give some guidance?

@TheSeamau5, can you share the minimal example that demonstrates the bug in this thread?

[evancz]

Make sure the style of the change matches the surrounding code. More about that here.

I think this case deserves a test case as well so we don't have regressions.

Overall, I see that this fixes the bug, but I don't feel like we actually understood what's going on here. Perhaps @Xashili can give some guidance?

@TheSeamau5, can you share the minimal example that demonstrates the bug in this thread?

[TheSeamau5]

This is the minimum failing test (I've added some printing so you can see it)

import Text (asText)
import Array (..)
import List

main =
  asText
    (List.length
      (toList
        (map (\x -> x * x) empty)))

The result is this:

Whereas, what the code does is just mapping a function over an empty array (which should return an empty array), then convert the array to a list (which should be the empty list), then get its length (which should be 0), then display it. Instead, we get 1.

To convince yourself that there's something lurking in that array, if you do this:

import Text (asText)
import Array (..)

main =
  asText
    (toList
      (map (\x -> x * x) empty))

you get this:

But, this is not toList's fault because:

import Text (asText)
import Array (..)

main =
  asText
    (toList empty)

yields:

It's map's fault. Turns out, it forgot to check for the empty array. So I added that check.

[TheSeamau5]

This is the minimum failing test (I've added some printing so you can see it)

import Text (asText)
import Array (..)
import List

main =
  asText
    (List.length
      (toList
        (map (\x -> x * x) empty)))

The result is this:

Whereas, what the code does is just mapping a function over an empty array (which should return an empty array), then convert the array to a list (which should be the empty list), then get its length (which should be 0), then display it. Instead, we get 1.

To convince yourself that there's something lurking in that array, if you do this:

import Text (asText)
import Array (..)

main =
  asText
    (toList
      (map (\x -> x * x) empty))

you get this:

But, this is not toList's fault because:

import Text (asText)
import Array (..)

main =
  asText
    (toList empty)

yields:

It's map's fault. Turns out, it forgot to check for the empty array. So I added that check.

[TheSeamau5]

I have no idea how Array works exactly but I figured that height == 0 is the strongest guarantee that you have an empty array, i think.

[TheSeamau5]

I have no idea how Array works exactly but I figured that height == 0 is the strongest guarantee that you have an empty array, i think.

[xash]

height is 0 if and only if the array is a leaf, i. e. a tree, that has actual items in its .table-array. If height is greater then 0, then the array is a branch, i. e. a tree, that has other trees in its .table-array. So your method would break every map over a leaf. :-)

I'm not able to make a commit right now (or the next days), but I guess the error lays around line 327 in the map function, when creating the new .table-array: instead of table: new Array(a.table) it should be table: new Array(a.table.length), as the former gives an array as an argument to the array constructor. This creates an array with the size of 1.

Same bug and fix should be in the indexedMap function. Thanks for the catch!

[xash]

height is 0 if and only if the array is a leaf, i. e. a tree, that has actual items in its .table-array. If height is greater then 0, then the array is a branch, i. e. a tree, that has other trees in its .table-array. So your method would break every map over a leaf. :-)

I'm not able to make a commit right now (or the next days), but I guess the error lays around line 327 in the map function, when creating the new .table-array: instead of table: new Array(a.table) it should be table: new Array(a.table.length), as the former gives an array as an argument to the array constructor. This creates an array with the size of 1.

Same bug and fix should be in the indexedMap function. Thanks for the catch!

[TheSeamau5]

Oops. Sorry about that.

Yeah, just push the fix whenever you can. In the meantime I'll try setting up tests with Array. I use it a lot so I'd like to make sure everything works well. I'll let you know if I find something else that's funny.

[TheSeamau5]

Oops. Sorry about that.

Yeah, just push the fix whenever you can. In the meantime I'll try setting up tests with Array. I use it a lot so I'd like to make sure everything works well. I'll let you know if I find something else that's funny.

[evancz]

Fix suggested by @Xashili is done with #171 by @brown-dragon. Can someone add the relevant tests?

[evancz]

Fix suggested by @Xashili is done with #171 by @brown-dragon. Can someone add the relevant tests?