[libcxx][NFC] Make sequence containers slightly more SFINAE-friendly during CTAD.

[var-const]

Summary

Disable the constructors taking `(size_type, const value_type&,
allocator_type) if allocator_type` is not a valid allocator.
Otherwise, these constructors are considered when resolving e.g.
(int*, int*, NotAnAllocator()), leading to a hard error during
instantiation. A hard error makes the Standard's requirement to not
consider deduction guides of the form `(Iterator, Iterator,
BadAllocator)` during overload resolution essentially non-functional.

The previous approach was to SFINAE away allocator_traits. This patch
SFINAEs away the specific constructors instead, for consistency with
basic_string -- see [LWG3076](wg21.link/lwg3076) which describes
a very similar problem for strings (note, however, that unlike LWG3076,
no valid constructor call is affected by the bad instantiation).

[var-const]

Note: testing the difference between the old and new SFINAE (differing in whether the parameter is deduced) is tricky. The non-deduced version works well on Clang for SFINAE purposes -- I can be wrong, but it seems like implementation-specific behavior (or worse, undefined behavior that happens to "work") that we cannot rely on. In tests, the difference only comes up in type traits like static_assert(std::is_default_constructible<std::vector<int, BadAlloc>>) -- it looks like the compiler checks all function signatures but not function bodies in that case, leading to a situation where there's a hard error due to incorrect SFINAE while "obvious" errors in the function bodies when the bad allocator is used are ignored.

[• Quuxplusone]

For testing, I think you've got the right idea: the only way to test this (besides ugly/fragile .verify.cpp tests) will be SFINAE. You just implemented SequenceContainerDeductionGuidesSfinaeAway in a previous patch, right? So shouldn't this just be a one- or two-line addition to that function?

// Cannot deduce from (size_type, value_type, BAD_alloc)
static_assert(SFINAEs_away<Container, size_t, void*, BadAlloc>);

If that doesn't work as a regression-test, then I'm confused about the effect of this patch.

libcxx/include/deque

1275

Why the extra space on all these lines? Just to highlight this section in Phab? (I would guess "removing hard tabs," but we reject hard tabs at buildkite time.)

1293–1294

This change seemed OK until I realized that I didn't understand the maze that gets us from _Allocator (on line 1293) to allocator_type (on line 1294). From the context, I assume that allocator_type here is understood by the compiler as an alias for __base::allocator_type i.e. __deque_base<_Tp, _Allocator>::allocator_type i.e. _Allocator i.e. deque's own _Allocator parameter, and then CTAD applies. I just don't fully understand how the CTAD machinery is working — it has to instantiate __deque_base<SomeType, BadAllocator> in order to find out that __deque_base<_Tp, _Allocator>::allocator_type is _Allocator?
But when I try that myself on Godbolt, the compiler can't thread that maze.
https://godbolt.org/z/oMTqqvhhW
I must be doing something wrong in that Godbolt, but what?

libcxx/include/string

855

While you're here, please remove , nullptr_t from the existing default argument.

[var-const]

In D114311#3144651, @Quuxplusone wrote:

For testing, I think you've got the right idea: the only way to test this (besides ugly/fragile .verify.cpp tests) will be SFINAE. You just implemented SequenceContainerDeductionGuidesSfinaeAway in a previous patch, right? So shouldn't this just be a one- or two-line addition to that function?

// Cannot deduce from (size_type, value_type, BAD_alloc)
static_assert(SFINAEs_away<Container, size_t, void*, BadAlloc>);

If that doesn't work as a regression-test, then I'm confused about the effect of this patch.

Yes, this works (the existing

// Cannot deduce from (iter, iter, BAD_alloc)
static_assert(SFINAEs_away<Container, Iter, Iter, BadAlloc>);

works as well -- it would give a hard error without either this patch or the previous approach that SFINAEd away allocator_traits).

What I meant to say is that it's hard to test for the difference between having one or two arguments in the template function:

// This is, IIUC, the correct form because it makes sure that `__is_allocator` is called on a deduced type.
template <class _MaybeAllocator = _Allocator, class = __enable_if_t<__is_allocator<_MaybeAllocator>::value> >
// However, this works as well, even though `_Allocator` is not, AFAICT, a deduced type. It seems to me that
// it shouldn't work and perhaps Clang is being permissive here -- otherwise I must be missing something.
template <class = __enable_if_t<__is_allocator<_Allocator>::value> >

I couldn't think of a great way to test this difference. In existing tests, it only shows up in certain type traits, e.g. static_assert(std::is_default_constructible_v<T>) works with the 2-argument form but not 1-argument form. However, it seemed like something that _happened_ to depend on this difference rather than actively check for it, if that makes sense.

[• Quuxplusone]

libcxx/include/forward_list

670

(1) Don't you need to guard this one on __is_allocator<allocator_type>, too? I think we're missing this case in forwardlist.cons/deduct.fail.cpp.
(2) Personally I'd be interested in just disabling the implicit guides for all these ctors (via __identity_t), and using explicit deduction guides for everything instead. Seems like explicit deduction guides would be a lot less error-prone and a lot more self-documenting, and I believe we'd be allowed to do it under the As-If Rule (I don't think a user could detect our trick).

673–677

Yes, this works (the existing SFINAEs_away<Container, Iter, Iter, BadAlloc> works as well -- it would give a hard error without either this patch or the previous approach that SFINAEd away allocator_traits)

Well, an existing test can't be a regression test, by definition. It already passed without this patch. :) Unless you're calling this PR "NFC [no functional change [intended]]", then it ought to have some functional effect and that means we ought to be able to write a regression test for that specific effect.

What I meant to say is that it's hard to test for the difference between having one or two arguments in the template function:

// This is, IIUC, the correct form because it makes sure that `__is_allocator` is called on a deduced type.
template <class _MaybeAllocator = _Allocator, class = __enable_if_t<__is_allocator<_MaybeAllocator>::value> >
// However, this works as well, even though `_Allocator` is not, AFAICT, a deduced type. It seems to me that
// it shouldn't work and perhaps Clang is being permissive here -- otherwise I must be missing something.
template <class = __enable_if_t<__is_allocator<_Allocator>::value> >

I couldn't think of a great way to test this difference.

I think it's going to be difficult/impossible in this case, yeah. Here's a reduced test case:
https://godbolt.org/z/cbaM9hsxj
Normally the one-template-argument version is wrong because the __enable_if_t it will be instantiated relatively eagerly. (As soon as the class definition is instantiated, it'll instantiate all of the member functions' declarations, which will include the one that depends on a non-existent enable_if_t<false> type.)
However, in this case, CTAD happens "more eagerly than that." First we transform the constructor template declaration into CTAD's fictional function template declaration, discover that it'd be ill-formed, and CTAD fails; so then we decide not to instantiate the class definition at all.
However, if someone later attempts to instantiate vector<int, NotAnAllocator> by name, the single-template-argument ctor will cause failure, where the two-template-argument ctor would not cause failure.
However, in this case, we don't care because instantiating vector<int, NotAnAllocator> is not allowed.
Basically, the two-template-argument version template<class Y=X, class=enable_if_t<f(Y)>> is absolutely needed when !f(X) is legal but you're just trying to SFINAE away part of the class's API in that case. When !f(X) is flat-out illegal and you're just trying to teach that fact to CTAD, you can get away with the one-template-argument version.

[ldionne]

My understanding is that this is effectively a NFC, since you were previously using __allocator_traits to achieve the exact same effect. This is only making all the containers consistent. That's why there are no test changes, right?

libcxx/include/deque

1293

It's not really a "maybe" allocator. It's definitely _Allocator. I've also used _DependentAlloc in the past (e.g. in <tuple>) for the same purpose, in case you like that better.

libcxx/include/forward_list

670

On (2), my vote would be towards keeping our implementation as close to the Standard as possible, which means keeping a mix of implicit and explicit deduction guides like we do right now. I don't like how the Standard is specified in that regard (I'd rather have explicit deduction guides only*), however at least we can more easily report issues against the Standard if we don't stray away from it, and it's easier to validate whether we are conforming.

(*) I'd be even happier if implicit deduction guides didn't even exist, but that's a different story.

673–677

Interesting, thanks for your analysis. I still think we should go with the two argument case for simple consistency and to avoid confusion. The fact that CTAD deduction guide generation will not cause the class to be instantiated is IMO an interesting detail, but one what folks shouldn't have to think about when reading this code.

[var-const]

Feedback

libcxx/include/deque

1275

Removed (I think).

1293

What I was trying to convey is that the type is being tested for whether it is an allocator. Changed to _DependentAlloc.

1293–1294

I think that, unlike the other type aliases, allocator_type doesn't depend on the base class. See line 1257:

typedef _Allocator allocator_type;

libcxx/include/forward_list

670

(1) I'm not sure. My primary concern was with the overloads that overlap with explicit deduction guides, and I haven't seen a case where this two-argument overload would cause issues. I'd rather keep the scope limited because expanding it later if necessary is straightforward whereas removing the additional constraints, should the need arise, would likely break users.

673–677

+1, thanks a lot for the detailed analysis!

I have marked this patch as [NFC] because it simply changes the mechanism for achieving the same effect (making sure bad allocators don't cause hard errors).

We have to use the two-argument version because the single-argument version causes compilation errors on test code like static_assert(std::is_default_constructible<std::vector<int, BadAlloc>>).

libcxx/include/string

855

Done.

[ldionne]

Thanks! LGTM but the C++03 CI will have to pass. I haven't looked too closely but I don't think it's a major issue.

libcxx/include/deque

1293

Oh, I see what you were doing now. Hmm I guess I'm neutral on the rename then. I guess I'm still slightly happier with _DependentAlloc since it's a tiny bit more consistent with other places in the library.

[• Quuxplusone]

No objection from me, modulo that the C++03 forward_list test is still failing for some reason.

libcxx/include/deque

1293–1294

Ah, I'd missed that line. Okay, there's less of a maze than I thought. :)
I still think it's confusingly subtle that the _Allocator on line 1293 is understood by the compiler as the same type as the allocator_type on line 1294, but I have no concrete suggestion to make it better. (We could use allocator_type on line 1293: _DependentAlloc = allocator_type: that's still kinda weird. Or we could use _Allocator on line 1294: size_type __n, const value_type& __v, const _Allocator& __a: that's also weird.) No action required AFAIC.

libcxx/include/forward_list

673–677

We have to use the two-argument version because the single-argument version causes compilation errors on test code like static_assert(std::is_default_constructible<std::vector<int, BadAlloc>>).

IIUC, that's expected and a good thing. A user who instantiates std::vector<int, BadAlloc> is in IFNDR-land and deserves a hard error, just as if they instantiated std::vector<int&, std::allocator<int&>> or std::function<int*>.
I don't think we should jump through any hoops to either permit or reject that bad code: let's just do whatever seems natural. I'm not necessarily agitating in favor of the single-argument version, just disagreeing with the conclusion that we have to do the two-argument version in order to accept this IFNDR code. In particular, let's not add any tests that rely on our specific behavior in this case.

[var-const]

Fix forward_list in C++03 mode.

[ldionne]

libcxx/include/forward_list

673–677

I think I agree with this. I guess we should be using the one argument version in that case, to avoid promoting the use of something crazy like std::vector<int, bad_alloc>. It's just unfortunate that it will work with CTAD through this kind of complex chain of things.

[var-const]

Switch to use the one-argument version.

[ldionne]

Thanks! Also, nice that we are fixing tests that were previously IFNDR!

libcxx/include/__memory/allocator_traits.h

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
#endif
     template <class _Ap = _Alloc, class = void, class =
         __enable_if_t<!__has_select_on_container_copy_construction<const _Ap>::value> >
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
     static allocator_type select_on_container_copy_construction(const allocator_type& __a) {
         return __a;
     }
 };
 
-// A version of `allocator_traits` for internal usage that SFINAEs away if the
-// given allocator doesn't have a nested `value_type`. This helps avoid hard
-// errors when forming implicit deduction guides for a container that has an
-// invalid Allocator type. See https://wg21.link/LWGXXXXX.
-// TODO(varconst): use the actual link once available.
-template <class _Alloc, class _ValueType = typename _Alloc::value_type>
-struct _LIBCPP_TEMPLATE_VIS __allocator_traits : allocator_traits<_Alloc> {};
-
 template <class _Traits, class _Tp>
 struct __rebind_alloc_helper {
 #ifndef _LIBCPP_CXX03_LANG
     using type _LIBCPP_NODEBUG = typename _Traits::template rebind_alloc<_Tp>;
 #else
     using type = typename _Traits::template rebind_alloc<_Tp>::other;
 #endif
 };

libcxx/include/deque

 }
 
 template <class _Tp, class _Allocator>
 class __deque_base
 {
     __deque_base(const __deque_base& __c);
     __deque_base& operator=(const __deque_base& __c);
 public:
-    typedef _Allocator                               allocator_type;
-    typedef allocator_traits<allocator_type>         __alloc_traits;
-    typedef typename __alloc_traits::size_type       size_type;
+    typedef _Allocator                                allocator_type;
+    typedef allocator_traits<allocator_type>          __alloc_traits;
+    typedef typename __alloc_traits::size_type        size_type;
 
-    typedef _Tp                                      value_type;
-    typedef value_type&                              reference;
-    typedef const value_type&                        const_reference;
-    typedef typename __alloc_traits::difference_type difference_type;
-    typedef typename __alloc_traits::pointer         pointer;
-    typedef typename __alloc_traits::const_pointer   const_pointer;
+    typedef _Tp                                       value_type;
+    typedef value_type&                               reference;
+    typedef const value_type&                         const_reference;
+    typedef typename __alloc_traits::difference_type  difference_type;
+    typedef typename __alloc_traits::pointer          pointer;
+    typedef typename __alloc_traits::const_pointer    const_pointer;
 
     static const difference_type __block_size;
 
     typedef typename __rebind_alloc_helper<__alloc_traits, pointer>::type __pointer_allocator;
     typedef allocator_traits<__pointer_allocator>        __map_traits;
     typedef typename __map_traits::pointer               __map_pointer;
     typedef typename __rebind_alloc_helper<__alloc_traits, const_pointer>::type __const_pointer_allocator;
     typedef typename allocator_traits<__const_pointer_allocator>::const_pointer __map_const_pointer;
public:
     // types:
 
     typedef _Tp value_type;
     typedef _Allocator allocator_type;
 
     static_assert((is_same<typename allocator_type::value_type, value_type>::value),
                   "Allocator::value_type must be same type as value_type");
 
-    typedef __deque_base<value_type, allocator_type>                 __base;
+    typedef __deque_base<value_type, allocator_type>       __base;
 
-    typedef typename __base::__alloc_traits                          __alloc_traits;
-    typedef typename __base::reference                               reference;
-    typedef typename __base::const_reference                         const_reference;
-    typedef typename __base::iterator                                iterator;
-    typedef typename __base::const_iterator                          const_iterator;
-    typedef typename __allocator_traits<allocator_type>::size_type   size_type;
-    typedef typename __base::difference_type                         difference_type;
+    typedef typename __base::__alloc_traits                __alloc_traits;
+    typedef typename __base::reference                     reference;
+    typedef typename __base::const_reference               const_reference;
+    typedef typename __base::iterator                      iterator;
+    typedef typename __base::const_iterator                const_iterator;
+    typedef typename __base::size_type                     size_type;
+    typedef typename __base::difference_type               difference_type;
 
-    typedef typename __base::pointer                                 pointer;
-    typedef typename __base::const_pointer                           const_pointer;
-    typedef _VSTD::reverse_iterator<iterator>                        reverse_iterator;
-    typedef _VSTD::reverse_iterator<const_iterator>                  const_reverse_iterator;
+    typedef typename __base::pointer                       pointer;
+    typedef typename __base::const_pointer                 const_pointer;
+    typedef _VSTD::reverse_iterator<iterator>              reverse_iterator;
+    typedef _VSTD::reverse_iterator<const_iterator>        const_reverse_iterator;
 
     using typename __base::__deque_range;
     using typename __base::__deque_block_range;
     using typename __base::_ConstructTransaction;
 
     // construct/copy/destroy:
     _LIBCPP_INLINE_VISIBILITY
     deque()
         _NOEXCEPT_(is_nothrow_default_constructible<allocator_type>::value)
         {}
     _LIBCPP_INLINE_VISIBILITY explicit deque(const allocator_type& __a) : __base(__a) {}
     explicit deque(size_type __n);
 #if _LIBCPP_STD_VER > 11
     explicit deque(size_type __n, const _Allocator& __a);
 #endif
     deque(size_type __n, const value_type& __v);
-    deque(size_type __n, const value_type& __v, const allocator_type& __a);
+
+    template <class _MaybeAllocator = _Allocator, class = __enable_if_t<__is_allocator<_MaybeAllocator>::value> >
+    deque(size_type __n, const value_type& __v, const allocator_type& __a) : __base(__a)
+    {
+        if (__n > 0)
+            __append(__n, __v);
+    }
+
     template <class _InputIter>
         deque(_InputIter __f, _InputIter __l,
               typename enable_if<__is_cpp17_input_iterator<_InputIter>::value>::type* = 0);
     template <class _InputIter>
         deque(_InputIter __f, _InputIter __l, const allocator_type& __a,
               typename enable_if<__is_cpp17_input_iterator<_InputIter>::value>::type* = 0);
     deque(const deque& __c);
     deque(const deque& __c, const __identity_t<allocator_type>& __a);

 template <class _Tp, class _Allocator>
 deque<_Tp, _Allocator>::deque(size_type __n, const value_type& __v)
 {
     if (__n > 0)
         __append(__n, __v);
 }
 
 template <class _Tp, class _Allocator>
-deque<_Tp, _Allocator>::deque(size_type __n, const value_type& __v, const allocator_type& __a)
-    : __base(__a)
-{
-    if (__n > 0)
-        __append(__n, __v);
-}
-
-template <class _Tp, class _Allocator>
 template <class _InputIter>
 deque<_Tp, _Allocator>::deque(_InputIter __f, _InputIter __l,
               typename enable_if<__is_cpp17_input_iterator<_InputIter>::value>::type*)
 {
     __append(__f, __l);
 }
 
 template <class _Tp, class _Allocator>

libcxx/include/forward_list

 
 #include <__config>
 #include <__utility/forward.h>
 #include <algorithm>
 #include <initializer_list>
 #include <iterator>
 #include <limits>
 #include <memory>
+#include <type_traits>
 #include <version>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>

 
 public:
     typedef _Tp    value_type;
     typedef _Alloc allocator_type;
 
     static_assert((is_same<typename allocator_type::value_type, value_type>::value),
                   "Allocator::value_type must be same type as value_type");
 
-    typedef value_type&                                                reference;
-    typedef const value_type&                                          const_reference;
-    typedef typename allocator_traits<allocator_type>::pointer         pointer;
-    typedef typename allocator_traits<allocator_type>::const_pointer   const_pointer;
-    typedef typename __allocator_traits<allocator_type>::size_type     size_type;
-    typedef typename allocator_traits<allocator_type>::difference_type difference_type;
+    typedef value_type&                                                 reference;
+    typedef const value_type&                                           const_reference;
+    typedef typename allocator_traits<allocator_type>::pointer          pointer;
+    typedef typename allocator_traits<allocator_type>::const_pointer    const_pointer;
+    typedef typename allocator_traits<allocator_type>::size_type        size_type;
+    typedef typename allocator_traits<allocator_type>::difference_type  difference_type;
 
     typedef typename base::iterator       iterator;
     typedef typename base::const_iterator const_iterator;
 #if _LIBCPP_STD_VER > 17
     typedef size_type                                __remove_return_type;
 #else
     typedef void                                     __remove_return_type;
 #endif
 
     _LIBCPP_INLINE_VISIBILITY
     forward_list()
         _NOEXCEPT_(is_nothrow_default_constructible<__node_allocator>::value)
         {} // = default;
     _LIBCPP_INLINE_VISIBILITY
     explicit forward_list(const allocator_type& __a);
     explicit forward_list(size_type __n);
 #if _LIBCPP_STD_VER > 11
     explicit forward_list(size_type __n, const allocator_type& __a);
 #endif
     forward_list(size_type __n, const value_type& __v);
-    forward_list(size_type __n, const value_type& __v, const allocator_type& __a);
+    template <class _MaybeAllocator = _Alloc, class = __enable_if_t<__is_allocator<_MaybeAllocator>::value>>
+    forward_list(size_type __n, const value_type& __v, const allocator_type& __a) : base(__a)
+    {
+        insert_after(cbefore_begin(), __n, __v);
+    }
+
     template <class _InputIterator>
         forward_list(_InputIterator __f, _InputIterator __l,
                      typename enable_if<
                        __is_cpp17_input_iterator<_InputIterator>::value
                      >::type* = nullptr);
     template <class _InputIterator>
         forward_list(_InputIterator __f, _InputIterator __l,
                      const allocator_type& __a,

 
 template <class _Tp, class _Alloc>
 forward_list<_Tp, _Alloc>::forward_list(size_type __n, const value_type& __v)
 {
     insert_after(cbefore_begin(), __n, __v);
 }
 
 template <class _Tp, class _Alloc>
-forward_list<_Tp, _Alloc>::forward_list(size_type __n, const value_type& __v,
-                                        const allocator_type& __a)
-    : base(__a)
-{
-    insert_after(cbefore_begin(), __n, __v);
-}
-
-template <class _Tp, class _Alloc>
 template <class _InputIterator>
 forward_list<_Tp, _Alloc>::forward_list(_InputIterator __f, _InputIterator __l,
                                         typename enable_if<
                                           __is_cpp17_input_iterator<_InputIterator>::value
                                         >::type*)
 {
     insert_after(cbefore_begin(), __f, __l);
 }

libcxx/include/list

class _LIBCPP_TEMPLATE_VIS list
     typedef typename base::__node_allocator    __node_allocator;
     typedef typename base::__node_pointer      __node_pointer;
     typedef typename base::__node_alloc_traits __node_alloc_traits;
     typedef typename base::__node_base         __node_base;
     typedef typename base::__node_base_pointer __node_base_pointer;
     typedef typename base::__link_pointer __link_pointer;
 
 public:
-    typedef _Tp                                                      value_type;
-    typedef _Alloc                                                   allocator_type;
+    typedef _Tp                                            value_type;
+    typedef _Alloc                                         allocator_type;
     static_assert((is_same<value_type, typename allocator_type::value_type>::value),
                   "Invalid allocator::value_type");
-    typedef value_type&                                              reference;
-    typedef const value_type&                                        const_reference;
-    typedef typename base::pointer                                   pointer;
-    typedef typename base::const_pointer                             const_pointer;
-    typedef typename __allocator_traits<allocator_type>::size_type   size_type;
-    typedef typename base::difference_type                           difference_type;
-    typedef typename base::iterator                                  iterator;
-    typedef typename base::const_iterator                            const_iterator;
-    typedef _VSTD::reverse_iterator<iterator>                        reverse_iterator;
-    typedef _VSTD::reverse_iterator<const_iterator>                  const_reverse_iterator;
+    typedef value_type&                                    reference;
+    typedef const value_type&                              const_reference;
+    typedef typename base::pointer                         pointer;
+    typedef typename base::const_pointer                   const_pointer;
+    typedef typename base::size_type                       size_type;
+    typedef typename base::difference_type                 difference_type;
+    typedef typename base::iterator                        iterator;
+    typedef typename base::const_iterator                  const_iterator;
+    typedef _VSTD::reverse_iterator<iterator>              reverse_iterator;
+    typedef _VSTD::reverse_iterator<const_iterator>        const_reverse_iterator;
 #if _LIBCPP_STD_VER > 17
-    typedef size_type                                                __remove_return_type;
+    typedef size_type                                      __remove_return_type;
 #else
-    typedef void                                                     __remove_return_type;
+    typedef void                                           __remove_return_type;
 #endif
 
     _LIBCPP_INLINE_VISIBILITY
     list()
         _NOEXCEPT_(is_nothrow_default_constructible<__node_allocator>::value)
     {
 #if _LIBCPP_DEBUG_LEVEL == 2
         __get_db()->__insert_c(this);
 #endif
     }
     _LIBCPP_INLINE_VISIBILITY
     explicit list(const allocator_type& __a) : base(__a)
     {
 #if _LIBCPP_DEBUG_LEVEL == 2
         __get_db()->__insert_c(this);
 #endif
     }
     explicit list(size_type __n);
 #if _LIBCPP_STD_VER > 11
     explicit list(size_type __n, const allocator_type& __a);
 #endif
     list(size_type __n, const value_type& __x);
-    list(size_type __n, const value_type& __x, const allocator_type& __a);
+    template <class _MaybeAllocator = _Alloc, class = __enable_if_t<__is_allocator<_MaybeAllocator>::value> >
+    list(size_type __n, const value_type& __x, const allocator_type& __a) : base(__a)
+    {
+#if _LIBCPP_DEBUG_LEVEL == 2
+        __get_db()->__insert_c(this);
+#endif
+        for (; __n > 0; --__n)
+            push_back(__x);
+    }
+
     template <class _InpIter>
         list(_InpIter __f, _InpIter __l,
              typename enable_if<__is_cpp17_input_iterator<_InpIter>::value>::type* = 0);
     template <class _InpIter>
         list(_InpIter __f, _InpIter __l, const allocator_type& __a,
              typename enable_if<__is_cpp17_input_iterator<_InpIter>::value>::type* = 0);
 
     list(const list& __c);

 #if _LIBCPP_DEBUG_LEVEL == 2
     __get_db()->__insert_c(this);
 #endif
     for (; __n > 0; --__n)
         push_back(__x);
 }
 
 template <class _Tp, class _Alloc>
-list<_Tp, _Alloc>::list(size_type __n, const value_type& __x, const allocator_type& __a)
-    : base(__a)
-{
-#if _LIBCPP_DEBUG_LEVEL == 2
-    __get_db()->__insert_c(this);
-#endif
-    for (; __n > 0; --__n)
-        push_back(__x);
-}
-
-template <class _Tp, class _Alloc>
 template <class _InpIter>
 list<_Tp, _Alloc>::list(_InpIter __f, _InpIter __l,
                         typename enable_if<__is_cpp17_input_iterator<_InpIter>::value>::type*)
 {
 #if _LIBCPP_DEBUG_LEVEL == 2
     __get_db()->__insert_c(this);
 #endif
     for (; __f != __l; ++__f)

libcxx/include/string

 #else
         _NOEXCEPT;
 #endif
 
     _LIBCPP_INLINE_VISIBILITY
     basic_string(basic_string&& __str, const allocator_type& __a);
 #endif // _LIBCPP_CXX03_LANG
 
-    template <class = __enable_if_t<__is_allocator<_Allocator>::value, nullptr_t> >
+    template <class _MaybeAllocator = _Allocator, class = __enable_if_t<__is_allocator<_MaybeAllocator>::value, nullptr_t> >
     _LIBCPP_INLINE_VISIBILITY
     basic_string(const _CharT* __s) : __r_(__default_init_tag(), __default_init_tag()) {
       _LIBCPP_ASSERT(__s != nullptr, "basic_string(const char*) detected nullptr");
       __init(__s, traits_type::length(__s));
 #   if _LIBCPP_DEBUG_LEVEL == 2
       __get_db()->__insert_c(this);
 #   endif
     }
 
-    template <class = __enable_if_t<__is_allocator<_Allocator>::value, nullptr_t> >
+    template <class _MaybeAllocator = _Allocator, class = __enable_if_t<__is_allocator<_MaybeAllocator>::value, nullptr_t> >
         _LIBCPP_INLINE_VISIBILITY
-        basic_string(const _CharT* __s, const _Allocator& __a);
+        basic_string(const _CharT* __s, const _Allocator& __a) : __r_(__default_init_tag(), __a) {
+      _LIBCPP_ASSERT(__s != nullptr, "basic_string(const char*, allocator) detected nullptr");
+      __init(__s, traits_type::length(__s));
+#if _LIBCPP_DEBUG_LEVEL == 2
+      __get_db()->__insert_c(this);
+#endif
+    }
 
 #if _LIBCPP_STD_VER > 20
     basic_string(nullptr_t) = delete;
 #endif
 
     _LIBCPP_INLINE_VISIBILITY
     basic_string(const _CharT* __s, size_type __n);
     _LIBCPP_INLINE_VISIBILITY
     basic_string(const _CharT* __s, size_type __n, const _Allocator& __a);
     _LIBCPP_INLINE_VISIBILITY
     basic_string(size_type __n, _CharT __c);
 
-    template <class = __enable_if_t<__is_allocator<_Allocator>::value, nullptr_t> >
+    template <class _MaybeAllocator = _Allocator, class = __enable_if_t<__is_allocator<_MaybeAllocator>::value, nullptr_t> >
         _LIBCPP_INLINE_VISIBILITY
-        basic_string(size_type __n, _CharT __c, const _Allocator& __a);
+        basic_string(size_type __n, _CharT __c, const _Allocator& __a) : __r_(__default_init_tag(), __a) {
+      __init(__n, __c);
+#if _LIBCPP_DEBUG_LEVEL == 2
+      __get_db()->__insert_c(this);
+#endif
+    }
 
     basic_string(const basic_string& __str, size_type __pos, size_type __n,
                  const _Allocator& __a = _Allocator());
     _LIBCPP_INLINE_VISIBILITY
     basic_string(const basic_string& __str, size_type __pos,
                  const _Allocator& __a = _Allocator());
 
     template<class _Tp, class = __enable_if_t<__can_be_converted_to_string_view<_CharT, _Traits, _Tp>::value && !__is_same_uncvref<_Tp, basic_string>::value> >
else
         __set_long_cap(__cap+1);
         __set_long_size(__sz);
     }
     traits_type::copy(_VSTD::__to_address(__p), __s, __sz);
     traits_type::assign(__p[__sz], value_type());
 }
 
 template <class _CharT, class _Traits, class _Allocator>
-template <class>
-basic_string<_CharT, _Traits, _Allocator>::basic_string(const _CharT* __s, const _Allocator& __a)
-    : __r_(__default_init_tag(), __a)
-{
-    _LIBCPP_ASSERT(__s != nullptr, "basic_string(const char*, allocator) detected nullptr");
-    __init(__s, traits_type::length(__s));
-#if _LIBCPP_DEBUG_LEVEL == 2
-    __get_db()->__insert_c(this);
-#endif
-}
-
-template <class _CharT, class _Traits, class _Allocator>
 inline
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const _CharT* __s, size_type __n)
      : __r_(__default_init_tag(), __default_init_tag())
 {
     _LIBCPP_ASSERT(__n == 0 || __s != nullptr, "basic_string(const char*, n) detected nullptr");
     __init(__s, __n);
 #if _LIBCPP_DEBUG_LEVEL == 2
     __get_db()->__insert_c(this);

 {
     __init(__n, __c);
 #if _LIBCPP_DEBUG_LEVEL == 2
     __get_db()->__insert_c(this);
 #endif
 }
 
 template <class _CharT, class _Traits, class _Allocator>
-template <class>
-basic_string<_CharT, _Traits, _Allocator>::basic_string(size_type __n, _CharT __c, const _Allocator& __a)
-    : __r_(__default_init_tag(), __a)
-{
-    __init(__n, __c);
-#if _LIBCPP_DEBUG_LEVEL == 2
-    __get_db()->__insert_c(this);
-#endif
-}
-
-template <class _CharT, class _Traits, class _Allocator>
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const basic_string& __str,
                                                         size_type __pos, size_type __n,
                                                         const _Allocator& __a)
     : __r_(__default_init_tag(), __a)
 {
     size_type __str_sz = __str.size();
     if (__pos > __str_sz)
         this->__throw_out_of_range();

libcxx/include/vector

 };
 
 template <class _Tp, class _Allocator /* = allocator<_Tp> */>
 class _LIBCPP_TEMPLATE_VIS vector
     // This base class is historical, but it needs to remain for ABI compatibility.
     : private __vector_base<_Tp, _Allocator>
 {
 private:
-    typedef __vector_base<_Tp, _Allocator>                           __base;
-    typedef allocator<_Tp>                                           __default_allocator_type;
+    typedef __vector_base<_Tp, _Allocator>                  __base;
+    typedef allocator<_Tp>                                  __default_allocator_type;
 public:
-    typedef vector                                                   __self;
-    typedef _Tp                                                      value_type;
-    typedef _Allocator                                               allocator_type;
-    typedef allocator_traits<allocator_type>                         __alloc_traits;
-    typedef value_type&                                              reference;
-    typedef const value_type&                                        const_reference;
-    typedef typename __allocator_traits<allocator_type>::size_type   size_type;
-    typedef typename __alloc_traits::difference_type                 difference_type;
-    typedef typename __alloc_traits::pointer                         pointer;
-    typedef typename __alloc_traits::const_pointer                   const_pointer;
-    typedef __wrap_iter<pointer>                                     iterator;
-    typedef __wrap_iter<const_pointer>                               const_iterator;
-    typedef _VSTD::reverse_iterator<iterator>                        reverse_iterator;
-    typedef _VSTD::reverse_iterator<const_iterator>                  const_reverse_iterator;
+    typedef vector                                          __self;
+    typedef _Tp                                             value_type;
+    typedef _Allocator                                      allocator_type;
+    typedef allocator_traits<allocator_type>                __alloc_traits;
+    typedef value_type&                                     reference;
+    typedef const value_type&                               const_reference;
+    typedef typename __alloc_traits::size_type              size_type;
+    typedef typename __alloc_traits::difference_type        difference_type;
+    typedef typename __alloc_traits::pointer                pointer;
+    typedef typename __alloc_traits::const_pointer          const_pointer;
+    typedef __wrap_iter<pointer>                            iterator;
+    typedef __wrap_iter<const_pointer>                      const_iterator;
+    typedef _VSTD::reverse_iterator<iterator>               reverse_iterator;
+    typedef _VSTD::reverse_iterator<const_iterator>         const_reverse_iterator;
 
     static_assert((is_same<typename allocator_type::value_type, value_type>::value),
                   "Allocator::value_type must be same type as value_type");
 
     _LIBCPP_INLINE_VISIBILITY
     vector() _NOEXCEPT_(is_nothrow_default_constructible<allocator_type>::value)
         {
 #if _LIBCPP_DEBUG_LEVEL == 2
#if _LIBCPP_DEBUG_LEVEL == 2
         __get_db()->__insert_c(this);
 #endif
     }
     explicit vector(size_type __n);
 #if _LIBCPP_STD_VER > 11
     explicit vector(size_type __n, const allocator_type& __a);
 #endif
     vector(size_type __n, const value_type& __x);
-    vector(size_type __n, const value_type& __x, const allocator_type& __a);
+
+    template <class _MaybeAllocator = _Allocator, class = __enable_if_t<__is_allocator<_MaybeAllocator>::value> >
+    vector(size_type __n, const value_type& __x, const allocator_type& __a)
+        : __base(__a)
+    {
+#if _LIBCPP_DEBUG_LEVEL == 2
+      __get_db()->__insert_c(this);
+#endif
+      if (__n > 0)
+      {
+          __vallocate(__n);
+          __construct_at_end(__n, __x);
+      }
+    }
+
     template <class _InputIterator>
         vector(_InputIterator __first,
                typename enable_if<__is_cpp17_input_iterator  <_InputIterator>::value &&
                                  !__is_cpp17_forward_iterator<_InputIterator>::value &&
                                  is_constructible<
                                     value_type,
                                     typename iterator_traits<_InputIterator>::reference>::value,
                                  _InputIterator>::type __last);
#endif
     if (__n > 0)
     {
         __vallocate(__n);
         __construct_at_end(__n, __x);
     }
 }
 
 template <class _Tp, class _Allocator>
-vector<_Tp, _Allocator>::vector(size_type __n, const value_type& __x, const allocator_type& __a)
-    : __base(__a)
-{
-#if _LIBCPP_DEBUG_LEVEL == 2
-    __get_db()->__insert_c(this);
-#endif
-    if (__n > 0)
-    {
-        __vallocate(__n);
-        __construct_at_end(__n, __x);
-    }
-}
-
-template <class _Tp, class _Allocator>
 template <class _InputIterator>
 vector<_Tp, _Allocator>::vector(_InputIterator __first,
        typename enable_if<__is_cpp17_input_iterator  <_InputIterator>::value &&
                          !__is_cpp17_forward_iterator<_InputIterator>::value &&
                          is_constructible<
                             value_type,
                             typename iterator_traits<_InputIterator>::reference>::value,
                           _InputIterator>::type __last)