eranpeer / FakeIt

C++ mocking made easy. A simple yet very expressive, headers only library for c++ mocking.
MIT License
1.24k stars 173 forks source link

Missing virtual destructors #260

Open matthewT53 opened 2 years ago

matthewT53 commented 2 years ago

Hello, when building FakeIt 2.0.9 from conan-center I get the following warnings as errors:

/home/someUser/.conan/data/fakeit/2.0.9/_/_/package/2c17f3a304303e51860708bc68822afdefc5985f/include/fakeit.hpp:740:12: error: ‘s
truct fakeit::VerificationEventHandler’ has virtual functions and accessible non-virtual destructor [-Werror=non-virtual-dtor]     740 |     struct VerificationEventHandler {                                                                                    
      |            ^~~~~~~~~~~~~~~~~~~~~~~~ 
/home/someUser/.conan/data/fakeit/2.0.9/_/_/package/2c17f3a304303e51860708bc68822afdefc5985f/include/fakeit.hpp:746:12: error: ba
se class ‘struct fakeit::VerificationEventHandler’ has accessible non-virtual destructor [-Werror=non-virtual-dtor]                746 |     struct EventHandler : public VerificationEventHandler {                                                       
      |            ^~~~~~~~~~~~                                                                                                  

I am building my C++ project with meson with warning level = 3 and Werror set to true.

There are non-virtual-dtor errors in more classes. I've tried to fix the issue by adding virtual destructors into the affected classes however when I try to run the unit tests I get free(); Invalid pointer.

[--------------]
[ RUN          ] VirtualOffsetSelectorTest::verifyAllIndexes
[       PASSED ] VirtualOffsetSelectorTest::verifyAllIndexes
[--------------]

[--------------]
[ RUN          ] BasicVerification::verificationProgressShouldBeConvertibleToBool
free(): invalid pointer
malcolmdavey commented 2 years ago

Just tried what I think you are doing on windows Visual Studio 2019, but haven't got that problem.

Could you show your changed classes? Also are you only getting this when building the tests?

Will take more time to get gcc going to try it out.

FranckRJ commented 1 year ago

I'm not sure fakeit is really designed to be build with a high error level (there a some hacks inside the library). I don't know meson but I'm pretty sure you should be able to disable / reduce warning level for system headers (everything included with <>, it's a feature in GCC/Clang and MSVC).

But if we can reduce the number of warnings (or locally silence the ones we're sure are "false-positive") it's always better. So I guess we shouldn't ignore these too much.

malcolmdavey commented 1 year ago

I've tried to fix the issue by adding virtual destructors into the affected classes however when I try to run the unit tests I get free(); Invalid pointer.

I'm wondering if this case is fixed by some of the recent dtor changes (I think will be in 2.3.1) e.g. https://github.com/eranpeer/FakeIt/pull/289