Skip to content

feat(deque): Add blit_to and append method #2397

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

Open
wants to merge 60 commits into
base: main
Choose a base branch
from

Conversation

ethereal-dream
Copy link
Contributor

No description provided.

Change implementation of deque::join
Change truncate from function to array method
Change join,drain,flatten from function to deque method
Change the implementation of T::drain
Copy link

Incorrect tail pointer calculation in append method

Category
Correctness
Code Snippet
self.tail = self.tail + src_len (line 1588)
Recommendation
Calculate tail based on the new buffer structure: self.tail = dst_len + src_len - 1 or verify the deque's internal invariants for tail positioning
Reasoning
The tail pointer should represent the last valid element index. Simply adding src_len to the existing tail doesn't account for the fact that we've created a completely new buffer where elements are laid out differently.

Potential buffer layout mismatch in blit operations

Category
Correctness
Code Snippet
UninitializedArray::unsafe_blit(new_dst, 0, self.buf, 0, dst_len) (line 1583)
UninitializedArray::unsafe_blit(new_dst, dst_len, other.buf, 0, src_len) (line 1584)
Recommendation
Use logical iteration methods that respect the deque's circular buffer structure instead of direct buffer copying, or ensure both deques are in contiguous layout before blitting
Reasoning
Deques typically use circular buffers where elements may wrap around. Copying raw buffer segments (0 to length) may not preserve the logical order of elements if the deque's head/tail pointers indicate a wrapped state.

Inefficient memory allocation in append method

Category
Performance
Code Snippet
let new_dst = UninitializedArray::make(dst_len + src_len)
UninitializedArray::unsafe_blit(new_dst, 0, self.buf, 0, dst_len)
UninitializedArray::unsafe_blit(new_dst, dst_len, other.buf, 0, src_len)
Recommendation
Check if self has sufficient capacity before allocating. If capacity allows, use in-place operations: iterate through other and push_back each element to self
Reasoning
Always allocating a new buffer is wasteful when the existing buffer might have enough capacity. Element-wise copying via push_back would be safer and potentially more efficient for small appends.

@coveralls
Copy link
Collaborator

coveralls commented Jul 2, 2025

Pull Request Test Coverage Report for Build 234

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 89.234%

Totals Coverage Status
Change from base Build 193: 0.0%
Covered Lines: 3473
Relevant Lines: 3892

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants