Feature Request: Allow specifying calling convention attributes to Mock methods

[cesenaLA]

A method could specify a calling convention attribute like stdcall, cdecl, fastcall, thiscall; supported by GCC, Clang and MSVC. which is part of the signature of the function. Currently we don't have a way to specify a calling convention in trompeloeil making it impossible to mock any function using these attributes.

GMock does support a similar feature and allows to mock these kind of methods, so it would be nice to have it here too.

Looks like TROMPELOEIL_MAKE_MOCK_ could be updated to accommodate another argument calling_convention https://github.com/rollbear/trompeloeil/blob/45583abb9920e65d6f715378ddab690c42793a1f/include/trompeloeil/mock.hpp#L3524-L3526

And insert it in the generated method https://github.com/rollbear/trompeloeil/blob/45583abb9920e65d6f715378ddab690c42793a1f/include/trompeloeil/mock.hpp#L3586-L3589

Is this something you could consider?

[rollbear]

Hmmm.

For gcc and clang, this already works since they support having the attribute at the end, so you can write:

MAKE_MOCK1(func, int(int), __attribute__((cdecl)));

MSVC makes it more difficult since it wants the attribute in an odd location.

Most common use of mocks is to mock virtual functions from an interface, and since these attributes specify the calling convention, I would expect the attribute to be applied to base function declaration, and that inherited implementations would have to follow it. Is this incorrect? Or is this not the desired use case?

https://godbolt.org/z/q7634G7Yo

[puetzk]

MSVC makes it more difficult since it wants the attribute in an odd location.

Indeed, that's the motive of this issue. MSVC wants calling conventions between the return type and the identifier. You're quite correct that gcc/clang are more flexible in the positioning of __attribute__,

Most common use of mocks is to mock virtual functions from an interface, and since these attributes specify the calling convention, I would expect the attribute to be applied to base function declaration, and that inherited implementations would have to follow it. Is this incorrect? Or is this not the desired use case?

That's the desired use case, but the inherited implementations "have to follow it", and don't do so implicitly. If the override fails to match calling convention, you get a compile error C2695: 'Foo::foo': overriding virtual function differs from 'IFoo::foo' only by calling convention (https://godbolt.org/z/vsvT8PcGs).

GCC is the same here - your example only works because __attribute__((cdecl)) was the default anyway. If you change it to __attribute__((stdcall)) or thiscall or anything non-default, you get "error: conflicting type attributes specified for ..." just like MSVC. So this "mock virtual functions from an interface" requires MAKE_MOCK* to declare its implementation with the correct calling convention.

[rollbear]

Sigh. So implementers go out of their way to make their user's lives miserable, when there is an obvious and simple solution. Great!

I'll try, but I will not make any promises.

[puetzk]

I think the most natural place (given that both implementations consider it to be part of the signature) for override matching) is that all 3 compilers (MSVC/g++/clang) allow calling conventions to appear in a function type, e.g. MAKE_MOCK1(func, int __stdcall(int));

You can detect the qualifiers in the usual type-traits sort of way (I'm not aware of a standard trait for this, but you can implement one in the usual way with specializations).

https://godbolt.org/z/eqqrrv1hG

#include <type_traits>

#ifdef __GNUC__
#define __stdcall __attribute__((stdcall))
#endif

template<typename F> struct is_stdcall;
template<typename R, typename... Args> struct is_stdcall<R(Args...)> : std::false_type {};
template<typename R, typename... Args> struct is_stdcall< R __stdcall(Args...)> : std::true_type {};

static_assert(is_stdcall<int __stdcall(int)>::value);
static_assert(!is_stdcall<int(int)>::value);

[puetzk]

Hmm, though this provokes a warning "stdcall' attribute ignored [-Wattributes]" from gcc on x64 (cdecl and stdcall are the same on x86_64, though different on x86). And that also makes an is_stdcall trait built from specializations ambiguous. Grr... easy enough to work around, but getting less tidy...

[puetzk]

We could skirt closer to Abominable Function Types and actually use sig directly as the function type (also avoiding return_of_t<TROMPELOEIL_REMOVE_PAREN(sig)>, TROMPELOEIL_PARAM_LIST

It seems to be OK if only the declaration (and not definition) mentions the calling convention, so one can do

using sig = int __stdcall (int);
struct IFoo {
    virtual sig foo = 0;
};

struct CFoo : IFoo {
    sig foo;
};

int CFoo::foo(int x) { return x; }

and you do get the __stdcall (callee-clean) ABI applied.

Which would be nice, since that would let you just use sig as the function type, and no even have to take it apart with return_of_t and TROMPELOEIL_PARAM_LIST. Even constness can be absorbed into that, though you're then dealing with Abominable Function Types (legal, but very weird).

But I don't know of any way to combine the declaration and definition when using a function type like this. And a macro like MAKE_MOCK needs to keep everything inlined since it can't easily emit a definition later.

[puetzk]

Starting to see why gmock needed you to wrap these attributes in a Calltype(...) so it could do token-pasting games to separate them from the other __VA_ARGS__...

[rollbear]

After thinking about this, I'm leaning towards not supporting it in the library per se. You can achieve what you want with the traditional extra level of indirection. Make a thin function, with whatever attributes you need, and have it call a mock. I can maybe add an example of this usage in the docs.

[puetzk]

Hmm. I assume you mean doing something like Mocking operator(). That's clearly possible, but pretty inconvenient if you have to do it for every method, because you can't overload by calling convention. You have to come renaming every method, e.g.

class A {
    int __stdcall foo(int x) { return cdecl_foo(x); }
    MAKE_MOCK1(cdecl_foo, int(int));
};

Which then means all the code setting expectations, etc has to use that cdecl_foo name rather than the real foo, and all the error messages will also show this. It's certainly possible, but rather clunky (of course, MSVC really gets the blame for making the whole thing unnecessarily awkward by not letting you just put callconv in the spec position).

[puetzk]

It seems easy enough to support the addition in TROMPELOEIL_MAKE_MOCK_ as @cesenaLA proposed, callconv is just another field like sig or spec that or might not might be empty, and gets expanded into its correct place in the declaration between return_of_t and name.

The hard part is the rest - how to sort a calling convention out of the __VA_ARGS__. Which is presumably the same reason you ended up with MAKE_CONST_MOCK0 instead being able to do MAKE_MOCK0(foo,void(), const)). But of course having more than just const/not-const quickly leads to a combinatorial explosion of different qualifiers.

But... in the specific case we hit (implementing COM interfaces defined in .idl/.tlb files), there's actually no such thing as const, so there's no need for the permutations. So I was being narrowly selfish, it would be quite satisfactory to just have the low-level support and then the 16 MAKE_STDMETHOD_MOCKn(name,...) forwards that called TROMPELOEIL_MAKE_MOCK_(name,,STDMETHODCALLTYPE,n, __VA_ARGS__,,) (picking STDMETHOD as the name for this limited subset by analogy to The macros for declaring and implementing COM interfaces / basetyps.h

Which does still add a fair amount of clutter (the 16 MAKE_STDMETHOD_MOCKn forwards, their #ifndef TROMPELOEIL_LONG_MACROS short aliases, etc) to mock.hpp - though I guess you could further add only the new argument to TROMPELOEIL_MAKE_MOCK_ and then exile the rest to mock_callconv.hpp or something (to avoid inflicting compile time overhead on folks who have no need for it). But it seems straightforward clutter...

Sketched this in https://github.com/rollbear/trompeloeil/compare/main...puetzk:trompeloeil:STDMETHOD and I at least didn't break TROMPELOEIL_BUILD_TESTS=ON. Unless you immediately hate it I or @cesenaLA will at least give this a shot on the test code he was trying to adapt from googletest to Catch2/trompeloeil; I think it should cover what COM interfaces need (which was his motivating use).

[rollbear]

OK, cool. I'll have a look. I'm a bit busy the next few days. Remind me if I haven't come back to you some time next week.

[rollbear]

Sorry for the delay. From a short glance, your sketch looks OK. Can you make a PR?