Fix infinite loop in xt::roll on arrays with a zero-length dimension#2922
Open
f14XuanLv wants to merge 1 commit into
Open
Fix infinite loop in xt::roll on arrays with a zero-length dimension#2922f14XuanLv wants to merge 1 commit into
f14XuanLv wants to merge 1 commit into
Conversation
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Description
xt::rollenters an infinite loop (and then divides by zero) when the inputhas a zero-length axis:
roll(e, shift)whene.size() == 0:while (shift < 0) shift += flat_size;loops forever withflat_size == 0,then
shift %= 0is undefined behaviour.roll(e, shift, axis)when the rolled axis has length 0:same issue on
axis_dim == 0.Fix
For a shape$s = (s_0, \dots, s_{n-1})$ ,$\exists k: s_k = 0$ , then $\mathrm{roll}(a, \mathrm{shift}, \mathrm{axis}) = a$ ,
if
i.e. roll is the identity.
In both overloads, after constructing
cpy = empty_like(e), returncpyearly when
cpy.size() == 0. The no-axis overload's hand-rolledstd::accumulate(shape, 1L, multiplies<size_t>())is replaced bycpy.size()(numerically equivalent, and O(1) on contiguous layouts).
When the rolled axis is non-empty but another axis has length 0
(e.g.
shape={3,0,4}rolled along axis 0), the original code didn't loop —detail::roll's inner loops became no-ops because the stride productcollapses to 0 — but the recursion was wasted work. The widened guard
cpy.size() == 0skips it cleanly. Observable output is unchanged.Tests
Three regression cases in
test_xmanipulation.cpp, one per code path:{0}{3, 0}1{3, 0, 4}0All
test_xmanipulationtests pass: 28 cases, 188 assertions. Verifiedagainst
numpy.roll, which returns a same-shape empty array for these inputs.