nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.59k stars 1.47k forks source link

openarray usages should be more restricted #9592

Open krux02 opened 6 years ago

krux02 commented 6 years ago

The compiler doesn't complain about openarray[openarray[...]] at all, but when something is actually done with it, it is almost certain that it will fail.

openarray as a generic constraint should also be forbidden.

Example

These examples should show misuses of openarray that should just be caught as sane compile errors to prevent users to dig further into something that doesn't work.

# this compile so far, but as soon as anything is done with it, it breaks compilation
func foo[T](x: openarray[openarray[T]]): T =
  x[0][0]

# openArray is used as a constraint for a template
proc foo1[T:openArray](a:T):auto = low(a)
proc foo2[T](a:openArray[T]):auto = low(a)

proc test()=
  var a: array[3..6, int]
  # but here the generic parameter is an `array`, not an `openarray`
  echo (foo1(a), foo2(a)) # prints (Field0: 3, Field1: 0)

test()

If nim refuses proc foo1[T:openArray](a:T) early, the surprise from above can be prevented.

Expected Output

Error: openarray of openarray is not allowed

Additional Information

This is the unification of these issues: https://github.com/nim-lang/Nim/issues/9041 https://github.com/nim-lang/Nim/issues/9042 https://github.com/nim-lang/Nim/issues/9032

timotheecour commented 6 years ago

the title doesn't reflect some of the linked issues

mratsim commented 6 years ago

openarray of openarrays or an alternative using Iterable/Indexable concepts is needed for scientific computing.

krux02 commented 6 years ago

@mratsim the problem is, openarray is seen as an abstract concept that magically works with everything. But it's not. It is just a pointer and a length pair and an automatic conversion to it from seq and array. This conversion is cheap and doesn't cause problems, it just doesn't work on nested structures. Here a recursive conversion would be necessary and that isn't cheap anymore because it can't be done without allocation anymore. Meaning the compiler won't create it for you.

And no openarray of openarray is not needed for scientific computing. In C and Fortran (BLAS) code there is the pattern to add a stride parameter to the functions so that arbitrary rectangles from matrices can be passed down to functions, not just one dimensional slices. It would allow to implement the functional patters such as fold or just a sliding window in multiple dimensions, just think of an edge detection filter for images. But I am afraid to say that is just not something the core language provides right now. Multidimensional slicing is something that I would want to have in Nim, but right now the standard library can't provide it and there is a feature stop until version 1.0 is hit. Maybe you found a good solution for it in your arraymancer project, but I really didn't investigate it very deeply, so I don't know if you already solved it.

timotheecour commented 6 years ago

agreed on fact that openarray[openarray] cannot make sense as it wouldn't be a O(1) conversion

... but right now the standard library can't provide it ...

i actually implemented a N-dimensional tensor library that supports multidimensional slicing and projection in nim, that works similarly as D's mir ndslice (where dimensions are known at CT); so I know it can be done :) ; not sure that kind of stuff belongs in stdlib though (a good test IMO for something belonging in stdlib is whether compiler needs it)

there is a feature stop until version 1.0 is hit

where was this mentioned?

Araq commented 6 years ago

there is a feature stop until version 1.0 is hit

where was this mentioned?

There isn't one, but we have a v1 plan and we stick to it.

timotheecour commented 6 years ago

by "There isn't one" I'm assuming you're meaning "there isn't a feature stop" ; ok (good...)

mratsim commented 6 years ago

If conversion is not O(1), I agree that would misrepresent the type.

Within Arraymancer or any tensor library, it's not a problem, strides is part of the tensor object. The main use case of openarray of openarray for me is for library API:

[[1, 2, 3], [4, 5, 6]].toTensor(), @[[1, 2, 3], [4, 5, 6]].toTensor(), [@[1, 2, 3], @[4, 5, 6]].toTensor() and @[@[1, 2, 3], @[4, 5, 6]].toTensor()

My solution is this one:

iterator flatIter*(s: string): string {.noSideEffect.} =
  yield s

iterator flatIter*[T: not char](s: openarray[T]): auto {.noSideEffect.}=
  ## Inline iterator on any-depth seq or array
  ## Returns values in order
  for item in s:
    when item is array|seq:
      for subitem in flatIter(item):
        yield subitem
    else:
      yield item

but this was rejected as a stdlib feature due to the auto return type: https://github.com/nim-lang/Nim/pull/6807.

timotheecour commented 6 years ago

see my reply there https://github.com/nim-lang/Nim/pull/6807#issuecomment-435568953 ; but ya, openArray isn't the answer for that, there are other ways, eg joiner (see comment there)

Araq commented 6 years ago

@mratsim your toTensor should be a macro.