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

vector and deque erase_range on empty range calls MOVE_ALLOC(X, X) #211

Closed
jeanPerier opened this issue Feb 16, 2024 · 1 comment
Closed

Comments

@jeanPerier
Copy link

There is a test (see [1]) for vector and deque that calls erase_range(iterator, iterator) (empty range). This currently ends-up calling MOVE_ALLOC(X, X) (at [2] and [3]) which is not legal Fortran (see [4]).
While most compilers do not emit runtime errors, they silently deallocate X, which in the case of erase_range ends-up destroying the vector/deque elements after the iterator. I expect the intention for erase_range(iterator, iterator) is to be a no-op instead, the vector/deque code should probably do nothing in this case.

You can observe the issue if you add an ASSERT to test the element values (like ASSERT(v%at(2), two)) after the erase_range call in the test_erase_empty_range test.

gfortran builds will fail with:

[Test_deferred_stringVector.pf:851]
String assertion failed:
    expected: <"">
   but found: <"two">
  first diff: 

[1] :

next_iter = v%erase(iter, iter)

[2]:
__T_MOVE__(a%item, b%item)

[3]:
__T_MOVE__(a%item, b%item)

[4]: Fortran 2018 section 16.9.137 point 4 mandates: "if FROM and TO are the same variable, it shall be unallocated when MOVE_ALLOC is invoked"

I found this issue because flang generate runtime errors for such MOVE_ALLOC.

@tclune
Copy link
Member

tclune commented Feb 16, 2024

Ah - I don't think the problem is the use of MOVE_ALLOC per se. Rather the code should never go into the following loop when delta is 0:

delta=last%current_index-first%current_index
do i=last%current_index, this%size_
ii = this%idx(i)
associate( &
a => this%buckets(ii-delta)%ptr%bucket_items, &
b => this%buckets(ii)%ptr%bucket_items)
__T_MOVE__(a%item, b%item)
end associate
end do

The loop is meant to shift the elements but when the shift is 0, the loop is shifting each element on top of itself.

So maybe a bit more than one line, but still a simple fix in a few places.

@tclune tclune closed this as completed in 8a76459 Feb 16, 2024
tclune added a commit that referenced this issue Feb 16, 2024
…generate-erase-loops

Fixes #211 - degenerate erase() bug
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

No branches or pull requests

2 participants