Home » U++ Library support » U++ Core » Problem with Vector::Add (pick/clone semantics)
Problem with Vector::Add (pick/clone semantics) [message #52213] |
Fri, 09 August 2019 12:14 ![Go to next message Go to next message](theme/default/images/down.png) |
shutalker
Messages: 15 Registered: November 2017 Location: Moscow
|
Promising Member |
|
|
Hi all!
I've encountered with the following problem. When I try to define such object as
const static VectorMap<String, Vector<String>> MY_MAP = {
{"s1", pick(Vector<String>{"s11", "s12", "s13", "s14"})},
{"s2", pick(Vector<String>{"s21", "s22", "s23", "s24"})},
{"s3", pick(Vector<String>{"s31", "s32", "s33", "s34"})},
{"s4", pick(Vector<String>{"s41", "s42", "s43", "s44"})},
{"s5", pick(Vector<String>{"s51", "s52", "s53", "s54"})}
};
I get several errors like this
/home/alexis/upp/uppsrc/Core/Vcont.hpp (158): error: call to implicitly-deleted copy constructor of 'Upp::Vector<Upp::String>'
...
/upp/uppsrc/Core/Core.h (357): In file included from /home/alexis/upp/uppsrc/Core/Core.h:357:
(): T *q = new(Rdd()) T(x);
/home/alexis/upp/uppsrc/Core/Vcont.h (132): note: in instantiation of member function 'Upp::Vector<Upp::Vector<Upp::String> >::GrowAdd' requested here
(): T& Add(const T& x) { return items < alloc ? *(new(Rdd()) T(clone(x))) : GrowAdd(x); }
/home/alexis/upp/uppsrc/Core/Map.h (51): note: in instantiation of member function 'Upp::Vector<Upp::Vector<Upp::String> >::Add' requested here
(): T& Add(const K& k, const T& x) { key.Add(k); return value.Add(x); }
/home/alexis/upp/uppsrc/Core/Map.h (179): note: in instantiation of member function 'Upp::AMap<Upp::String, Upp::Vector<Upp::String>, Upp::Vector<Upp::Vector<Upp::String> > >::Add' requeste
d here
(): AMap(std::initializer_list<std::pair<K, T>> init) { for(const auto& i : init) Add(i.first, i.second); }
/home/alexis/upp/uppsrc/Core/Map.h (236): note: in instantiation of member function 'Upp::AMap<Upp::String, Upp::Vector<Upp::String>, Upp::Vector<Upp::Vector<Upp::String> > >::AMap' reques
ted here
(): VectorMap(std::initializer_list<std::pair<K, T>> init) : B::AMap(init) {}
I guess the reason is
T *q = new(Rdd()) T(x); // <-- should be clone(x)
So I made a little patch that fixed the problem. Please, check it and give feedback if I did something wrong ![Smile](images/smiley_icons/icon_smile.gif)
UPD
I use upp from git repository https://github.com/ultimatepp/mirror
Used compiler: FreeBSD clang version 6.0.0 (tags/RELEASE_600/final 326565) (based on LLVM 6.0.0)
-
Attachment: vcont.patch
(Size: 1.67KB, Downloaded 219 times)
[Updated on: Fri, 09 August 2019 12:20] Report message to a moderator
|
|
|
Re: Problem with Vector::Add (pick/clone semantics) [message #52217 is a reply to message #52213] |
Fri, 09 August 2019 20:12 ![Go to previous message Go to previous message](theme/default/images/up.png) ![Go to next message Go to next message](theme/default/images/down.png) |
Novo
Messages: 1371 Registered: December 2006
|
Ultimate Contributor |
|
|
I personally would say that this is not a bug. This is a feature. ![Smile](images/smiley_icons/icon_smile.gif)
clone was intentionally removed from Add to prevent implicit cloning.
Basically, std::initializer_list will create a temporary const object and after that it will force you to create another copy of it. This is an unnecessary allocation.
U++ is warning you about that and offering you other tools like
VectorMap<String, Vector<String>> MY_MAP;
MY_MAP.Add("s1", Vector<String>{"s11", "s12", "s13", "s14"});
In this case objects will be moved.
Ideally, it would be great to have a set of overloaded operators VectorMap& operator()(const K& k, const T& v)
More details on this problem can be found here.
A comment to this article has an interesting code snippet:
template<std::size_t N>
Vec(T(&&a)[N])
: _vect(std::make_move_iterator(std::begin(a)), std::make_move_iterator(std::end(a)))
{}
Extra braces needed though, but somebody may find this more idiomatic:
Vec<int> v {{1, 2}};
Regards,
Novo
|
|
|
Re: Problem with Vector::Add (pick/clone semantics) [message #52218 is a reply to message #52213] |
Sat, 10 August 2019 06:33 ![Go to previous message Go to previous message](theme/default/images/up.png) ![Go to next message Go to next message](theme/default/images/down.png) |
Novo
Messages: 1371 Registered: December 2006
|
Ultimate Contributor |
|
|
Actually, it is possible to move data from std::initializer_list with a little hack:
template <typename T>
struct Foo {
Foo(std::initializer_list<T> init) {
for(const T& i : init)
v.Add(pick(const_cast<T&>(i)));
}
// Foo(std::initializer_list<T> init) {
// for(const T& i : init)
// v.Add(i);
// }
Vector<T> v;
};
struct Boo : Moveable<Boo> {
Boo() {}
Boo(const Boo&) = default;
Boo(Boo&&) = delete;
};
CONSOLE_APP_MAIN
{
Foo<Vector<int>> f = {{1}};
// Foo<Boo> f = {Boo()};
}
The problem is that this will require all types to have a move constructor.
A move constructor can be detected via std::is_move_constructible, but I couldn't figure out how to apply SFINAE to a constructor.
IMHO, all this code complexity is unnecessary in this case.
Regards,
Novo
|
|
|
Re: Problem with Vector::Add (pick/clone semantics) [message #52235 is a reply to message #52218] |
Tue, 13 August 2019 09:08 ![Go to previous message Go to previous message](theme/default/images/up.png) ![Go to next message Go to next message](theme/default/images/down.png) |
![](https://www.ultimatepp.org/forums/images/custom_avatars/3.png) |
mirek
Messages: 14003 Registered: November 2005
|
Ultimate Member |
|
|
I made it work, even without pick:
const static VectorMap<String, Vector<String>> MY_MAP = {
{"s1", Vector<String>{"s11", "s12", "s13", "s14"}},
{"s2", Vector<String>{"s21", "s22", "s23", "s24"}},
{"s3", Vector<String>{"s31", "s32", "s33", "s34"}},
{"s4", Vector<String>{"s41", "s42", "s43", "s44"}},
{"s5", Vector<String>{"s51", "s52", "s53", "s54"}}
};
(making this work is perhaps slight departure from "use clone/pick always", OTOH I feel uneasy altering initialization data (by pick) anyway).
Mirek
|
|
|
Re: Problem with Vector::Add (pick/clone semantics) [message #52238 is a reply to message #52235] |
Tue, 13 August 2019 16:02 ![Go to previous message Go to previous message](theme/default/images/up.png) ![Go to next message Go to next message](theme/default/images/down.png) |
Novo
Messages: 1371 Registered: December 2006
|
Ultimate Contributor |
|
|
mirek wrote on Tue, 13 August 2019 03:08I made it work, even without pick:
This won't compile:
const VectorMap<Vector<String>, String> MY_MAP = {
{Vector<String>{"s11", "s12", "s13", "s14"}, "s1"},
};
pick wasn't needed because Vector<String>{...} is an rvalue by itself.
It would be great to have all overloads of
VectorMap& VectorMap&::operator()(const K& k, const T& v)
similar to AMap::Add(k, v).
IMHO, the problem is not a constructor of VectorMap, but an implementation of std::initializer_list. I believe I saw an alternative implementation somewhere.
Regards,
Novo
|
|
|
Re: Problem with Vector::Add (pick/clone semantics) [message #52239 is a reply to message #52238] |
Tue, 13 August 2019 17:09 ![Go to previous message Go to previous message](theme/default/images/up.png) ![Go to next message Go to next message](theme/default/images/down.png) |
![](https://www.ultimatepp.org/forums/images/custom_avatars/3.png) |
mirek
Messages: 14003 Registered: November 2005
|
Ultimate Member |
|
|
Novo wrote on Tue, 13 August 2019 16:02mirek wrote on Tue, 13 August 2019 03:08I made it work, even without pick:
This won't compile:
const VectorMap<Vector<String>, String> MY_MAP = {
{Vector<String>{"s11", "s12", "s13", "s14"}, "s1"},
};
pick wasn't needed because Vector<String>{...} is an rvalue by itself.
Works now. Thanks.
Quote:
It would be great to have all overloads of
VectorMap& VectorMap&::operator()(const K& k, const T& v)
similar to AMap::Add(k, v).
Done. Long live std::forward...
Mirek
|
|
|
|
|
|
Goto Forum:
Current Time: Sun Sep 01 06:56:57 CEST 2024
Total time taken to generate the page: 0.00763 seconds
|