NCAR/nidas

what to do about throw() and noexcept

Opened this issue · 0 comments

In the merge of the remove-exception-specifiers branch, all of the dynamic exception specifiers with exceptions were removed. Those specifiers are deprecated in C++11, so they needed to be removed. Some of the empty exception specifiers, as in throw(), were removed also, but many more empty exception specifiers remain. Those throw() specifiers are still valid, and the intent is the same as the newer noexcept(true), but noexcept is now the standard and allows for more optimization by the compiler.

Gordon started a discussion about the value of throw() and noexcept(true) here:

043d2e1#commitcomment-55232414

We can leave the remaining throw() specifiers without harm, but I'm still tempted to remove them since I think the old specifiers may confuse and restrict the API more than help it. And what should be done about the ones that have already been removed, like for process()? Do any need to be added back?

One of the tips in Meyers is to Declare functions noexcept if they won't emit exceptions. The section goes into how that helps with optimization and maybe a little with clarifying intention. On the other hand, it also states that most functions are exception-neutral, and functions should not be declared noexcept unless they are certain to never need to throw an exception.

From @maclean's comments:

As I understand things, if a method that is declared noexcept(true), throws an exception, then terminate() is called, basically like an assert. That might actually be what we want, (forcing us to fix the issue). Otherwise we might need to add exception handling code to where process() is called in order to avoid confusing error situations. That code might be required in quite a few places - though I haven't looked into it.

If a process() method runs into an unrecoverable error, where it needs to call terminate(), then of course it should just call terminate() instead of throwing from a function declared noexcept.

I guess what I'm talking about is semantics, rather than optimization. It's nice to see noexcept(true) in the contract of a function that you want to use, indicating that you don't have to worry about exceptions and any complicated error handling. So keeping many of the throw() (but converting to noexcept) seems good.

Yes, if noexcept is part of the signature, that is one way to remind to programmers that the implementation is not supposed to throw, but other than that it does not help the programmer. Unlike for Java, the C++ compiler does not guarantee that the implementation will not throw, whereas declaring noexcept in a base class means no implementation can ever throw. If someday a sensor wants to throw in process(), then noexcept(true) in the base class signature prevents that. If a caller does not want to worry about exceptions or complicated error handling, it does not need to, whether or not the function it calls is noexcept. It's not a problem for an exception to be passed up the call stack. If the exception is not caught, it will be clear where the exception originated and easy enough to get the call stack with a debugger, same as if terminate() was called inside process().

My understanding of noexcept is that it is either an optimization of the call stack, or it is a way to identify which methods can be called safely while an object is transitioning between states, to guarantee the object cannot be left in an invalid state. That in turn is primarily to enable move optimizations. Thus the necessity of noexcept for move operations and destructors, where the operation must complete without exceptions or else the object might exist in two places or in indeterminate states. If the primary purpose of noexcept is to allow optimizations, then I'm not sure it belongs in the NIDAS APIs where such optimizations are not a concern.

In the end, isn't there enough confusion and complication about this (revised) language feature that we shouldn't use it until we really know we need it? Obviously it will be helpful to document the potential exceptions from a method; that's part of it's interface, like the return value. Callers do not need to worry about catching exceptions unless they know a function throws an exception they can handle, and programmers don't need to bother with maintaining null exception specifiers. I suspect that in practice it will never matter if process() and other methods are noexcept or not, so is it really worth maintaining them that way?

Any other opinions on what to do about throw() and noexcept?

Since I tried to do a quick survey, here's an excerpt of functions that were declared throw(), but I'm not sure exactly which of them had specifiers removed in the merge. Probably it would be much more expedient at this point to remove the rest than add some back...

void LooperClient::looperNotify() throw();
void DSMEngine::killXmlRpcThread() throw();
void DSMSensor::printStatus(std::ostream& ostr) throw();
void DSMSensor::printStatusTrailer(std::ostream& ostr) throw();
void DSMSensor::printStatusHeader(std::ostream& ostr) throw();
bool DSMSensor::receive(const Sample *samp) throw();
void DSMSensor::distributeRaw(const Sample* s) throw();
void DSMSensor::removeSampleTag(SampleTag* val) throw();
void DSMSensor::removeSampleTag(const SampleTag*) throw();
bool DSMSensor::process(const Sample*,std::list<const Sample*>& result) throw();
void LamsSensor::derivedDataNotify(const nidas::core::DerivedDataReader * s) throw();
void SampleClient::flush() throw();
bool SampleClient::receive() throw();
void SampleIOProcessor::disconnectSource(SampleSource*) throw();
void SampleIOProcessor::printStatus(std::ostream&,float,int&) throw();
void SampleIOProcessor::init(dsm_time_t) throw();
void DSMServerApp::killStatusThread() throw();
void SensorHandler::add(DSMSensor* sensor) throw();
void SensorHandler::remove(PolledDSMSensor* psensor) throw();
void SensorHandler::scheduleAdd(RemoteSerialConnection* conn) throw();
void SensorHandler::scheduleClose(RemoteSerialConnection * conn) throw();
void SensorHandler::add(RemoteSerialConnection* conn) throw();
void SensorHandler::remove(RemoteSerialConnection* conn) throw();
void printChronyHeader(std::ostream& ostr) throw();
void printChronyTrailer(std::ostream& ostr) throw();
int AsciiScanf::Sscanf(const char* input, float* output, int nout) throw();
size_t IOStream::readBuf(void* buf, size_t len) throw();
size_t IOStream::backup() throw();
size_t IOStream::backup(size_t len) throw();
void SampleInputStream::distribute(const nidas::core::Sample* s) throw();
Sample* SampleInputStream::nextSample() throw();
Sample* SampleInputStream::sampleFromHeader() throw();