Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange behavior of prepend! with subarrays #16642

Open
bkamins opened this issue May 29, 2016 · 7 comments
Open

Strange behavior of prepend! with subarrays #16642

bkamins opened this issue May 29, 2016 · 7 comments
Labels
arrays [a, r, r, a, y, s]

Comments

@bkamins
Copy link
Member

bkamins commented May 29, 2016

The following code produces unexpected result:

x = [1, 2, 3]
y = sub(x, :)
prepend!(x, y)

and first three entries of x are garbage in result. The behavior is inconsistent with the following code:

x = [1, 2, 3]
y = x
prepend!(x, y)

The reason is that prepend! checks if a===items and otherwise assumes that items is unchanged when array a is grown at the beginning.

@TotalVerb
Copy link
Contributor

I don't know how easy it would be to completely fix this. Memory can be aliased in a lot of different ways.

@bkamins
Copy link
Member Author

bkamins commented May 29, 2016

But now we have undefined behavior of prepend! when memory is aliased (as in two examples above). The simple fix I see is to copy items before resizing of a but this looses performance.

@timholy
Copy link
Member

timholy commented May 29, 2016

I haven't spent (and ATM can't spend) any real time thinking about this, but it occurs that operations like push! and prepend! should perhaps throw an error on a view---views are not resizable, since the container is immutable.

@TotalVerb
Copy link
Contributor

Those operations already do throw errors on views, since they're not defined for them. The issue here is prepending to an array the elements of some view that is based on that array itself.

@tkelman
Copy link
Contributor

tkelman commented May 29, 2016

should be able to write a band aid SubArray specialized method that checks if the parent array is the one being prepended to, and does a copy if so?

@martinholters
Copy link
Member

martinholters commented May 30, 2016

I guess there are many more possibilities for hidden aliasing. An easy one:

a=collect(1:10);
a[2:6]=sub(a,1:5);
all(a[1:6].==1) # gives true

Similarly, other view-of-another-array types would be affected by the same problem. If it wasn't for the special treatment of Arrays by reshape (using internal data sharing), ReshapedArray would also be a hot candidate. My fear is that without a good strategy, we will need lots of band aid. For the prepend! case, an additional check a===parent(items) would do; maybe this could similarly be included in other functions that mutate an Array based on another AbstractArray.

EDIT: Not a case likely to occur in practice, but

a=collect(1:10);
prepend!(a, reshape(reshape(sub(a, 1:4), 2, 2), 4))

nicely exposes the problem with a ReshapedArray as items, so specializing on SubArray would not be sufficient.

@rfourquet rfourquet added the arrays [a, r, r, a, y, s] label Jun 19, 2020
@laborg
Copy link
Contributor

laborg commented Oct 19, 2023

Ref: #50824

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s]
Projects
None yet
Development

No branches or pull requests

7 participants