Feature/copy

[apmccartney]

Added a generic copy function for those occassion which you want to pass an lvalue to function accepting rvalue references, but don't want to move the data

[whaeck]

This is essentially the copy equivalent of the std::move function. I assume we'll be using this when we do not want to move an argument into an rvalue reference?

I am going to be the advocate of the devil here: if we use this too often, would it not be better to not use rvalue references (using a local variable instead) in a function call and explicitly use std::move when we want to move instead of copy? I think this is the more standard way (from what I see on the internet, I would assume this is the case) and as such it would be easier for other people to understand the code.

[apmccartney]

This is essentially the copy equivalent of the std::move function. I assume we'll be using this when we do not want to move an argument into an rvalue reference?

Yes, specifically an lvalue to a function requiring an rvalue reference argument.

if we use this too often, would it not be better to not use rvalue references (using a local variable instead) in a function call and explicitly use std::move when we want to move instead of copy?

I'm not sure I understand. Does the following example capture your suggestion?

Given a function with the prototype

void foo( expensiveType&& e);

you are suggesting

expansiveType bar;
auto copy = bar;
foo( std::move(copy) );
/* additional operations using bar */

as opposed to

expansiveType bar;
foo( utility::copy(bar) );
/* additional operations using bar */

The former is more expensive (without assuming certain compiler optimizations not guaranteed by the standard) and is more verbose. More over, the latter introduces a new variable to keep track of (and to name), as well as the potential to mistakenly use the `copy` variable in while still in an invalid state following the move operation. For most standard library container types, this results segmentation fault at runtime, which introduces extra pressure to test thoroughly

I think this is the more standard way (from what I see on the internet, I would assume this is the case)

In my experience, common wisdom is to specify these sort of prototypes using value semantics, e.g.

void foo( expensiveType e);

and when a move is desired, call to std::move, otherwise the value is implicit copied. My issue with this convention is that the default behavior (with respect to lvalues) is both

• implicit
• and expensive.

In my experience, when refactoring for performance working on NJOY and elsewhere, the largest gains are generally made by eliminating unnecessary implicit data copying of lvalues because of value semantics. By retaining the function prototype accepting an rvalue reference, a developer retain the ability to copy expensive types (by way of the generic copy), but can't do so accidentally.

[whaeck]

What I meant was your last example:

void foo( expensiveType e);

and

expansiveType bar;
foo( std::move(bar) 

It is a good thing to have these discussions. I like your approach. We actually know we are copying/moving expensive types. Both approaches are essentially equivalent but differ in the sense that one will copy by default while the other will move by default unless we specifically say otherwise. And if we have to std::move all the time, we are probably doing it wrong.

[jlconlin]

Good job adding the tests for trim. What's going on with appveyor? I'm not sure those failures should prevent us from merging.

[apmccartney]

@jlconlin The appveyor hook has been removed and the pull can now be merged.