Oss 7 copy return for return by ref methods

[FranckRJ]

New PR because I can't reopen the old one : #280

[coveralls]

Coverage: 99.925% (+0.0002%) from 99.925% when pulling 36ad3e6 on iress:OSS-7-copy-return-for-return-by-ref-methods into 3c36d1a on eranpeer:dev.

[malcolmdavey]

Any change of getting this one in? Is there anything outstanding.

[malcolmdavey]

For issue #258

[FranckRJ]

Sorry for the late reply. In the original PR I made this comment: #280 (comment)

The first two points aren't relevant anymore, but if you could answer the third one it would be nice.

[malcolmdavey]

Sorry for the late reply. In the original PR I made this comment: #280 (comment)

The first two points aren't relevant anymore, but if you could answer the third one it would be nice.

Return(T&& t): from what I understand it's to fix an issue when passing rvalues to Return and the return type of the mocked function is a reference, but I have several questions about it, that makes me not too confident about merging it in its current state:
why using a new template parameter instead of R like others Return ? I don't think it's a bad idea per se but it's not how others Return a written and change a bit how the method work, I think it may be better to keep it consistent.
why do you check that T is copy constructible ? What about move-only types ?

1. Can't remember about the first case, why I specifically changed it. Will experiment (though I think with out the VS projects might take me a while to get it all running/testing etc)
2. Also I think for the AlwaysReturn, we can't have move-only types. But we could with the Return I guess. As longs as we are happy with that.

[malcolmdavey]

Currently I'm getting failures for this line/test: cannot convert argument 1 from 'int' to 'int &'

 			When(Method(mock, returnIntByRef)).Return(1);

Suggested change

	template<typename T>
	typename std::enable_if<!std::is_reference<T>::value && std::is_copy_constructible<T>::value, MethodStubbingProgress<R, arglist...>&>::type
	Return(T&& t) {
	template<typename U = R>
	typename std::enable_if<!std::is_reference<T>::value && std::is_copy_constructible<T>::value, MethodStubbingProgress<R, arglist...>&>::type
	Return(R&& r) {

[FranckRJ]

By "currently" you mean the code in the PR (so, without the changes in your message) or the code once you apply the changes in your message?

[malcolmdavey]

I mean, if I try this "suggested change" to the PR, then it fails for basic integer values (maybe because of other overloads)

[malcolmdavey]

Using this

        typename std::enable_if<!std::is_reference<T>::value && std::is_copy_constructible<T>::value, MethodStubbingProgress<R, arglist...>&>::type
    	Return(R&& r) {

I get this error message:

referece_types_tests.cpp
c:\devel\git\fakeit\tests\referece_types_tests.cpp(196): error C2664: 'fakeit::MethodStubbingProgress &fakeit::MethodStubbingProgress::Return(void)': cannot convert argument 1 from 'int' to 'int &'
with
[
R=int &
]

Not saying there isn't a way to fix it but just haven't found it yet.

Also thinking the existing overload just shouldn't support R value refs at all, so should be specific to lvalue refs, in stead of all refs, as it will always be a dangling pointer, and hence always undefined behaviour.

[FranckRJ]

Superseded by #332.