Add documentation for realloc and resize possibly not modifying argument #235
Conversation
| @@ -9,6 +9,7 @@ Usage: | |||
| ``` | |||
|
|
|||
| Reallocates a view to have the new dimensions. Can grow or shrink, and will not preserve content. | |||
| May not modify the view, if sizes already match. | |||
There was a problem hiding this comment.
Maybe a native speaker can chime in but I feel like "may not" is unfortunate because it could be interpreted as "[presumably the implementation] is not allowed"
There was a problem hiding this comment.
Yeah, that is a reasonable mis-interpretation. The intended meaning would be conveyed (with more explicit precision) by "Might not modify the view, if its extents match those requested"
There was a problem hiding this comment.
@PhilMiller , @dalg24 - regarding line 12 above; why not be definitive, and say "if sizes already match, the View will not be modified." Here, "may" suggests other possible outcomes, and there are none, if the requested sizes are the same as the current sizes , correct?
There was a problem hiding this comment.
This is reaching back to very old discussion, but I believe the intention was to avoid committing the implementation to any particular outcome. Once it's documented to definitely do something, that's a nearly-eternal promise.
| ## Possibly Unexpected Behavior Warning | ||
|
|
||
| `realloc` will only modify the specific `View` instance passed to it. | ||
| Any other `View` which aliases the same allocation will be unmodified. | ||
| Consequently, if the `use_count()` of the `View` is larger than 1, the | ||
| old allocation will not be deleted. | ||
| Note that if the size arguments already match the extents of the `View` | ||
| argument, that `realloc` may not create a new `View`. |
There was a problem hiding this comment.
Did you consider having a code snippet to illustrate how this may have potentially surprising behavior?
Also should we spell out that it is best to think of if as what std::realloc is to raw pointers, that is the original view is "invalidated" and that keep using another object that aliases that same original view is a bad idea.
|
I agree that this (with the noted rewording) accurately enough describes the current behavior, but I strongly feel that it's not the behavior we should provide. |
|
For cross-reference purposes, this was opened to start addressing kokkos/kokkos#5589 |
|
this will need to be redone on top of the rst file |
No description provided.