Templatized base types do not work with mock_interface

[rcdailey]

So I do some trickery to forward template types through mock_interface in the middle of a hierarchy. Example:

   template<typename TStateMachine>
   class MockState : public trompeloeil::mock_interface<Fsm::State<TStateMachine>>
   {
   public:
      IMPLEMENT_MOCK1(Enter);
      IMPLEMENT_MOCK1(Exit);
      IMPLEMENT_MOCK1(Update);
   };

And here is it being used:

class TestStateMachine : public Fsm::StateMachine<TestStateMachine> {};
class TestState : public Testing::MockState<TestStateMachine> {};

Fsm::State here requires a template type, which I forward through my MockState. This code won't compile because the typename keyword is not used in the implement mock macros. Here is the code that fixes the problem:

#define TROMPELOEIL_IMPLEMENT_MOCK_(num, name) \
  TROMPELOEIL_MAKE_MOCK_(name,,num, typename decltype(::trompeloeil::nonconst_member_signature(&trompeloeil_interface_name::name))::type,override,)
#define TROMPELOEIL_IMPLEMENT_CONST_MOCK_(num, name) \
  TROMPELOEIL_MAKE_MOCK_(name,const,num, typename decltype(::trompeloeil::const_member_signature(&trompeloeil_interface_name::name))::type,override,)

I will submit a PR for this issue.

[rcdailey]

@rollbear: Can you dedicate some time to this one?

[rollbear]

I was hoping you'd add a test case, I asked for.

Anyway, I went ahead and tried to add the following to compiling_tests.hpp, so as to get it under test.

template <typename T>
struct t_interface
{
  virtual ~t_interface() = default;
  virtual void func(T) = 0;
  virtual void cfunc(T) const = 0;
};

template <typename T>
struct t_mock : trompeloeil::mock_interface<t_interface<T>>
{
  IMPLEMENT_MOCK1(func);
  IMPLEMENT_CONST_MOCK1(cfunc);
  MAKE_MOCK1(local, void(T));
};

using int_mock = t_mock<int>;

It doesn't compile because trompeloeil_interface_name is not found. It's a dependent name from the base class wrapper, so that's understandable. I can make it compile by adding the abomination using typename trompeloeil::mock_interface<t_interface<T>>::trompeloeil_interface_name; at the top of t_mock, however, I don't want to go with that. I think something better is needed. Or at the very least, documentation is required.

Do you have any good ideas?

[rcdailey]

I was hoping you'd add a test case, I asked for.

Oh I apologize for that, I overlooked your reply and didn't see it. Turns out, however, that it wouldn't have mattered anyway since the approach seems to be wrong based on your findings.

It doesn't compile because trompeloeil_interface_name is not found. It's a dependent name from the base class wrapper, so that's understandable. I can make it compile by adding the abomination using typename trompeloeil::mock_interface<t_interface<T>>::trompeloeil_interface_name; at the top of t_mock, however, I don't want to go with that. I think something better is needed. Or at the very least, documentation is required.

This is the same error I ran into, and the addition of typename fixed it for me using clang 6.0 on Ubuntu 18.04. Are you sure you were testing with my fix?

[rollbear]

Yes, it's with your fix. The difference is that I took the template thing one step further, and made the mock class a template too. With the mock class being concrete, your fix works.

Maybe the easiest solution is to keep your fix as is, and add documentation saying that there must be a typename trompeloeil_interface_name, that aliases the interface to implement. It is not necessary to use trompeloeil::mock_interface<>, since all it does is to add that typename. The crux is that since it's a template inheriting a template, and the name sought is a dependent name of the base class, it's impossible (I think) to get without qualifying the name. Unfortunately the language does not allow using this in this context (at least not until P0847R1 or something like it, is voted into the standard.)

[rcdailey]

Let me take a peek at this, I agree with you that all of this should work. My example is a little different than yours but looks like it should still work.

[rcdailey]

I think we can solve this by using type traits pattern. The issue is that because you've made the derived type a template, the type is incomplete. I think using a type trait class would fix this, but it would add more classes and template specializations in the trompeloeil header file.

I'll go ahead and type up a working example for you

[rcdailey]

Actually even if type traits fixes the problem, it can't be done without requiring the user to type more code. Basically, they'd have to invoke a REGISTER_MOCK_INTERFACE() macro, which underneath implements a type trait base class to provide the complete types, or you'd have to change the actual mock interface definition to be a macro.

I tried something like &typename trompeloeil_interface_name::name but that's not valid syntax. Basically I thought I could use typename to force the compiler to not require a complete type for trompeloeil_interface_name but that doesn't seem possible.

What do you think?

[rcdailey]

@rollbear Let's go ahead and get what I have merged and we can figure out your second use case. Based on my investigation, we should at least get past my specific example and then find a solution to yours, which is a different issue, probably an edge case, and much more difficult to solve.

[rcdailey]

If you want tests I'll have to find the time to do it. I'm not familiar with your code base or test structure. It will be a learning curve.

[rcdailey]

Sadly, I am not going to be able to provide you the unit test coverage you requested. My development platform is Windows and it has become clear to me that no testing is done on Windows. For example, your thread_terror target links pthread which is not available on MSVC. I've already started down a rabbit trail of compiler errors.

I sadly do not have the time to write tests and make everything work on Windows.

At this point I will request that you merge in my PR as-is, with no tests, or write the test coverage yourself on the platform you typically test/develop on (I assume Linux). It's been a long time and this bug continues to block me. I would appreciate getting this in as quickly as possible.

For now I will maintain this change on my fork and pull releases from that. Thank you for your time @rollbear.

[rcdailey]

Found out my change still doesn't work on clang on linux, but found a surprisingly much simpler approach:

#define MOCK_INTERFACE(base) \
    using trompeloeil_interface_name = base

template <typename T>
struct t_interface
{
  virtual ~t_interface() = default;
  virtual void func(T) = 0;
  virtual void cfunc(T) const = 0;
};

template <typename T>
struct t_mock : t_interface<T>
{
  MOCK_INTERFACE(t_interface<T>);
  IMPLEMENT_MOCK1(func);
  IMPLEMENT_CONST_MOCK1(cfunc);
  MAKE_MOCK1(local, void(T));
};

using int_mock = t_mock<int>;

Don't really need a real base class for any of this, you just need to know the type of the base to get the function types. This, to me, is the ideal approach because this works for template and non-template base classes. Thoughts?

[rollbear]

Yeah, it's worth considering. I'm not that fond of the duplication of the base name, but it might be impossible to avoid.

[rcdailey]

@rollbear Not impossible to avoid, but would require you to do something like this:

template<typename T>
DEFINE_MOCK_INTERFACE(t_mock, t_interface<T>)
{
public:
  IMPLEMENT_MOCK1(func);
  IMPLEMENT_CONST_MOCK1(cfunc);
  MAKE_MOCK1(local, void(T));
};

And I've seen you state plenty of times that you don't like this approach.

[rollbear]

No, that's ugly. I'm actually contemplating accepting your original PR, but with documentation stating the limitation and how you can work around it by introducing the type by hand.

The thing that's preventing me from doing that is that I'd kind of like to trigger a static_assert(), with a message hinting to the documentation, otherwise I fear there'll be confused questions.

[rcdailey]

Honestly after struggling with this for so long, and trying different things, I think KISS principle will guide us the best direction at this point. My original PR is not comprehensive. It didn't even solve my original problem on Linux (only on Windows). So you should consider it a non-functional solution.

The only solution I recommend is the one with the macro you call and pass in the base type again. There's no perfect solution here, and it sounds like the syntactical redundancy of the solution I propose is the least-crappy of all the other trade-offs.

This is also backward compatible: Keep the interface class around for existing code bases, but deprecate it. The macro will be the "new" way of doing it going forward, because it supports all variations of mock classes (no template, template, CRTP, etc)