-
Notifications
You must be signed in to change notification settings - Fork 86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Templatized base types do not work with mock_interface #116
Comments
When using a template base class as the parameter to `mock_interface`, the `IMPLEMENT_MOCKn()` macros fail to compile due to `typename` missing before the `decltype(..)::type` statement. Fixes rollbear#116 Signed-off-by: Robert Dailey <[email protected]>
@rollbear: Can you dedicate some time to this one? |
I was hoping you'd add a test case, I asked for. Anyway, I went ahead and tried to add the following to 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 Do you have any good ideas? |
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.
This is the same error I ran into, and the addition of |
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 |
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. |
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 |
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 I tried something like What do you think? |
@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. |
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. |
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 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. |
When using a template base class as the parameter to `mock_interface`, the `IMPLEMENT_MOCKn()` macros fail to compile due to `typename` missing before the `decltype(..)::type` statement. Fixes rollbear#116 Signed-off-by: Robert Dailey <[email protected]>
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? |
Yeah, it's worth considering. I'm not that fond of the duplication of the base name, but it might be impossible to avoid. |
@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. |
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 |
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) |
So I do some trickery to forward template types through
mock_interface
in the middle of a hierarchy. Example:And here is it being used:
Fsm::State
here requires a template type, which I forward through myMockState
. This code won't compile because thetypename
keyword is not used in the implement mock macros. Here is the code that fixes the problem:I will submit a PR for this issue.
The text was updated successfully, but these errors were encountered: