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

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

Open
MathiasMagnus opened this issue Nov 13, 2024 · 3 comments
Labels
LEWG issue needed A design defect that should be submitted to LEWG as a new issue

Comments

@MathiasMagnus
Copy link

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
Copy link
Contributor

Arguably a duplicate of #1036.

It's unfortunate that

  • unordered_map's move constructor is not noexcept due to several reasons, and
  • vector can't reliably know that unordered_map<int, move_only_type> is move-only,

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

@CaseyCarter CaseyCarter added the decision needed We need to choose something before working on this label Nov 13, 2024
@CaseyCarter
Copy link
Member

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 decision needed We need to choose something before working on this for now and we'll see if other maintainers can talk me down during the weekly sync meeting.

@StephanTLavavej StephanTLavavej added LEWG issue needed A design defect that should be submitted to LEWG as a new issue and removed decision needed We need to choose something before working on this labels Nov 13, 2024
@StephanTLavavej
Copy link
Member

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:

  • Performance would be enhanced when moves are selected instead of downgraded copies.
  • Wacky compiler errors (as in this case) would be avoided, because we wouldn't attempt to downgrade to copying for types with throwing moves that aren't actually copyable (because Standard container copy/moves are unconstrained).
  • We would lose the strong EH guarantee (provided since C++98) when elements are throwing-moved but a copy-downgrade would have been possible.
    • @StephanTLavavej (who overhauled vector to achieve strong EH on every conceivable codepath where it's theoretically required) is hugely against the strong EH guarantee for container insertions, which he has never seen relied upon in production, and is especially negative on providing this guarantee for anything beyond push_back/emplace_back, so he thinks there's no real loss here.

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.

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

No branches or pull requests

4 participants