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

append!, prepend!, and splice! should not modify arrays if convert throws #7642

Closed
simonster opened this issue Jul 17, 2014 · 4 comments
Closed
Labels
won't change Indicates that work won't continue on an issue or pull request

Comments

@simonster
Copy link
Member

The dequeue functions that take scalars are carefully written to call convert before resizing the vector, but the functions that take vectors (append!, prepend!, and splice!) mutate the array but leave the corresponding values uninitialized. It would be preferable if these functions either checked if convert fails before mutating the array, or undid the changes on failure. See also julia-users discussion.

quinnj added a commit that referenced this issue Jul 20, 2014
…rray elements. This causes a MethodError to be thrown before any array growing/shrinking happens. Fixes #7642
@quinnj quinnj mentioned this issue Jul 20, 2014
@JeffBezanson
Copy link
Member

I don't think this is a bug.

@StefanKarpinski
Copy link
Member

This strikes me as pretty buggy behavior. What's the rationale for this not being a bug?

@JeffBezanson
Copy link
Member

If a mutating operation fails, the object's state after the error is not well defined. But it would be nice to do mutations atomically when there is no cost. In this case I don't yet see a cheap way to do this.

@StefanKarpinski
Copy link
Member

You can try converting one element first and bail out if that fails. This won't make this atomic, but it will make it less likely to leave the array to be spliced into in a broken state.

quinnj added a commit that referenced this issue Jul 22, 2014
quinnj added a commit that referenced this issue Jul 23, 2014
… conversion fails. This allows us to un-modify the original arrays so the user gets back what they put in. Fixes #7642
quinnj added a commit that referenced this issue Jul 23, 2014
…conversion fails. If it does, the original arrays are rewound to their original state. Fixes #7642
quinnj added a commit that referenced this issue Jul 24, 2014
… the same typed arrays. This guards against the possibility of failing on arrays of different types and messing up the original array. Fixes #7642
quinnj added a commit that referenced this issue Jul 31, 2014
…e for atomicity. Adds further method definitions when array element types don't match to try to convert the second array to the element type of the first. Fixes #7642
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
won't change Indicates that work won't continue on an issue or pull request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants