Fix a GCC warning, and fix a crash under GCC when de-virtualization is enabled

eranpeer / FakeIt

C++ mocking made easy. A simple yet very expressive, headers only library for c++ mocking.

MIT License

1.22k stars 169 forks source link

Fix a GCC warning, and fix a crash under GCC when de-virtualization is enabled #214

Closed hedayat closed 3 years ago

hedayat commented 3 years ago

Fix a GCC warning about incompatible member function pointer cast
Disable de-virtualization in union_cast, which effectively prevents crashes when optimizaton is enabled under GCC when mocking classes with inline virtual dtor.

coveralls commented 3 years ago

Coverage remained the same at 99.923% when pulling 614c7851c6cc47b1149cab1a01f42aeb9705706e on hedayat:gcc-waraning-devirt-fix into 814f28cd509ef2a8cdf7fe370bd9ec6d3b181dc7 on eranpeer:master.

VirtualTim commented 3 years ago

You should probably stick the gcc specific stuff in a #if defined (__GNUG__). This sounds important, so hopefully it gets merged?

Edit: I had to merge this change in to prevent segaults in release builds.

hedayat commented 3 years ago

I've not tested, but AFAIK other compilers should ignore both unknown (scoped) attributes, and unknown pragmas. So, most probably compiler conditionals are not needed.

Unfortunately, the repo seems inactive. BTW, FakeIt docs says that it doesn't support optimized builds; so this PR is actually an enhancement rather than bug fix. So, not THAT important.

VirtualTim commented 3 years ago

I think this change may also have to be added to everything under /single_header/. I'm not really sure how this repo works, I'm guessing you're meant to run generate_fakeit_single_header.py, and that'll update those?

hedayat commented 3 years ago

I think this change may also have to be added to everything under /single_header/. I'm not really sure how this repo works, I'm guessing you're meant to run generate_fakeit_single_header.py, and that'll update those?

I'm not sure too. Anyway, since it is merged, I'll let himself do whatever is needed.