Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++][test] Non-rebindable test_alloc in string.capacity/deallocate_size.pass.cpp #113609

Closed
StephanTLavavej opened this issue Oct 24, 2024 · 1 comment · Fixed by #113638
Closed
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite

Comments

@StephanTLavavej
Copy link
Member

This test_alloc has a rebind struct, but not a rebinding constructor:

template <class T, class Sz>
struct test_alloc {
typedef Sz size_type;
typedef typename std::make_signed<Sz>::type difference_type;
typedef T value_type;
typedef value_type* pointer;
typedef const value_type* const_pointer;
typedef typename std::add_lvalue_reference<value_type>::type reference;
typedef typename std::add_lvalue_reference<const value_type>::type const_reference;
template <class U>
struct rebind {
typedef test_alloc<U, Sz> other;
};
TEST_CONSTEXPR_CXX14 pointer allocate(size_type n, const void* = nullptr) {
allocated_ += n;
return std::allocator<value_type>().allocate(n);
}
TEST_CONSTEXPR_CXX14 void deallocate(pointer p, size_type s) {
allocated_ -= s;
std::allocator<value_type>().deallocate(p, s);
}
};

This fails to meet the allocator requirements. MSVC's STL rejects this because we attempt to rebind the allocator (to an internal "container proxy" type).

I observe other requirement deficiencies here (no comparison operators) but the rebinding one is the specific problem for us.

@github-actions github-actions bot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 24, 2024
@frederick-vs-ja
Copy link
Contributor

When attempting to fix this, I realized that MSVC STL's "container proxy" objects can make allocated_ have different values when they are present... I wish assert(allocated_ == 0 || allocated_ >= i) passes as the "different value" should be larger than normal.

NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this issue Nov 4, 2024
…vm#113638)

Making it meet the requirements for allocator since C++11. Fixes
llvm#113609.

This PR doesn't make it meet the C++03 allocator requirements, because
that would make the type too verbose and libc++ has backported many
C++11 features to the C++03 mode.

Drive-by: Removes the `TEST_CONSTEXPR_CXX14` on `allocate`/`dealocate`
which is never in effect (and causes IFNDR-ness before C++23), since
these functions modify the namespace-scoped variable `allocated_`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants