Enforce strides to be not null when ndim is nonzero

[tqchen]

This PR updates the spec to enforce the strides to be not null when the ndim is nonzero.
This is going to help consumers to handle cases more uniformly.

[hpkfft]

I suggest requiring NULL if ndim == 0. That is:

 *  which is NULL if and only if ndim == 0, pointing to
 *  an array of ndim elements that specify the strides.

[seberg]

While I do think suggesting it makes sense, I am not sure I see a benefit (how can you rely on it)?

On a general note, we really can't just change this without leaving a comment? From a purity point of view: before version 1.2 the strides were allowed to be NULL when the array was C contiguous.
I'll settle for something fuzzier, but I feel we need a paper trail (for at least a year or so)?

It's not like we know nobody does this!

[tqchen]

let us go with suggesting it instead of requiring it for now. Agree on @seberg to add the paper trail here

[hpkfft]

What do you mean by "it"?
I do not think it makes sense to suggest strides be NULL if ndim == 0 if you're going to require strides not to be NULL when ndim > 0.
Either suggest both (and then there is no need to increment the version number), or require both (in v1.2).

[tqchen]

I meant the ndim == 0 indicate strides == NULL. Like @seberg said, I agree it makes sense, however, also not sure see a benefit where one can rely on it(unlike the strides not null case), so would be good to know what are potential usecases here

[seberg]

I would frame it as: the consumer can rely on strides[dim] working. Apparently that was desired.
To me that isn't much about memory safety, but maybe you can convince me that there is a serious memory safety angle somewhere additionally.

[hpkfft]

So, yeah, fundamentally just a code quality thing.
But I do think the clause "which is NULL if and only if ndim == 0" is clearer to humans than "cannot be NULL if ndim != 0".
From a verification point of view, it's easier to write a test suite when a standard specifies everything rather than when it leaves the value of strides unspecified in corner cases.
Just my two cents--I'm not trying to make trouble if others prefer to leave it unspecified.

[hpkfft]

If ndim == 1, then (in the future) the consumer can rely on strides[0] working. Yes, I see that.

@seberg, in NumPy numpy/numpy#29872, we now have:

  dl_tensor->shape = (ndim > 0) ? (int64_t *)((char *)ptr + offset) : NULL;
  dl_tensor->strides = (ndim > 0) ? dl_tensor->shape + ndim : NULL;

That seemed to me to be worth the minor inconvenience.
Otherwise, shape and strides would point past the end of allocated memory.
Is that still OK with you?

@tqchen, in the opening comment of #177, you wrote, "Enforcing strides to be not NULL would help with memory safety here. " Would you be willing to further explain your motivation, and perhaps, what you feel is best from a memory safety point of view if ndim ==0?

[tqchen]

i somewhat agree with @seberg that consumers can rely on strides[dim] working for dim that 0<= dim < ndim

[tqchen]

specificially, i think it is OK for shape and strides(and maybe desirable from the implementation simplicity pov) to point to the end of allocated memory for ndim=0 case

[tqchen]

Updated to add the suggestion to set pointers to NULL when ndim==0, add notes about change in DLPack v1.2

[tqchen]

#177

[hpkfft]

Minor: "handling" is misspelled.
Do you have evidence that most frameworks already always return non-NULL strides when ndim != 0?
NumPy, for example, returns NULL for C-contiguous arrays before today's not-yet-released development version.
Even if the statement is true for most frameworks, it may not be useful to mention it in the header since consumers ought to correctly handle NULL anyway (e.g., to support the current and past releases of NumPy).

[tqchen]

noted, updated the typo. And rmeoved the note on frameworks here

[tqchen]

Thanks everyone for the feedbacks. We updated to explicit state the constraint
consumer can always rely on strides[dim] being valid for 0 <= dim < ndim.

This ensures that the standard still allows shape/strides to points to the last of the allocated memory when ndim=0, and not having to special case for the scenario

[tqchen]

most outstanding comments are resolved, see also note above on the rationale on the ndim=0 case, i think we can plan to merge soon soon and have a 1.2 release. Thanks @hpkfft @seberg and everyone for chiming in

not enforcing null per notes in discussion

[tqchen]

planning to merge in 24hours