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

`create` does not work for UncheckedArray, as sizeof(UncheckedArray[T])==0 #19000

Open shirleyquirk opened 3 years ago

shirleyquirk commented 3 years ago

Example

proc mutate(x:var float) = x = 3.9

proc a(degree:int) =
  echo "a: ",degree
  var x = create(UncheckedArray[float],degree)      #second call returns the same pointer,
  #var x = cast[ptr UncheckedArray[float]](alloc0(sizeof(float)*degree)) #works
  echo x.repr

  doAssert(x[0]==0.0)
  mutate(x[0])
  x.dealloc()

proc main() =
  a(3)
  a(7)
  a(3)

main()

Current Output

a: 3
ptr 0x7fe7c52e5048 --> [...]

a: 7
ptr 0x7fe7c52e5048 --> [...]

/usercode/in.nim(21)     in
/usercode/in.nim(18)     main
/usercode/in.nim(12)     a
/playground/nim/lib/system/assertions.nim(29) failedAssertImpl
/playground/nim/lib/system/assertions.nim(22) raiseAssert
/playground/nim/lib/system/fatal.nim(49) sysFatal
Error: unhandled exception: /usercode/in.nim(12, 11) `x[0] == 0.0` 3.9 [AssertionError]

Expected Output

a: 3
ptr 0x7fad8198f070 --> [...]

a: 7
ptr 0x7fad81990120 --> [...]

a: 3
ptr 0x7fad8198f160 --> [...]

with --gc:arc the above work

Nim Compiler Version 1.5.1 [Linux: amd64]
Compiled at 2021-06-20
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: ad5063aed101f0102fb867f0b4e518ede7fa188b
active boot switches: -d:release
shirleyquirk commented 3 years ago

silly me. sizeof(UncheckedArray[T]) == 0

shirleyquirk commented 3 years ago

Solution:

proc create[T](x:typedesc[UncheckedArray[T]],size:int):ptr UncheckedArray[T]{.inline,benign,raises:[].} = 
  cast[ptr UncheckedArray[T]](alloc0(sizeof(T)*size))

or make sizeof(UncheckedArray[T]) == sizeof(T) ??? don't know what that would break

demotomohiro commented 3 years ago

Documentation comment of create says:

Allocates a new memory block with at least T.sizeof * size bytes.

So I think when create is called with T such that sizeof(T) == 0, it should be compile time error. Because T.sizeof * size is always 0 and create cannot allocate a new memory block that you actually needs.

Making sizeof(UncheckedArray[T]) != 0 can be a breaking change. For example, payloadCheck() proc in tests/misc/tsizeof.nim become failed.

From https://nim-lang.org/docs/manual.html#types-unchecked-arrays

Additionally, an unchecked array is translated into a C array of undetermined size:

In C, C array of undetermined size in a struct is count as 0 bytes.

#include <stdio.h>
#include <stdlib.h>

typedef struct {
  int len;
  int cap;
  int data[];
} MySeq;

int main(void)
{
    printf("sizeof(int): %ld, sizeof(MySeq):%ld\n", sizeof(int), sizeof(MySeq));
    return EXIT_SUCCESS;
}

Output:

sizeof(int): 4, sizeof(MySeq): 8

I think you should create a new proc like proc createUncheckedArray(T: typedesc; size = 1.Positive): ptr UncheckedArray[T] if you really need such a proc.

shirleyquirk commented 3 years ago

Aha, put like that it's very clear that sizeof(UncheckedArray[T]) should be 0

Having just shot myself in the foot, I'm pro a static assert that create isn't returning a footgun, but I guess that would be a breaking change too. Maybe a warning?

If a proc returning a ptr UncheckedArray were to exist in the stdlib, my bike shedding stance is that an overload on create could never be ambiguous, and would be intuitive.

metagn commented 5 days ago

cast[ptr UncheckedArray[float]](create(float, degree)) should be another workaround, maybe even the intended way? Maybe create with a size parameter should return ptr UncheckedArray[T] instead?

juancarlospaco commented 5 days ago

Should have at least 1 test.