Disallow new_<T[n]> and new_array<T[n]>?

Issue #174 resolved
Dan Bonachea created an issue

What do we think about the following contrived, questionable code:

  upcxx::global_ptr<int[5]> gp = upcxx::new_<int[5]>();
  upcxx::delete_(gp);

I think it's currently unclear whether this is permitted by our spec, although it definitely does not match our intention.

FWIW the implementation currently allows the first line above, but the second line fails to compile.

The only spec-ese that seems relevant is the new_ precondition:

Precondition: T(args...) must be a valid call to a constructor for T.

I'm not sure what "valid call to a constructor" means for an array type, although g++ doesn't seem to mind when our implementation invokes placement new with that expression on the array type.

A similar ambiguity applies to a call like new_array<T[n]>(1). This does not appear to be prohibited by anything in the spec, but fails to compile in the current implementation.

Should we clarify/enforce that T must be a non-array type in one or both these functions, ie disallownew_<T[n]>() and/or new_array<T[n]>(m)? Or should we allow one or both and fix the implementation accordingly?

In terms of impact, it's possible someone is currently using new_<T[n]>(). However memory thus allocated is not deletable in the current implementation, unless one passes it to upcxx::deallocate (which happens to work, but definitely violates the spec). new_array<T[n]>(1) does not currently compile.

Prohibiting array types when instantiating these functions is easy to specify/enforce. However I'm slightly concerned about that decision's impact on generic programming parametric on T, where T may or may not be an array type. With these prohibitions in place, such code has no concise way to allocate and default initialize an object of type T, and would need to use type traits to query/decompose T and sfinae tricks to invoke our appropriate allocation routine. The alternative is we could specify array types are permitted and incorporate such tricks into our implementation to ensure array types are handled correctly.

Comments (3)

  1. Amir Kamil

    I’m leaning towards prohibition. Users should really be using std::array<T, n> rather than T[n], especially for dynamically allocated arrays. I don’t think we should twist our implementation into knots for this.

  2. Colin MacLean

    An array type for new_ doesn’t make much sense. It’s equivalent to:

    int (*arr)[N];
    arr = (int(*)[N]) malloc(sizeof(int)*N);
    

    This doesn’t make much sense to allow, because while it’s legal, it’s awkward to use:

    (*arr)[n];
    arr[0][n];
    

    However, new_array with an array type makes perfect sense. It simply creates an array whose type is an array: a “multidimensional” array. mallocing a pointer to an array type is how you get heap allocated VLAs in C99, for instance.

    int (*arr)[N];
    arr = (int(*)[N]) malloc(sizeof(int)*N*m);
    arr[i][j];
    

    For arrays, the allocator just needs to call placement new on every item, but that’s straightforward to do recursively with templates. new_array to create “multidimensional” arrays is definitely useful and I really don’t think it should be prohibited.

  3. Log in to comment