[libcxx-commits] [libcxx] [libc++] Add exception guard for vector<bool>::__init_with_sentinel (PR #115491)

[Peng Liu via libcxx-commits]

================
@@ -0,0 +1,87 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef EXCEPTION_TEST_HELPERS_H
+#define EXCEPTION_TEST_HELPERS_H
+
+#include "count_new.h"
+
+template <class T>
+struct throwing_allocator {
+  using value_type      = T;
+  using is_always_equal = std::false_type;
----------------
winner245 wrote:

Thank you for this comment, which motivated me to think more deeply about the issue. I just realized that I must have misunderstood the question initially. Now I believe the question pertains to the line `using is_always_equal = std::false_type;`, which appears counter-intuitive for a stateless allocator class. 

This line was in the original class `Allocator` before my refactor, my best guess is that the original author intended to use this class to test the exception safety for the constructor `vector(vector&&, const allocator_type&)` in `vector/vector.cons/exceptions.pass.cpp`: 

```cpp
try { // Throw in vector(vector&&, const allocator_type&) from type
    std::vector<ThrowingT, Allocator<ThrowingT> > vec(Allocator<ThrowingT>(false));
    int throw_after = 1;
    vec.emplace_back(throw_after);
    std::vector<ThrowingT, Allocator<ThrowingT> > vec2(std::move(vec), Allocator<ThrowingT>(false));
  } catch (int) {
  }
  check_new_delete_called();
```

This test aims to verify the exception safety for the constructor while performing an O(n) element-wise move (rather than the O(1) resource stealing), which requires the instances of the Allocator class to compare unequal. However, merely  setting `Allocator::is_always_equal::value` to false does not ensure that the instances of this class are unequal, which renders the above test actually testing nothing, as you also confirmed [here](https://github.com/llvm/llvm-project/pull/116449#pullrequestreview-2462660650). 
 

Beyond this scenario, I couldn't find any reason why we might need this member type. I believe we can safely remove it. Would you suggest I remove it in this PR or in my next one while rebasing?

https://github.com/llvm/llvm-project/pull/115491