Default case handling in exception_ptr missing clue for coverity to treat switch fall-through as intentional

[James E. King, III <jim.king@…>]

This is not fall through but undefined behavior as indicated by the use of BOOST_ASSERT. Adding the fall through comment is incorrect and would be misleading.

[James E. King, III <jim.king@…>]

Is BOOST_ASSERT in every build or it is only in debug builds?

If the next line is never reached, there's probably a coverity comment hint to indicate that too like the old /* NOTREACHED */ for lint.

If it does not assert in a release build then it falls through and perhaps an exception would be a better choice so that it does not behave differently in debug vs. release?

Thanks.

[Emil Dotchevski]

In release or in debug, reaching that code indicates a logic error (bug). I'm not against tagging it as "NOTREACHED" if that helps silence automatic code analyzers, but "fall through" would mislead the reader that fall through is the intended behavior. It is not; if the default is reached, I have a bug.

(Throwing an exception is not a good choice for indicating logic errors.)

[James E. King, III <jim.king@…>]

Given BOOST_ASSERT is supposed to have the same characteristics as assert() that means it only happens in debug mode, and this means in release mode builds that is a fall-through. I find that more often than not Coverity is telling the truth; I could easily have marked this as intentional in a third party but I wanted to raise it. Whatever you decide is fine, it's your code. :)