search

breese / trial.circular

Circular span
Boost Software License 1.0
8 stars 2 forks source link

Fix first_unused_segment #2

Closed mtnpke closed 2 months ago

mtnpke commented 3 months ago

Hi, I found it strange that the const version of first_unused_segment is different from the non-const version and this, indeed, seems to be wrong. The tests then failed with the canonical (non-const) version due to an implementation difference of how next is used depending on the constructor, as far as I could tell. I tried my best to make it uniform. It still passes all tests now.

breese commented 3 months ago

Good catch.

Your fixes looks correct, but just to be sure I have rewritten the segment test suite to make it more exhaustive in b20b1d725cf6b0fb2a93b6c9be9b349c7ca05273.

The commit does not conflict with your changes, so you should be able to rebase them onto your branch.

I have disabled most of the tests for the const version of first_unused_segment(). You can enable them with the TEST_CONST_FIRST_UNUSED_SEGMENT macro in the test suite.

I apologize for any trouble that this may be causing.

mtnpke commented 2 months ago

Great thanks, I seem to have missed the other *segment() tests in the separate file. Also, my fix for the constructor was not entirely correct, it turns out. It failed when constructing from a completely filled source (leading to next = 2*capacity), which was detected by span_iterator_suite.

Everything should be fine now. At least the tests are happy :)