[libcxx-commits] [libcxx] [libc++][test] Fix and refactor exception tests for std::vector (PR #117662)
Peng Liu via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Nov 27 08:09:36 PST 2024
winner245 wrote:
> Thanks a lot for the fixes, these tests are extremely important so it's good to fix them. Do you have an understanding of whether our tests for other (non-construction) operations are sufficient in terms of exceptions-testing?
Thank you for the question! I am glad that you asked, as it gives me an opportunity to elaborate on my recent work regarding exception safety. I have done a thorough analysis of vector, particularly focusing on exception safety. I found that we are lacking some critical exception tests, and some existing ones are flawed. I believe all vector operations providing strong exception guarantees should be tested under various exception scenarios. Here are my findings:
### Operations that provide strong guarantees:
- **Constructors**: We have decent exception test coverage for constructors, though a few important ones are either missing or problematic. With my recent PRs, I believe we have covered all constructors pretty well for both `vector` and `vector<bool>`.
- **`resize`, `reserve`,** and **`shrink_to_fit`**: Currently, there is only one exception test for `reserve` in `reserve.pass.cpp`, which only considers the allocation exception. Other sources of exceptions, such as element type exceptions during the relocation process, are not covered. Additionally, no exception tests are provided for `resize` and `shrink_to_fit`, althoug they are subject to the same sources of exceptions.
- **`emplace_back`** and **`push_back`**: Our current implementation of `push_back` calls `emplace_back`. Therefore, at least one of these operations requires a comprehensive exception test coverage. However, there is only one test for `push_back` in `push_back_exception_safety.pass.cpp`, which only considers copy-ctor exceptions, neglecting other exceptions such as allocation exception or other direct ctor exceptions.
### Remaining operations
What remains to analyze are the assignment functions, insertion functions, and erasure functions, which do not provide exception strong guarantees in general.
- **Erasure functions**: `pop_back` and `clear` do not throw exceptions, and thus do not need exception testing. The `erase` overloads throw only when the the assignment operators throw during the movement of the tail elements to the front after removal. So this boils down to the assignment operators.
- **Assignment functions** (including `operator=`, all `assign` overloads, and C++23's `assign_range`): The standard does not mandate strong exception guarantees, hence we do not have any exception tests coverage. However, my recent study indicated that we can achieve strong guarantees for assignment functions specifically during reallocations, without introducing any additional operations. I reviewed the implementations in GCC and MSVC and found that GCC has already adopted a similar approach to mine for providing strong guarantees in this context. This motivated my latest PR, which also includes a comprehensive coverage of all sources of exceptions for the assignment functions. I truly appreciate your positive feedback on this work and hope it generates interest in evaluating whether this improvement is necessary for libc++.
- **Insertion functions** (including all `insert` overloads and C++23's `insert_range`): The standard only requires these functions to provide a strong exception guarantee when the insertion is done at the end to insert a single-element (e.g., `a.insert(a.end(), x)`). Following my work on the assignment functions, I also developed a method to make all these insertion functions provide strong exception guarantees during reallocations. Again, this improvement is based on some reordering tricks but in a more tricky way, thus introducing no extra time cost. I will await the outcome of my PR for assignment functions before deciding whether to submit this one for review.
This summarizes my current understanding of exception safety for `vector`. I would love to hear if you have any comments or suggestions on my work.
https://github.com/llvm/llvm-project/pull/117662
More information about the libcxx-commits
mailing list