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

The CTAD of the standard container is still instantiated when Alloc does not satisfy Allocator. #4060

Open
hewillk opened this issue Sep 29, 2023 · 4 comments
Labels
LWG issue needed A wording defect that should be submitted to LWG as a new issue

Comments

@hewillk
Copy link
Contributor

hewillk commented Sep 29, 2023

Not sure if this is a language bug or a library bug:

https://godbolt.org/z/EY6K9eKP9

#include <vector>
#include <list>

template<template <class...> class Cnt, class Alloc>
concept can_ctad = requires (int* it) {
  Cnt(it, it, Alloc{});
};

struct NotAlloc { };

static_assert(!can_ctad<std::vector, NotAlloc>); // hard error in MSVC-STL
static_assert(!can_ctad<std::list, NotAlloc>); // hard error in MSVC-STL
@CaseyCarter CaseyCarter added the question Further information is requested label Sep 29, 2023
@CaseyCarter
Copy link
Contributor

The errors are from the (size_type, const T&, const Alloc&) constructors:

list(_CRT_GUARDOVERFLOW size_type _Count, const _Ty& _Val, const _Alloc& _Al)

and

_CONSTEXPR20 vector(_CRT_GUARDOVERFLOW const size_type _Count, const _Ty& _Val, const _Alloc& _Al = _Alloc())

These definitions (list is similar):

STL/stl/inc/vector

Lines 442 to 458 in 4751057

using _Alty = _Rebind_alloc_t<_Alloc, _Ty>;
using _Alty_traits = allocator_traits<_Alty>;
public:
static_assert(!_ENFORCE_MATCHING_ALLOCATORS || is_same_v<_Ty, typename _Alloc::value_type>,
_MISMATCHED_ALLOCATOR_MESSAGE("vector<T, Allocator>", "T"));
static_assert(is_object_v<_Ty>, "The C++ Standard forbids containers of non-object types "
"because of [container.requirements].");
using value_type = _Ty;
using allocator_type = _Alloc;
using pointer = typename _Alty_traits::pointer;
using const_pointer = typename _Alty_traits::const_pointer;
using reference = _Ty&;
using const_reference = const _Ty&;
using size_type = typename _Alty_traits::size_type;
using difference_type = typename _Alty_traits::difference_type;

encourage the compiler to deduce size_type from allocator_traits<typename allocator_traits<_Alloc>::rebind_alloc<_Ty>>::size_type, the formation of which is problematic for a non-allocator _Alloc. This is conforming behavior as far as I can see.

Do we want to block deduction through all _Alloc constructor parameters, or do we want to file an LWG issue to have the standard mandate which deductions should work and which should not? It seems to me that users would be happier with a guarantee of portability than with asking implementations to please agree with other implementations.

@CaseyCarter CaseyCarter added the decision needed We need to choose something before working on this label Sep 29, 2023
@frederick-vs-ja
Copy link
Contributor

In libc++, the constructors are constrained to make CTAD SFINAE-friendly (LLVM-D114311).

In libstdc++, size_type of vector and list are always defined as size_t, so the allocator type is not inspected.

It seems better to me to file an LWG issue to standardize libc++'s strategy. On the other hand, we may make allocator_traits SFINAE-friendly on value_type, i.e. make allocator_traits<Alloc> have no declared members when Alloc::value_type doesn't exist.

@StephanTLavavej StephanTLavavej added LWG issue needed A wording defect that should be submitted to LWG as a new issue and removed question Further information is requested decision needed We need to choose something before working on this labels Oct 4, 2023
@StephanTLavavej
Copy link
Member

We talked about this at the weekly maintainer meeting and agree that an LWG issue is needed for this. We believe that the right strategy will involve either constraining all affected constructors, or adding global wording that affects container constructors (similar to the global wording that already exists for ordered/unordered deduction guides, N4958 [associative.reqmts.general]/181 and [unord.req.general]/248). Due to previous LWG issues, a couple of basic_string constructors were patched up, see [string.cons]/18.

Finally, we already have targeted changes in at least one list constructor. Here, we blocked deduction through the Allocator parameter, instead of constraining it (presumably because this is a "copy with alloc" constructor, where the const list& argument is already providing type information for the Allocator). We didn't do enough research to look into the history of when and why we made this change; that should be done before filing any LWG issue here:

list(const list& _Right, const _Identity_t<_Alloc>& _Al) : _Mypair(_One_then_variadic_args_t{}, _Al) {

@frederick-vs-ja
Copy link
Contributor

We didn't do enough research to look into the history of when and why we made this change; that should be done before filing any LWG issue here:

The change was done in #2032 (WG21-P1518R2). The paper was technically a change in C++23, but we treated it as a DR and applied it to old modes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LWG issue needed A wording defect that should be submitted to LWG as a new issue
Projects
None yet
Development

No branches or pull requests

4 participants