[libc++][test] Improve ThrowingT to Accurately Throw after throw_after > 1 Use

[winner245]

This PR fixes the ThrowingT class, which currently fails to raise exceptions after a specified number of copy construction operations. The class is intended to throw in a controlled manner based on a specified counter value throw_after. However, its current implementation of the copy constructor fails to achieve this goal.

The problem arises because the copy constructor does not initialize the throw_after_n_ member, leaving throw_after_n_ to default to nullptr as defined by the in-class initializer (int* throw_after_n_ = nullptr;). As a result, its copy constructor always checks against nullptr, causing an immediate exception rather than throwing after the specified number throw_after of uses. The fix is straightforward: simply initialize the throw_after_n_ member in the member initializer list.

This issue was previously uncovered because all exception tests for std::vector in exceptions.pass.cpp used a throw_after value of 1, which coincidentally aligned with the class's behavior.

A small test demonstrating the improper behavior of ThrowingT is available at Godbolt link.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

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

#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

---
Full diff: https://github.com/llvm/llvm-project/pull/114077.diff


1 Files Affected:

- (modified) libcxx/test/std/containers/sequences/vector/vector.cons/exceptions.pass.cpp (+3-2) 


``````````diff
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_;

[ldionne]

Thanks a lot for the fix -- good find!