Add support for std::function in MockFunction (#2277)

[adambadura]

MocFunction is slightly changed to transform its argument through (added in this change) SignatureOf meta-function. The meta-function extracts the function signature type from the provided type. Thanks to this approach MockFunction<F> can now be used with F being both function signature (like bool(int)) and std::function (like std::function<bool(int)>).

Besides the immediate benefit, extra extensibility is added. Library user can specialize SignaturOf for other types (like boost::function) to gain the same support level.

Note that this PR comes from #2319.

[kuzkry]

Nice style, that's a very clean piece of code. I like the cleverness of how you approached writing unit tests as you employ a compiler to do most of the work.

[kuzkry]

One thing I forgot to ask you is whether the diff in googlemock/include/gmock/gmock-spec-builders.h has to be so huge :)

[adambadura]

One thing I forgot to ask you is whether the diff in googlemock/include/gmock/gmock-spec-builders.h has to be so huge :)

I don't know how to make it smaller. The change is simple - added part around SignatureOf and moving of MockFunction to internal namespace. Yet Git somehow perceives it in a more difficult way.

[kuzkry]

Yes, sometimes the diff algorithm gets crazy. Thanks and keep up the good work :)

[akonradi]

@gennadiycivil this was split out from #2319, would you mind taking a look? I was looking for something like this today and realized @adambadura had already written it in OSS land.

[CJ-Johnson]

@adambadura Thank you for the PR! After discussing this with the other owners, we're not interested in changing the "meaning" of the name MockFunction (from class template to alias template).

That said, we would be willing to consider a new name for this exact behavior. The new name would be an alias template out the gate. Is that a change you'd be open to?

[adambadura]

@adambadura Thank you for the PR! After discussing this with the other owners, we're not interested in changing the "meaning" of the name MockFunction (from class template to alias template).

That said, we would be willing to consider a new name for this exact behavior. The new name would be an alias template out the gate. Is that a change you'd be open to?

@CJ-Johnson Sure. I prefer my approach, but I'm fine with your approach as well. And definitely either one is better than none. ;)

However, what precisely you want to get?

1. I will get the MockFunction back to the main namespace (from internal where I have put it in this change).
2. But what name to pick for the new entity that will handle std::function? MockFunctor?
3. Should the new entity work with both std::function and function signatures? I would recommend so to have a universal solution.
4. Should the SignatureOf remain part of the public interface or should it be handled by the new entity directly? Either way, I would strongly recommend an approach that allows users to extend the behavior for compatible function wrappers (like boost::function for example).

[sbenzaquen]

Sorry for the delay.
I consulted with a coworker and my misgivings about this change have been solved.
Let's do this:

1. MockFunction<T> supports T=std::function<F>. We talked about supporting other callable types, like function pointers, but decided that we don't need to do that now. If there is a need in the future, it could be easily added.
2. SignatureOf would be an implementation detail, live in internal and not be mentioned in the .md documentation.

Thanks for the effort!

[sbenzaquen]

After testing this change, I found that this breaks valid use cases.
We can't make testing::MockFunction<T> be an alias with a dependent type on it. That breaks type deduction, specializations, etc.

Eg, this code:

template <typename T>
void Foo(testing::MockFunction<T>& t);

will not be able to deduce the T anymore when passing an instance of the type.

We have to keep testing::MockFunction as a class template.
An alternative solution is to make testing::MockFunction be like:

template <typename T>
class MockFunction : public internal::MockFunction<internal::SignatureOfT<T>> {
  using Base = internal::MockFunction<internal::SignatureOfT<T>>;
 public:
  using Base::Base;
};

[adambadura]

@sbenzaquen, I have implemented it the way you suggested. Also, I have added tests that check the property you asked for so that no such error is made in the future.

However, I don't know why the verification failed. It did work on my (similar I think) configuration at my end. And I don't know how to get to exact logs of the failure. What can I do about it?

[sbenzaquen]

The problem is that tests in the different _test.cc files have the same names and are colliding when running the gmock_all test. (they are all linked together and failing then).
Maybe rename the one in generated- to have Generated in its name.

[adambadura]

The problem is that tests in the different _test.cc files have the same names and are colliding when running the gmock_all test. (they are all linked together and failing then).
Maybe rename the one in generated- to have Generated in its name.

It did help! Thanks! I just couldn't see what is the problem. I tried building on Linux subsystem on my Windows and even on Linux itself at work but got the same and didn't know how to proceed. Now it works.

Although I used a different naming approach, to mimic already existing one in those files.

However, I wonder why those tests are duplicated in the first place.

[adambadura]

@sbenzaquen , can we finally proceed with this change?