Overview
Examples
Screenshots
Comparisons
Applications
Download
Documentation
Tutorials
Bazaar
Status & Roadmap
FAQ
Authors & License
Forums
Funding Ultimate++
Search on this site
Search in forums












SourceForge.net Logo
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
shutalker is currently offline  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

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 202 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 messageGo to next message
Novo is currently offline  Novo
Messages: 1358
Registered: December 2006
Ultimate Contributor
I personally would say that this is not a bug. This is a feature. Smile
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

Report message to a moderator

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 messageGo to next message
Novo is currently offline  Novo
Messages: 1358
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

Report message to a moderator

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 messageGo to next message
mirek is currently offline  mirek
Messages: 13975
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

Report message to a moderator

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 messageGo to next message
Novo is currently offline  Novo
Messages: 1358
Registered: December 2006
Ultimate Contributor
mirek wrote on Tue, 13 August 2019 03:08
I 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

Report message to a moderator

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 messageGo to next message
mirek is currently offline  mirek
Messages: 13975
Registered: November 2005
Ultimate Member
Novo wrote on Tue, 13 August 2019 16:02
mirek wrote on Tue, 13 August 2019 03:08
I 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

Report message to a moderator

Re: Problem with Vector::Add (pick/clone semantics) [message #52242 is a reply to message #52239] Thu, 15 August 2019 03:47 Go to previous messageGo to next message
Novo is currently offline  Novo
Messages: 1358
Registered: December 2006
Ultimate Contributor
mirek wrote on Tue, 13 August 2019 11:09
Done. Long live std::forward...

Mirek

Thank you!

Regards,
Novo

Report message to a moderator

Re: Problem with Vector::Add (pick/clone semantics) [message #52280 is a reply to message #52213] Mon, 26 August 2019 10:42 Go to previous messageGo to next message
shutalker is currently offline  shutalker
Messages: 15
Registered: November 2017
Location: Moscow
Promising Member
Mirek, Novo, thank you! Your explanations and this
article are very helpful, though I should find out more about initialization by myself Smile

Report message to a moderator

Re: Problem with Vector::Add (pick/clone semantics) [message #52281 is a reply to message #52213] Mon, 26 August 2019 19:27 Go to previous message
mr_ped is currently offline  mr_ped
Messages: 825
Registered: November 2005
Location: Czech Republic - Praha
Experienced Contributor
About C++ initialization .. (animated gif .. sort of joke... but not really):
https://i.imgur.com/3wlxtI0.gifv

Report message to a moderator

Previous Topic: Vector< Vector<int> > compilation error
Next Topic: SHA1 Doesn't work any more for data over 65536 bytes
Goto Forum:
  

[ Syndicate this forum (XML) ] [ RSS ] [ PDF ]

Current Time: Fri Apr 19 01:39:08 CEST 2024

Total time taken to generate the page: 0.02848 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.5.
Copyright ©2001-2013 FUDforum Bulletin Board Software