[libc++][test] Fix invalid low-level const conversion in limited_allocator

[winner245]

This PR fixes an invalid low-level const conversion bug in limited_allocator. As this class is a widely used utility for testing allocator-aware containers, it is important to fix it. It is a very simple fix by just adding a const qualification.

The bug did not crash our tests likely because our current usage of the template has not invoked the specific member function. However, we can trigger an immediate compilation error through explicit template instantiation or by using the operators == or !=.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

This PR fixes an invalid low-level const conversion bug in limited_allocator. As this class is a widely used utility for testing allocator-aware containers, it is important to fix it. It is a very simple fix by just adding a const qualification.

The bug did not crash our tests likely because our current usage of the template has not invoked the specific member function. However, we can trigger an immediate compilation error through explicit template instantiation or by using the operators == or !=.

---

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

1 Files Affected:

• (modified) libcxx/test/support/test_allocator.h (+4-1)

diff --git a/libcxx/test/support/test_allocator.h b/libcxx/test/support/test_allocator.h
index dcd15332ca304f..49dad642184da3 100644
--- a/libcxx/test/support/test_allocator.h
+++ b/libcxx/test/support/test_allocator.h
@@ -467,7 +467,10 @@ class limited_allocator {
   TEST_CONSTEXPR_CXX20 pointer allocate(size_type n) { return handle_->template allocate<T>(n); }
   TEST_CONSTEXPR_CXX20 void deallocate(pointer p, size_type n) { handle_->template deallocate<T>(p, n); }
   TEST_CONSTEXPR size_type max_size() const { return N; }
-  TEST_CONSTEXPR BuffT* getHandle() const { return handle_.get(); }
+
+  // In C++11, constexpr non-static member functions are implicitly const, but this is no longer the case since C++14.
+  TEST_CONSTEXPR_CXX14 BuffT* getHandle() { return handle_.get(); }
+  TEST_CONSTEXPR const BuffT* getHandle() const { return handle_.get(); }
 };
 
 template <class T, class U, std::size_t N>

[ldionne]

How did you come across this issue?

[winner245]

I was working on a draft PR #118141 adding exception tests for other vector operations, which heavily used the limited_allocator class and somehow triggered the usage of operator==.

[ldionne]

IMO we should instead do this:

Suggested change

	TEST_CONSTEXPR_CXX14 BuffT* getHandle() { return handle_.get(); }
	TEST_CONSTEXPR const BuffT* getHandle() const { return handle_.get(); }
	TEST_CONSTEXPR const BuffT* getHandle() const { return handle_.get(); }

Let's just provide the const version, since the non-const version is not needed. This method is only used by operator== below.

The comment can also be removed if we only have the const version. WDYT?

[winner245]

Agree. We only need a const version.

[ldionne]

(I ran clang-format separately and rebased your patch)