!!! BREAKING CHANGE !!! - catch exceptions as const reference in <LEVEL>_THROWS_AS

[a4z]

catch exceptions as const reference,
static code analyzers do not like it different, this is also according to all C++ guidelines.
(MFC might make it different, but the exception from the rule should not be generalize to all use cases)
CHECK_THROWS_AS (foo, const ExceptionType&) ;
is definitly not as nice and expressive as
CHECK_THROWS_AS (foo, ExceptionType) ;

[onqtam]

hmmm - both boost.test and googletest add const& to the exception type, but Catch does not.
I couldn't think of any situation where I would want to catch by value...

I'll have to think about this - I'm not yet comfortable to change this since it would be a breaking change.

[a4z]

than catch is broken and google and boost test not.
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#e15-catch-exceptions-from-a-hierarchy-by-reference

[onqtam]

I decided to merge this - it's in the dev branch - but I cannot say when it'll go in master...

[martinmoene]

@a4z @onqtam @horenmar Just to have the information at hand here.

Please note that Catch wants or wanted to provide flexibility, that is, the behaviour is intended and not inherently broken per se. The doc explains: Expects that an exception of the specified type is thrown during evaluation of the expression. Note that the exception type is used verbatim and you should include (const) reference.

[onqtam]

I just looked at my docs and remembered making this decision explicitly in the past... but now I got swayed over to the other camp... I really cannot decide what the right way to go is...

[martinmoene]

... Connecting to Phil.

[a4z]

I apologize for using the word broken for catch, it was maybe to strong? (I am not native English speaker)
I just wanted express that I think that boost and google-test do it right, the default case should not handle the edge cases, and that hidden (ok its in the doc :-) calling a copy constructor of the exception is not nice, since without a static analyzer I would never have noticed this.

are there test cases for the edge case where defaulting const& would break things?

[martinmoene]

@a4z Catch's catch may be 'broken' in the view on the world of Boost and Google Test ;) . Catch just took another stance wanting to give its users freedom to and also expecting them to give the complete expression to catch. That's all there's to it.

[chambm]

Freedom to shoot yourself in the foot is what C++ is all about, right?

[a4z]

well, @martinmoene , I see i like this, prom a pragmatic view: Sitting with 20 embedded devs, 7 hate the 21 century, 7 don't care as long as nothing changes, 3 would like to move forward but have already resigned , and the rest would like to introduce todays development practices. like meaning full unit tests and BDD testing.
The requirement of writing const & self, is absolute counterproductive in this environment, and I understand that. This is my pragmatic view.

But however, adding a patch to my package build is also a solution, and what whatever argument s used to continue with this obviously less intuitive and born out of a workaround way, it do not have to follow it anyway ;-)
And than see what happens, there have been some promising sounding words for future versions of Catch anyway, maybe doctest will lead, of follow, it will be interesting to observe