[libcxx-commits] [libcxx] [libc++][test] Improve ThrowingT to Accurately Throw after throw_after > 1 Use (PR #114077)

[Peng Liu via libcxx-commits]

https://github.com/winner245 created https://github.com/llvm/llvm-project/pull/114077

### Summary
This PR proposes an improvement to the `ThrowingT` class used in the exception safety tests for `vector` (`libcxx/test/std/containers/sequences/vector/vector.cons/exceptions.pass.cpp`). The current implementation of the copy constructor may unintentionally throw immediately after its first use. The proposed changes aim to refine the control to exactly control that exception is thrown after the specified number of copy constructions. 

It seems that this issue was not previously discovered because all exception tests for `std::vector` used a `throw_after` value of 1. Under this specific condition, the class threw exceptions at the expected time purely by coincidence.

### Details
The `ThrowingT` class is intended to allow specifying a counter (throw_after) that controls when exceptions should be thrown during copy constructions. However, its copy constructor did not initialize the `throw_after_n_` member in the member initializer-list, leaving the `throw_after_n_` member initialized to `nullptr` through the in-class initializer (`int* throw_after_n_ = nullptr;`). As a result, the copy constructor always checks against `nullptr`, causing it to throw an exception immediately rather than after the specified number of copies.

The proposed changes include initializing `throw_after_n_` in the copy constructor and assigning to it in the copy assignment operator, ensuring that exceptions are thrown only after the specified number of operations, for any `throw_after > 1`.

 
### Test 
[Godbolt link](https://godbolt.org/z/PfsheM8Mf)
```cpp
#include <iostream>
#include <vector>  

struct ThrowingT {
  int* throw_after_n_ = nullptr;
  ThrowingT() { throw 0; }

  ThrowingT(int& throw_after_n) : throw_after_n_(&throw_after_n) {
    if (throw_after_n == 0)
      throw 0;
    --throw_after_n;
  }

  ThrowingT(const ThrowingT&) {
    if (throw_after_n_ == nullptr || *throw_after_n_ == 0)
      throw 1;
    --*throw_after_n_;
  }

  ThrowingT& operator=(const ThrowingT&) {
    if (throw_after_n_ == nullptr || *throw_after_n_ == 0)
      throw 1;
    --*throw_after_n_;
    return *this;
  }
};


struct ThrowingT_Fix {
  int* throw_after_n_ = nullptr;
  ThrowingT_Fix() { throw 0; }

  ThrowingT_Fix(int& throw_after_n) : throw_after_n_(&throw_after_n) {
    if (throw_after_n == 0)
      throw 0;
    --throw_after_n;
  }

  ThrowingT_Fix(const ThrowingT_Fix& rhs) : throw_after_n_{rhs.throw_after_n_} {
    if (throw_after_n_ == nullptr || *throw_after_n_ == 0)
      throw 1;
    --*throw_after_n_;
  }

  ThrowingT_Fix& operator=(const ThrowingT_Fix& rhs) {
    throw_after_n_ = rhs.throw_after_n_;
    if (throw_after_n_ == nullptr || *throw_after_n_ == 0)
      throw 1;
    --*throw_after_n_;
    return *this;
  }
};

int main() { 
    try { // Throw in vector(size_type, value_type) from type
        int throw_after = 999;
        ThrowingT v(throw_after);
        std::vector<ThrowingT> get_alloc(1, v);  // This should not throw!
    } catch (int) {
        std::cout << "get_alloc(1, v) throwed unexpectedly\n";
    }

    try { // Throw in vector(size_type, value_type) from type
        int throw_after = 999;
        ThrowingT_Fix v(throw_after);
        std::vector<ThrowingT_Fix> get_alloc(998, v);  // do not throw, as expected
    } catch (int) {
        std::cout << "get_alloc(998, v) throwed unexpectedly\n";
    }
    std::cout << "get_alloc(998, v) did not throw as expected\n";

    try { // Throw in vector(size_type, value_type) from type
        int throw_after = 999;
        ThrowingT_Fix v(throw_after);
        std::vector<ThrowingT_Fix> get_alloc(999, v); // throw, as expected
    } catch (int) {
        std::cout << "get_alloc(999, v) throwed as expected\n";
    }
}
``` 

#### Testing Result
```
get_alloc(1, v) throwed unexpectedly
get_alloc(998, v) did not throw as expected
get_alloc(999, v) throwed as expected

>From 1fa679242c90af106d565ccda1b0a8f167d3cd0e Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Tue, 29 Oct 2024 11:22:41 -0400
Subject: [PATCH] Improve ThrowingT to accurately control when to throw

---
 .../sequences/vector/vector.cons/exceptions.pass.cpp         | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libcxx/test/std/containers/sequences/vector/vector.cons/exceptions.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.cons/exceptions.pass.cpp
index c9c1bac2fb4a0c..bd9ea9e1b660ce 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.cons/exceptions.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.cons/exceptions.pass.cpp
@@ -49,13 +49,14 @@ struct ThrowingT {
     --throw_after_n;
   }
 
-  ThrowingT(const ThrowingT&) {
+  ThrowingT(const ThrowingT& rhs) : throw_after_n_{rhs.throw_after_n_} {
     if (throw_after_n_ == nullptr || *throw_after_n_ == 0)
       throw 1;
     --*throw_after_n_;
   }
 
-  ThrowingT& operator=(const ThrowingT&) {
+  ThrowingT& operator=(const ThrowingT& rhs) {
+    throw_after_n_ = rhs.throw_after_n_;
     if (throw_after_n_ == nullptr || *throw_after_n_ == 0)
       throw 1;
     --*throw_after_n_;