Resolve malformed override specifiers with function pointers

[thetic]

These raised warnings with -Wsuggest-override. I personally find this easier to read as well.

[basvodde]

I don't agree with the typedef being easier to read, I personally prefer not using a typedef here. But I'm curious about the warnings that it resolved ?

[thetic]

src/CppUTestExt/../../include/CppUTestExt/MockCheckedActualCall.h:95:20: error: ‘virtual void (* MockCheckedActualCall::returnFunctionPointerValue())()’ can be marked override [-Werror=suggest-override]
   95 |     virtual void (*returnFunctionPointerValue())() _override;
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/CppUTestExt/../../include/CppUTestExt/MockCheckedActualCall.h:96:20: error: ‘virtual void (* MockCheckedActualCall::returnFunctionPointerValueOrDefault(void (*)()))()’ can be marked override [-Werror=suggest-override]
   96 |     virtual void (*returnFunctionPointerValueOrDefault(void (*)()))() _override;
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/CppUTestExt/../../include/CppUTestExt/MockCheckedActualCall.h:216:20: error: ‘virtual void (* MockActualCallTrace::returnFunctionPointerValue())()’ can be marked override [-Werror=suggest-override]
  216 |     virtual void (*returnFunctionPointerValue())() _override;
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/CppUTestExt/../../include/CppUTestExt/MockCheckedActualCall.h:217:20: error: ‘virtual void (* MockActualCallTrace::returnFunctionPointerValueOrDefault(void (*)()))()’ can be marked override [-Werror=suggest-override]
  217 |     virtual void (*returnFunctionPointerValueOrDefault(void (*)()))() _override;
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/CppUTestExt/../../include/CppUTestExt/MockCheckedActualCall.h:292:20: error: ‘virtual void (* MockIgnoredActualCall::returnFunctionPointerValue())()’ can be marked override [-Werror=suggest-override]
  292 |     virtual void (*returnFunctionPointerValue())() _override { return NULLPTR; }
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/CppUTestExt/../../include/CppUTestExt/MockCheckedActualCall.h:293:20: error: ‘virtual void (* MockIgnoredActualCall::returnFunctionPointerValueOrDefault(void (*)()))()’ can be marked override [-Werror=suggest-override]
  293 |     virtual void (*returnFunctionPointerValueOrDefault(void (*value)()))() _override { return value; }
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

See also https://github.com/cpputest/cpputest/pull/1604

[basvodde]

Oh, that is strange. Is it because it makes it a pointer to member function? Lemme have a look at this later. Thanks for all your contributions, I'll gradually be merging most of them.

[thetic]

Yeah, it's definitely strange. It feels like a gcc bug, but I reproduces across multiple versions of both gcc and clang.

[coveralls]

Coverage remained the same at 99.64% when pulling c749833e2fee965db3c3c7feb8da15b4c35bd412 on thetic:function-pointers into 875dde18e3fc940bfd7f14af121c7e486d922bac on cpputest:master.

[thetic]

Have you had a chance to look into this yet?

[basvodde]

What do we do with the function pointer typedef? I really do not want to use a typedef for all the FPs, personally I find it harder to read as it hides so much info.

[thetic]

https://isocpp.org/wiki/faq/pointers-to-members#typedef-for-ptr-to-memfn

[basvodde]

"Yea, right, I know: you are different. You are smart. You can do this stuff without a typedef. Sigh. I have received many emails from people who, like you, refused to take the simple advice of this FAQ"

haha.

[thetic]

it reproduces across multiple versions of both gcc and clang.

I was wrong on this one.

GCC expects:

void (*member() override)();

Clang expects:

void (*member())() override;

[basvodde]

Wow, and it is not standardized ?

To me the gcc expectation feels wrong...

[basvodde]

Considering the different syntax for gcc and clang, I guess I'll need to give up and go with a typedef.

Could we at least try a slightly more descriptive name as FunctionPointer and move the typedef outside of the class space so that it doesn't have to be declared on the class in the cpp file ?

[thetic]

Could we at least try a slightly more descriptive name as FunctionPointer and move the typedef outside of the class space so that it doesn't have to be declared on the class in the cpp file ?

I can't think of a good name that is more descriptive for void (*)(). I am open to suggestions.

My original intentions for putting the typedef in the class was to avoid naming collisions with client code.

[basvodde]

Ok. could you refer to is as cpputest_functionPointerRetrunValue or cppUTestFunctionPointerReturnValue or FunctionPointerReturnValue.

The reason is that it seems it is only in this context that the typedef is used. Other function pointers that are passed as parameter are still written in the 'normal' form.

If you strongly disagree with it, then I can also merge it as is... the usage of the typedef is quite localized.

[thetic]

OK. I've renamed the typedef to FunctionPointerReturnValue and reduced the change set to the bare minimum required to satisfy -Wsuggest-override.

[basvodde]

Thanks!