microsoft / STL

MSVC's implementation of the C++ Standard Library.
Other
10.2k stars 1.51k forks source link

<vector>: Cannot `resize`/`reserve` an instance of `std::vector<std::unordered_map<int, move_only_type>>` #5084

Open MathiasMagnus opened 1 day ago

MathiasMagnus commented 1 day ago

Describe the bug

vector and unordered_map seem to interact in an unexpected way. I can resize a vector of move-only type, I can emplace into an unordered_map of move-only mapped_type, but MS-STL won't allow me to resize a vector of unordered_map of move-only mapped_type.

Command-line test case

PS C:\Users\mate\Desktop> gc .\repro.cpp
#include <unordered_map>
#include <memory>
#include <vector>

int main()
{
    std::unordered_map<int, std::unique_ptr<int>> a;
    a.emplace(0, nullptr); // OK

    std::vector<std::unique_ptr<int>> b;
    b.resize(1); // OK

    std::vector<std::unordered_map<int, std::unique_ptr<int>>> c;
    //c.resize(1); // ERROR
    c.reserve(1); // ERROR

    return (int)(a.size() + b.size() + c.size());
}

PS C:\Users\mate\Desktop> cl /O2 /EHsc /std:c++17 /permissive- /W4 .\repro.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.41.34120 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

repro.cpp
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\xmemory(700): error C2280: 'std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>::pair(const std::pair<const int,std::unique_ptr<int,std::default_delete<int>>> &)': attempting to reference a deleted function
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\utility(250): note: see declaration of 'std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>::pair'
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\utility(250): note: 'std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>::pair(const std::pair<const int,std::unique_ptr<int,std::default_delete<int>>> &)': function was implicitly deleted because a data member invokes a deleted or inaccessible function 'std::unique_ptr<int,std::default_delete<int>>::unique_ptr(const std::unique_ptr<int,std::default_delete<int>> &)'
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\memory(3451): note: 'std::unique_ptr<int,std::default_delete<int>>::unique_ptr(const std::unique_ptr<int,std::default_delete<int>> &)': function was explicitly deleted
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\xmemory(700): note: the template instantiation context (the oldest one first) is
.\repro.cpp(7): note: see reference to class template instantiation 'std::unordered_map<int,std::unique_ptr<int,std::default_delete<int>>,std::hash<int>,std::equal_to<int>,std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>' being compiled
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\unordered_map(105): note: while compiling class template member function 'std::unordered_map<int,std::unique_ptr<int,std::default_delete<int>>,std::hash<int>,std::equal_to<int>,std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>::unordered_map(const std::unordered_map<int,std::unique_ptr<int,std::default_delete<int>>,std::hash<int>,std::equal_to<int>,std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>> &)'
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\xmemory(700): note: see the first reference to 'std::unordered_map<int,std::unique_ptr<int,std::default_delete<int>>,std::hash<int>,std::equal_to<int>,std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>::unordered_map' in 'std::_Default_allocator_traits<_Alloc>::construct'
        with
        [
            _Alloc=std::allocator<std::unordered_map<int,std::unique_ptr<int,std::default_delete<int>>,std::hash<int>,std::equal_to<int>,std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>>
        ]
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\xmemory(1779): note: see the first reference to 'std::_Default_allocator_traits<_Alloc>::construct' in 'std::_Uninitialized_backout_al<std::allocator<std::unordered_map<int,std::unique_ptr<int,std::default_delete<int>>,std::hash<int>,std::equal_to<int>,std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>>>::_Emplace_back'
        with
        [
            _Alloc=std::allocator<std::unordered_map<int,std::unique_ptr<int,std::default_delete<int>>,std::hash<int>,std::equal_to<int>,std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>>
        ]
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\xmemory(1833): note: see the first reference to 'std::_Uninitialized_backout_al<std::allocator<std::unordered_map<int,std::unique_ptr<int,std::default_delete<int>>,std::hash<int>,std::equal_to<int>,std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>>>::_Emplace_back' in 'std::_Uninitialized_copy'
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\unordered_map(106): note: see reference to function template instantiation 'std::_Hash<std::_Umap_traits<_Kty,_Ty,std::_Uhash_compare<_Kty,_Hasher,_Keyeq>,_Alloc,false>>::_Hash<std::allocator<std::_List_node<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>,std::_Default_allocator_traits<_Alloc>::void_pointer>>>(const std::_Hash<std::_Umap_traits<_Kty,_Ty,std::_Uhash_compare<_Kty,_Hasher,_Keyeq>,_Alloc,false>> &,const _Any_alloc &)' being compiled
        with
        [
            _Kty=int,
            _Ty=std::unique_ptr<int,std::default_delete<int>>,
            _Hasher=std::hash<int>,
            _Keyeq=std::equal_to<int>,
            _Alloc=std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>,
            _Any_alloc=std::allocator<std::_List_node<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>,std::_Default_allocator_traits<std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>::void_pointer>>
        ]
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\xhash(388): note: see reference to function template instantiation 'void std::_Hash<std::_Umap_traits<_Kty,_Ty,std::_Uhash_compare<_Kty,_Hasher,_Keyeq>,_Alloc,false>>::_Insert_range_unchecked<std::_List_unchecked_const_iterator<std::_List_val<std::_List_simple_types<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>,std::_Iterator_base0>,std::_List_unchecked_const_iterator<std::_List_val<std::_List_simple_types<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>,std::_Iterator_base0>>(_Iter,const _Sent)' being compiled
        with
        [
            _Kty=int,
            _Ty=std::unique_ptr<int,std::default_delete<int>>,
            _Hasher=std::hash<int>,
            _Keyeq=std::equal_to<int>,
            _Alloc=std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>,
            _Iter=std::_List_unchecked_const_iterator<std::_List_val<std::_List_simple_types<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>,std::_Iterator_base0>,
            _Sent=std::_List_unchecked_const_iterator<std::_List_val<std::_List_simple_types<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>,std::_Iterator_base0>
        ]
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\xhash(950): note: see reference to function template instantiation 'std::pair<std::_List_iterator<std::_List_val<std::_List_simple_types<_Ty>>>,bool> std::_Hash<std::_Umap_traits<_Kty,std::unique_ptr<int,std::default_delete<int>>,std::_Uhash_compare<_Kty,_Hasher,_Keyeq>,_Alloc,false>>::emplace<const std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>&>(const std::pair<const int,std::unique_ptr<int,std::default_delete<int>>> &)' being compiled
        with
        [
            _Ty=std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>,
            _Kty=int,
            _Hasher=std::hash<int>,
            _Keyeq=std::equal_to<int>,
            _Alloc=std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>
        ]
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\xhash(610): note: see reference to function template instantiation 'std::_List_node_emplace_op2<std::allocator<std::_List_node<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>,std::_Default_allocator_traits<_Alloc>::void_pointer>>>::_List_node_emplace_op2<const std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>&>(_Alnode &,const std::pair<const int,std::unique_ptr<int,std::default_delete<int>>> &)' being compiled
        with
        [
            _Alloc=std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>,
            _Alnode=std::allocator<std::_List_node<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>,std::_Default_allocator_traits<std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>::void_pointer>>
        ]
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\include\list(585): note: see reference to function template instantiation 'void std::_Default_allocator_traits<_Alloc>::construct<_Ty,const std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>&>(_Alloc &,_Objty *const ,const std::pair<const int,std::unique_ptr<int,std::default_delete<int>>> &)' being compiled
        with
        [
            _Alloc=std::allocator<std::_List_node<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>,std::_Default_allocator_traits<std::allocator<std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>>>::void_pointer>>,
            _Ty=std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>,
            _Objty=std::pair<const int,std::unique_ptr<int,std::default_delete<int>>>
        ]

Expected behavior

Have the code compile, like libstdc++ allows this.

Versions

Visual Studio

PS C:\Users\mate\Desktop> Get-CimInstance MSFT_VSInstance -Namespace root/cimv2/vs | `
>> where Caption -match 'Build Tools 2022' | `
>> select -exp Version
17.11.35312.102

MSVC

Microsoft (R) C/C++ Optimizing Compiler Version 19.41.34120 for x64

STL

#define _CPPLIB_VER       650
#define _MSVC_STL_VERSION 143
#define _MSVC_STL_UPDATE  202405L

Additional context

Obligatory Godbolt

frederick-vs-ja commented 1 day ago

Arguably a duplicate of #1036.

It's unfortunate that

so copy construction is attempted, which cause hard error (seems to be UB in standard wording).

CaseyCarter commented 18 hours ago

Given the existence of types whose copyability is indeterminate - in the Standard Library itself! - I think the requirement in [vector.modifiers]/2 that:

If an exception is thrown while inserting a single element at the end and T is Cpp17CopyInsertable or is_nothrow_move_constructible_v<T> is true, there are no effects. Otherwise, if an exception is thrown by the move constructor of a non-Cpp17CopyInsertable T, the effects are unspecified.

is unimplementable. I'd greatly prefer that we ignore this requirement by having vector reallocation always move instead of copy and submit an LWG issue to get the requirement fixed. Sometimes moving when we "should" copy is a much smaller bug than vector being broken by copyability-indeterminate types, and nobody is going to be surprised that vector moves instead of copying in 2025: C++ got move semantics 14 years ago.

I'll mark this https://github.com/microsoft/STL/labels/decision%20needed for now and we'll see if other maintainers can talk me down during the weekly sync meeting.

StephanTLavavej commented 11 hours ago

We talked about this at the weekly maintainer meeting and we believe this merits a Library Evolution issue. We would like to see @CaseyCarter's suggestion adopted - vector should just always move, and not worry about achieving the strong EH guarantee through move-if-noexcept logic. The effects of this would be, for our implementation:

For other implementations (where their container-internal sentinel nodes don't result in throwing moves), this issue still theoretically impacts them, but a user has to bring them a type with a throwing move ctor (which can be fairly easily obtained - just forget to mention noexcept on your move!!). In that case all of the same considerations apply, as our implementation is unique only about the do-containers-have-throwing-moves-due-to-dynamically-allocated-sentinel-nodes issue - our vector's behavior in response to whether an element has a throwing move is 100% Standard.