rethrow copy constructor nothrow bug

[rianquinn]

If you run clang-tidy against the latest version of hippomocks, you get two errors:

/home/user/hypervisor/include/hippomocks.h:165:25: warning: thrown exception type is not nothrow copy constructible [cert-err60-cpp] void rethrow() { throw exception; }

/home/user/hypervisor/include/hippomocks.h:4251:10: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [cert-err61-cpp] catch(BASE_EXCEPTION e)

The second one I was able to fix pretty easy, and the error is correct in that, exceptions should be cause by reference (i.e. the code should be BASE_EXCEPTION &e).

The first one however I was not sure how to address. It's complaining that a copy of the exception is being made, and the copy constructor is not marked nothrow. The only thing that came to mind would be to change this code to use a move constructor.

[dascandy]

You can't use a move constructor there, as it's the storage for an exception to be thrown for an ON_CALL that can run multiple times. It'll have to be a copy constructor - but then the error is on the user code not being copy-constructible, or probably in this case the test code. So the test code's copy constructor should not throw an exception. As far as I know this isn't generally true for standard exceptions either - std::runtime_error comes to mind - so why is clang-tidy complaining about it?

[rianquinn]

Right now I disabled this check as it's only occurring in hippomocks, but my guess is, it's complaining because if the copy constructors are not labeled "noexcept" (which I think most of them are not), a copy of the exception could throw an exception and it's detecting the fact that an exception is being thrown that is not coming from the construction of an exception, but rather a copy of one. In the case of hippomocks, storing an exception to be thrown later makes a lot of sense as this is a unit test, but it's note something you would normally do.

Another one I found, which I filed a bug for was it was complaining about functions being labeled virtual that were called from a constructor. Once again, this check makes sense as the derived class will never be called, but in most cases this is a false positive as you have to mark most functions as virtual if you want to mock them with hippomocks (or any other mocking library), so it seems to me the check is useless until the spec can address mocking engines.

[dascandy]

Many parts of C++ assume that the kind of mocking HippoMocks does cannot possibly exist and there is not even a discussion going on in the Reflection wg about this kind of mocking. Most parts of it are centered around data discoverability and cornercases in that area that are essentially a whole different set of tasks. I've got another project on Github (Rapscallion) that tries to solve that problem the other way around - instead of making your structs discoverable, just make them inherit from std::tuple or std::vector and auto-serialize those.

I can mostly understand the copy-constructor of an exception should never throw argument, but it does not even hold for the standard's own exceptions so it feels like a warning that will give many false-positives.

[dascandy]

I've fixed the problem in the BASE_EXCEPTION catch (by removing that code) in the cpp11 branch. Is this usable for you?

[dascandy]

Added the ability to register a throw functor instead of the object itself. The functor can be copied and repeated, and the exceptions are created new every time in the functor. This allows you to avoid the original problem. It's a bit easier to use than .Do would be, as you don't get the arguments in this handler, so []{ throw 42; } will work.