Time to switch to python3
What are the current users of ITK+python2? Maybe we should have a discussion about dropping C++03 in a new thread?
I fully support dropping the old compilers and C++03 with a change of the major release number.
Mostly because Python 2 is still the default on current stable Linux distributions.
The scientific Python community has made some noise about timelines for dropping support for Python 2 recently, and we may want to keep NumPy’s timeline, in mind, for instance.
Back to noexcept and nothrow, @Matt thank you for sharing that informative link! I didn’t know that noexcept
was a function specifier AND also an operator. The may be a defining difference between the two macros.
There may be places where this exception specification could be remove because it is not beneficial. Here is a patch I contributed related to the ( Looks like I introduced ITK_NOEXCEPT, as for c++03/C++11, then it changed):
https://github.com/InsightSoftwareConsortium/ITK/commit/06f8ef0175d901ec5d23197efb4f9044c1b62ab3
https://github.com/InsightSoftwareConsortium/ITK/commit/cf589ecb34f9dde6bbd5e5779f8ef12516f5d344
From the second patch, it is shown that I was working on ensuring that the execution of the delete event was exception safe. A tricky situation to track down, and ensure defined behavior during deconstruction!
Good stuff, the standard is confusing indeed, but that link is gold @matt.mccormick.
So, sharing a wrapping up from the link and cppreference.
- Since c++17:
throw()
andnoexcept
, (i.e, the specifier:noexcept(true)
) are exactly the same. - Pre c++17,
throw()
callsstd::unexpected
instead ofstd::terminate
if the function generates a exception.throw
specifier is deprecated since c++11.
Alsonoexcept
tagged functions can be checked at compile time with thenoexcept
operator:noexcept(myfunction)
.
Note the reason of this post: the dynamic exception specification: throw(type_id, type_id,…) is deprecated since c++11 and has nonoexcept
equivalent.
From: noexcept specifier (since C++11) - cppreference.com
noexcept is an improved version of throw(), which is deprecated in C++11. Unlike pre-C++17 throw(), noexcept will not call std::unexpected and may or may not unwind the stack, which potentially allows the compiler to implement noexcept without the runtime overhead of throw().
- c++03: there is no
noexcept
--specifier or operator–, we use thethrow()
specifier.
Proposition:
Remove deprecate ITK_NOEXCEPT_OR_THROW
and use a modified ITK_NOEXCEPT
:
From itk_compiler_detection: ITK_NOEXCEPT_EXPR(X)
is not used anywhere in ITK (after a fast grep), there is no similar functionality to the noexcept operator
in c++03. And the valid specifier with X=false
: noexcept(false)
is already the default for every function.
Also ITK_NOEXCEPT
does nothing right know in old compilers, when throw()
could be used instead
# if ITK_COMPILER_CXX_NOEXCEPT
# define ITK_NOEXCEPT noexcept
# define ITK_NOEXCEPT_EXPR(X) noexcept(X)
# else
# define ITK_NOEXCEPT
# define ITK_NOEXCEPT_EXPR(X)
# endif
I propose to deprecate ITK_NOEXCEPT_EXPR as well, and directly use the operator noexcept(expr)
when c++11 is allowed. Right now it can generate invalid code for c++03. From:
if( ITK_NOEXCEPT_EXPR(no_throw_function()) )
/* valid c++11:
* if( noexcept(no_throw_function()) ) // true
* invalid c++03
* if()
*/
So rename duplicate the macro from itkMacro.h, to deal with the noexcept
specifier.
#if ITK_COMPILER_CXX_NOEXCEPT
#define ITK_NOEXCEPT noexcept
#else
#define ITK_NOEXCEPT throw()
#endif
Edited:
Change ITK_NOEXCEPT to have same behavior than ITK_NOEXCEPT_OR_THROW, and deprecate the latter.
Remove ITK_NOEXCEPT from almost everywhere (destructors included), except low-level function such as Swap
, Unregister
etc.
@phcerdan thanks for taking a stab at a patch
However, from a higher level, to summarize:
- Compilers can do static inspection of the code to determine when exceptions will not be thrown, and they will optimize the binary accordingly.
- It is difficult for a programmer to accurately inspect code and determine whether exceptions will be thrown or not.
- Due to compilers needing to handle the above along with legacy compiler implementation details and the details of how
noexcept
has evolved, use ofnoexcept
generally can result in a performance hit instead of a performance gain. - We should just not use
noexcept
. - Let’s remove application of
ITK_NOEXCEPT
andITK_COMPILER_OR_THROW
and make a comment above these macros that they are no longer used.
I believe that some of the uses can be removed.
However, I think ITK_NOEXPECT_OR_THROW
may have been added to itk exception classes because the C++03, throw()
specifier was required, or to suppress a warning. So there is likely a reason why they are there. The specifiers for the exception may be able to be remoted when C++03 support is dropped.
I think marking these core itk object related to reference counting, and command call backs as noexcept
is important to ensure that our destructors do not emit exceptions. It is my understanding that keeping these specifiers will help enable compiler to emit errors or warnings when these fundamental requirement for compliant destructors is violated.
My suggestions:
- When support for C++03 is dropped, remove ITK_NOEXCEPT_OR_THROW from itk exception objects
- Keep exception specification for core itk objects, LightObject, SmartPointer ( could be convinced otherwise if there are conclusive performance differences across compilers )
- I think the specification for the VariableLengthVector, and in itk::mpl could be removed now
From the link that Matt shared:
We said there are two situations where functions do not throw exceptions. The second one is when we intend our function to provide a no-fail guarantee or a no-throw guarantee. The difference between the two guarantees is not trivial. A no-fail guarantee indicates that the function will always succeed in doing what it is supposed to. Such functions are needed by other functions to provide a strong (commit-or-rollback) guarantee. A no-throw guarantee is weaker. It indicates that in case the function fails to do its job, it must not throw exceptions: it might use an error code or some other technique for signalling error, or it may just conceal the fact that it failed. This guarantee is required of functions that are called during stack unwinding where some exception is already being handled and throwing a second one makes the unwinding process to fail. These functions include all destructors, constructors of objects representing exceptions, an conversion operators that return exceptions.
It sounds like objects representing exceptions could also benefit of keeping these keywords.
Looking at this link:
This is confirmations that classes derived from C++03 std::exception require throw() specification.
Then let’s keep throw()
around for C++03.
It is not necessary to keep the keywords. C++11 defaults to noexcept
when possible. It is better to follow the recommendation of the article:
First and most important one, if you are concerned about reduced performance, or about the risk of calling std::terminate, or you are just not sure about the new feature, or if you have doubts whether you should make your function noexcept or not, then just do not annotate your functions with noexcept at all.
I have been burned by lack of exception safety in deconstructs of ITK before. The no exception specifiers are important.
Exception safety for destructors are important. noexcept
is the default for C++11.
That makes sense, but quoting the cpp-coreguidelines:
noexcept is most useful (and most clearly correct) for frequently used, low-level functions.
Note:
Destructors, swap functions, move operations, and default constructors should never throw.
Also check the example in this issue, relevant if using external libraries with throwing destructors:
Not all destructors are noexcept by default; one throwing member poisons the whole class hierarchy
I will remove the ITK_NOEXCEPT_OR_THROW from all the destructors and see what happens, but what should we do about the low level functions?
For example itkLightObject::Unregister
Right now in the codebase, ITK_NOEXCEPT
expands to noexpect
(which is a low-level function used frequently) in c++11, and to nothing in c++03!
Not good, as @blowekamp was saying:
The gcc
error about it in c++03:
error: looser throw specifier for 'virtual itk::ExceptionObject::~ExceptionObject()'
I followerd @blowekamp suggestions:
Keep:
-
Child classes of
std::exception
need the destructor specified asthrow()
. In c++11 this is not needed, it is specified by default. This is the most common case in the codebase, and it can be easily removed when c++11 is required. -
Core methods of core objects: itkLightObject::UnRegister, itkSmartPointer, etc. This follows good usage from cpp-coreguidelines as well.
Remove:
- ITK_NOEXCEPT from regular functions, for example in itkVariableLengthVector and itkBinaryOperationConcept.h. I haven’t found matches in
itk::mpl
So, basically, I haven’t touch much, beyond the name change. ITK_NOEXCEPT is already defined outside of itkMacro.h, but it was doing nothing in c++03. I added throw()
in that case. VNL source code defined the same macro and I opened a PR there: COMP: Fix VXL_NOEXCEPT macro. by phcerdan · Pull Request #419 · vxl/vxl · GitHub.
Hi.
Why ?
None of the operations flagged as noexcept in itkVariableLengthVector, or in itkBinaryOperationConcept, throw anything. BTW, it matches std::vector<> interface where non-throwing operations are flagged noexcept and CppCoreGuidelines, and many other advices we can found (vittorio romeo's website …)
In all cases if you really want to remove the specifier, it still needs to stay in swap(), and in move operations.
According to a long time developer of the Visual Studio compiler backend, adding noexcept
can negatively impact performance:
Current implementation is not as poor, but still unintuitively is a net performance loss – especially on x86
Slide 18, the author recommends to use /EHsc
, which is equivalent to flag all C functions as noexcept
.
I understand it could have negative impacts in functions that call other functions that aren’t flagged as noexcept. In VLV case, noexcept is used in leaf functions and other inlined functions that call noexcept functions.
Before removing it, has anyone benchmarked any difference? Or maybe checked the difference on godbolt?
What’s quite sure, if you remove the noexcept
from move operations, inserting VLV in std::vectors, resizing vectors of VLV, etc. would trigger copies instead of moving the VLV.
In that case noexcept is required. This will be a net performance loss for sure.
Yeah, kept ITK_NOEXCEPT in VLV in swap functions, move constructor and assignment. It indeed gives the opportunity to big optimizations choosing different algorithms.
In line with the discussion: c++ - When should I really use noexcept? - Stack Overflow
The second point, and this is only intuition on my side.
noexcept
is required to call a function if an exception leaks. If the destructor, swap, operator=(&&), and the move-constructor call non-inlined functions that aren’t marked as noexcept
, I don’t see how the compiler could be able to get rid of the exception-leak check.
Also, I fail to see why the compiler would introduce a check in noexcept
functions where it’s clear nothing would throw.
That’s why I did mark all the leaf functions and intermediary functions as noexcept
, which follows the guideline on C++CoreGuideline & all that recommend to flag noexcept
functions that’ll never throw. This which is on par with the /EHsc
option that tell the compiler C functions are noexcept
.
Before undergoing this cleaning action, it would be interesting to check the difference in the assembly produced by MSVC, GCC, clang, and to measure the processing speed of a resampling of vector image with and without noexcept.
Benchmarking to experimentally see what, if any, performance differences occur in practice. I am currently swamped, but I will add vector image resampling test to the example benchmarks when I have a chance and see what we get.