Skip to content

[libc++][test] Fix MaybePOCCAAllocator to finally meet the allocator requirements #74960

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

Merged
merged 1 commit into from
Dec 10, 2023

Conversation

StephanTLavavej
Copy link
Member

Found while running libc++'s test suite with MSVC's STL.

After @CaseyCarter's LLVM-D118279 c5ba46e "[libcxx][test] MaybePOCCAAllocator should meet the Cpp17Allocator requirements" followed by @philnik777's LLVM-D68365 98d3d5b "[libc++] Implement P1004R2 (constexpr std::vector)", one more change is necessary.

MSVC's constexpr vector implementation noticed this because we always rebind allocators.

MSVC's constexpr vector implementation noticed this because we always rebind allocators.
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner December 10, 2023 02:24
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 10, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 10, 2023

@llvm/pr-subscribers-libcxx

Author: Stephan T. Lavavej (StephanTLavavej)

Changes

Found while running libc++'s test suite with MSVC's STL.

After @CaseyCarter's LLVM-D118279 c5ba46e "[libcxx][test] MaybePOCCAAllocator should meet the Cpp17Allocator requirements" followed by @philnik777's LLVM-D68365 98d3d5b "[libc++] Implement P1004R2 (constexpr std::vector)", one more change is necessary.

MSVC's constexpr vector implementation noticed this because we always rebind allocators.


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

1 Files Affected:

  • (modified) libcxx/test/support/allocators.h (+1-1)
diff --git a/libcxx/test/support/allocators.h b/libcxx/test/support/allocators.h
index 2b987ad872783e..02436fd9c35ef1 100644
--- a/libcxx/test/support/allocators.h
+++ b/libcxx/test/support/allocators.h
@@ -209,7 +209,7 @@ class MaybePOCCAAllocator {
         : id_(id), copy_assigned_into_(copy_assigned_into) {}
 
     template <class U>
-    MaybePOCCAAllocator(const MaybePOCCAAllocator<U, POCCAValue>& that)
+    TEST_CONSTEXPR MaybePOCCAAllocator(const MaybePOCCAAllocator<U, POCCAValue>& that)
         : id_(that.id_), copy_assigned_into_(that.copy_assigned_into_) {}
 
     MaybePOCCAAllocator(const MaybePOCCAAllocator&) = default;

@StephanTLavavej
Copy link
Member Author

Thanks! The checks are green, modulo what #74987 is about to fix, and unrelated sporadic timeouts/failures. Merging...

@StephanTLavavej StephanTLavavej merged commit 2ca101f into llvm:main Dec 10, 2023
@StephanTLavavej StephanTLavavej deleted the stl-23-pocca-me-maybe branch December 10, 2023 13:20
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants