search

morganstanley / hobbes

A language and an embedded JIT compiler
http://hobbes.readthedocs.io/
Apache License 2.0
1.17k stars 105 forks source link

10:11 in a slice gets interpreted as a very large index #313

Closed adam-antonik closed 4 years ago

adam-antonik commented 4 years ago

:t [0][10:11] (quote ([0])[convert(36660000000L)::time])

This isn't what I intended to do at all, and produced the confusion below. I think this should be mentioned in the docs on slices. I'm not sure that (Convert a long) really is the right constraint for linear indexing, and that adding a seperate constraint (AsIndex a long) with instances for int and short would make more sense.

Anyway, this is what I thought was happening:

Doing a slice with a starting index >= 10 on an array of arrays of something consistently fails when the slice indices are integers.

$ ./hi hi : an interactive shell for hobbes type ':h' for help on commands

a = newArray(20L) :: [[int]] [a[i] <- [] | i <- [0..19]] a[10:11] Segmentation fault (core dumped)

But this works if we use longs

a = newArray(20L) :: [[int]] [a[i] <- [] | i <- [0..19]] a[10L:11L] [[]]

Which is confusing, as the code in farray.hob for slice specifically accepts things that can be converted to longs, and converts them.

If we define our indices as integers but pull them from an array, then this also works.

a = newArray(20L) :: [[int]] [a[i] <- [] | i <- [0..19]] b = neArray(2L) :: [int] b[0] <- 10 b[1] <- 11 a[b[0]:b[1]] [[]]

There does seem to be something a bit wonky in how numeric constants are handled by evalExpr.

adam-antonik commented 4 years ago

It turns out that 10:11 is being interpreted as a time, and being converted to 36660000000L, which naturally is a sad number to index into such an array with. This really should be pointed out in the docs on slicing.

kthielen commented 4 years ago

Oh yes, I hit this myself a while back and was also similarly confused/annoyed. This should be a small lexer change, I will take a look.

kthielen commented 4 years ago

Thanks for pointing this out, I knew about the lexical ambiguity but left it longer than I should have. I have a fix here, which also excludes indexing in arrays by any type other than the usual integral type (not just anything convertible to long): https://github.com/Morgan-Stanley/hobbes/pull/314

I'll close this issue once the PR goes through.

HTH