Conversation
|
WalkthroughThe 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
contracts/utils/Arrays.sol (1)
139-146: Skip recursive calls for partitions smaller than 2 elementsThis branch still calls
_quickSorton empty/singleton sides, which is safe but adds avoidable overhead in worst-case pivot distributions. Consider guarding recursion with a>= 0x40size check.♻️ Proposed micro-optimization
- if (pos - begin < end - (pos + 0x20)) { - // left smaller → recurse left, loop on right - _quickSort(begin, pos, comp); - begin = pos + 0x20; - } else { - // right smaller or equal → recurse right, loop on left - _quickSort(pos + 0x20, end, comp); - end = pos; - } + uint256 rightBegin = pos + 0x20; + uint256 leftSize = pos - begin; + uint256 rightSize = end - rightBegin; + + if (leftSize < rightSize) { + // left smaller → recurse left, loop on right + if (leftSize >= 0x40) _quickSort(begin, pos, comp); + begin = rightBegin; + } else { + // right smaller or equal → recurse right, loop on left + if (rightSize >= 0x40) _quickSort(rightBegin, end, comp); + end = pos; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/utils/Arrays.sol` around lines 139 - 146, The _quickSort branch currently recurses on partitions that may be empty or singleton; add explicit size guards so you only recurse when a partition has at least two elements (>= 0x40 bytes). Specifically, before calling _quickSort(begin, pos, comp) ensure (pos - begin) >= 0x40, and before calling _quickSort(pos + 0x20, end, comp) ensure (end - (pos + 0x20)) >= 0x40; keep the existing loop-by-tail logic (updating begin = pos + 0x20 or end = pos) when the guarded recursion is skipped so behavior is unchanged. Include checks around the recursive calls in the _quickSort function using the existing variables begin, end, pos, 0x20 and 0x40.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@contracts/utils/Arrays.sol`:
- Around line 139-146: The _quickSort branch currently recurses on partitions
that may be empty or singleton; add explicit size guards so you only recurse
when a partition has at least two elements (>= 0x40 bytes). Specifically, before
calling _quickSort(begin, pos, comp) ensure (pos - begin) >= 0x40, and before
calling _quickSort(pos + 0x20, end, comp) ensure (end - (pos + 0x20)) >= 0x40;
keep the existing loop-by-tail logic (updating begin = pos + 0x20 or end = pos)
when the guarded recursion is skipped so behavior is unchanged. Include checks
around the recursive calls in the _quickSort function using the existing
variables begin, end, pos, 0x20 and 0x40.
|
Duplicate of #6324 |
This comment was marked as spam.
This comment was marked as spam.
3 participants
Fixes #6289
Fixed the _quickSort implementation in Arrays.sol to prevent stack overflow errors on large or reverse-sorted arrays.
Changes- uses "while " in order to prevent the overflow because in (true) loop it just assign slots in one time and just swap there while in recursion slots give in each recursive state :)