Containers of move-only flat_maps are broken |
||||
Issue description
I ran into compiler errors trying to compile something like:
base::flat_map<int, base::flat_map<int, std::unique_ptr<Foo>>> map;
map.emplace(42, {});
The exact failure mode can be reduced a bit further, to:
std::vector<base::flat_map<int, std::unique_ptr<Foo>>> v;
v.emplace_back();
emplacement ultimately attempts to invoke the deleted copy constructor on std::pair<int, std::unique_ptr<Foo>>.
This is surprising to me because insertion into a std::vector<std::pair<int, std::unique_ptr<Foo>>> compiles just fine.
Furthermore:
- A flat_map is essentially a flat_tree<std::pair<K, V>>
- flat_tree uses the default move constructor
- flat_tree's only contents are an instance of its Impl type, which
in this case is just a wrapped std::vector<std::pair<K, V>>
After staring at this for an hour I think I'm better off deferring to someone else here :)
,
Apr 9 2018
Oh, it looks like this is because vector (et al) use std::__uninitialized_move_if_noexcept_a and the move constructor is not noexcept. Is there a reason we can't make it noexcept? Or alternatively, why the inline-default move constructor doesn't trip over this but the out-of-line one does?
,
Apr 10 2018
,
Apr 10 2018
,
Apr 10 2018
We should probably use noexcept in more places. However, brettw previously reported a toolchain bug in the https://groups.google.com/a/chromium.org/forum/#!topic/cxx/ze4WJFg7RvU: apparently noexcept and = default don't play well on Android in certain cases. I'll test if this bug with noexcept and = default ( issue 706963 ) is still the case on the Android toolchain. That might be a blocker for doing this.
,
Apr 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aa8021c3ea0a183348e70f6d164c87f70054ee69 commit aa8021c3ea0a183348e70f6d164c87f70054ee69 Author: Daniel Cheng <dcheng@chromium.org> Date: Sat Apr 21 22:31:41 2018 Mark flat_tree's move constructor as noexcept. Bug: 830117 Change-Id: Icd3cc6959a256a1cc1691c51fdc95f1964af9fb9 Reviewed-on: https://chromium-review.googlesource.com/1010025 Reviewed-by: Ken Rockot <rockot@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#552594} [modify] https://crrev.com/aa8021c3ea0a183348e70f6d164c87f70054ee69/base/containers/flat_map.h [modify] https://crrev.com/aa8021c3ea0a183348e70f6d164c87f70054ee69/base/containers/flat_tree.h
,
Apr 21 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by roc...@chromium.org
, Apr 9 2018So I have a significantly reduced repro case now. The code below does not compile. If the move constructor is inline, or the copy constructor is deleted, it compiles. thakis@ can you confirm whether or not this looks like a clang bug? And is it reasonable to inline the flat_tree move constructor as a work-around? // ----- (snip) #include <memory> #include <vector> class Test { public: Test() = default; Test(const Test&) = default; Test(Test&&); std::vector<std::unique_ptr<int>> storage; }; Test::Test(Test&&) = default; int main() { std::vector<Test> v; v.emplace_back(); return 0; }