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

Default non-throw move ctor with std::unordered_map member is not move constructible #165

Closed
cloudhan opened this issue Oct 7, 2019 · 5 comments
Labels
invalid This issue is incorrect or by design

Comments

@cloudhan
Copy link
Member

cloudhan commented Oct 7, 2019

Command-line test case
STL version (git commit or Visual Studio version):

> cl
Microsoft (R) C/C++ Optimizing Compiler Version 19.23.28105.4 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

usage: cl [ option... ] filename... [ /link linkoption... ]

Minimal reproducible test case:

#include <type_traits>
#include <unordered_map>
// #include <absl/container/flat_hash_map.h>

class MyStruct {
public:
    MyStruct() {}

    MyStruct(const MyStruct&) = delete;
    MyStruct(MyStruct&&) noexcept = default;
    MyStruct& operator=(const MyStruct&) = delete;
    MyStruct& operator=(MyStruct&&) = delete;

private:
    std::unordered_map<int, int> map_;
    // absl::flat_hash_map<int, int> map_;
};


int main() {
    static_assert(std::is_constructible<MyStruct>::value, "static_assert is_constructible");
    static_assert(std::is_nothrow_constructible<MyStruct>::value, "static_assert is_nothrow_constructible");
    static_assert(std::is_move_constructible<MyStruct>::value, "static_assert is_move_constructible");
    static_assert(std::is_nothrow_move_constructible<MyStruct>::value, "static_assert is_nothrow_move_constructible");
    static_assert(std::is_move_assignable<MyStruct>::value, "static_assert is_move_assignable");

    return 0;
}

compilation command:

> cl /IC:\tools\vcpkg\installed\x64-windows\include\ /std:c++17 /EHs .\noexcept.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.23.28105.4 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

noexcept.cpp
.\noexcept.cpp(22): error C2338: static_assert is_nothrow_constructible
.\noexcept.cpp(23): error C2338: static_assert is_move_constructible
.\noexcept.cpp(24): error C2338: static_assert is_nothrow_move_constructible
.\noexcept.cpp(25): error C2338: static_assert is_move_assignable

Expected behavior
In the above case, std::is_move_constructible and std::is_nothrow_move_constructible should be true, aka, only two static_assert fail, line 22, line 25.

Additional context

  1. with abseil's flat_hash_map, it is indeed move constructible and nothrow move constructible.
  2. with MyStruct(MyStruct&&) = default;, it is indeed move constructible but NOT nothrow move constructible.
@cloudhan
Copy link
Member Author

cloudhan commented Oct 7, 2019

FYI, why absl::flat_hash_map is used? Because this code snippet is reduce from a complex form of library code, and abseil lib is included.

It compiles with GCC but not MSVC. MSVC keeps complaining copy ctor is deleted when you have something like std::move(my_struct_object) where my_struct_object is of type MyStruct. The error message is quite misleading. It should at least directly point to the line MyStruct(MyStruct&&) noexcept = default;.

@cloudhan
Copy link
Member Author

cloudhan commented Oct 7, 2019

@CaseyCarter CaseyCarter added the invalid This issue is incorrect or by design label Oct 7, 2019
@CaseyCarter
Copy link
Member

Expected behavior
In the above case, std::is_move_constructible and std::is_nothrow_move_constructible should be true

That is not Standard-mandated behavior. From the class template synopsis in [unord.map.overview], neither the default constructor nor move constructor is depicted as having a noexcept-specifier. Notably our implementation uses separately-allocated sentinel nodes which necessitates an allocation in both the default constructor and move constructor, so they cannot be noexcept.

@cloudhan
Copy link
Member Author

cloudhan commented Oct 8, 2019

OK, after reading the C++17 standard.

dcl.fct.def.default

3 If a function that is explicitly defaulted is declared with a noexcept-specifier that does not produce the same exception specification as the implicit declaration ([except.spec]), then
(3.1) if the function is explicitly defaulted on its first declaration, it is defined as deleted;
(3.2) otherwise, the program is ill-formed.
So it is indeed expected behavior.

Still hope you guys can improve the error message.

@CaseyCarter
Copy link
Member

There isn't much of anything the STL can do about diagnostics that compilers choose to emit for code unrelated to the STL. I suggest that you file a suggestion at Developer Community about improving diagnostics for this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This issue is incorrect or by design
Projects
None yet
Development

No branches or pull requests

2 participants