eclipse-iceoryx / iceoryx

Eclipse iceoryx™ - true zero-copy inter-process-communication
https://iceoryx.io
Apache License 2.0
1.6k stars 373 forks source link

Only use noexcept in functions that definitely cannot throw #1392

Open MatthiasKillat opened 2 years ago

MatthiasKillat commented 2 years ago

Brief feature description

Currently almost all functions are marked as noexcept. This has severe implications: every function that would throw an exception will terminate when the exception tries to leave the function (it is still generated in this function or lower in the call stack).

The goal is to only mark functions as noexcept if we can guarantee that they cannot throw under any circumstances. This is generally not trivial, but feasible due to iceoryx design and Error Handling philosophy.

  1. iceoryx itself never throws directly
  2. Linked 3rd party libraries will not throw across library boundaries. We also do not use any lib that can internally throw (this needs to be checked), at least we should not if the goal is to not use dynamic memory at all.
  3. C functions (e.g. POSIX calls) will never throw
  4. We only use header-only libraries (mainly STL) that cannot throw (in our use cases)

There are only two kinds of functions: those that could throw, and those that cannot. It is sometimes hard to classify them correctly, as this will depend on the implementation.

We keep the noexcept for functions where we are sure that they can never throw. In case where we cannot be sure we should not mark the function with noexcept as it is better to err in this way.

If a deployment that uses noexcept everywhere is required we can use the noexcept compile flag (is this portable?) and basically arrive at the current behavior.

Detailed information

  1. We have to be careful for any generic code, such as vector<T> where e.g. emplace can throw if the constructor of T throws (which it might, we cannot restrict T to non-throwing types within the C++ language). On the other hand functions like size will never throw.
  2. Types like string are less generic and should mostly not throw but we have to be careful since we use STL constructs internally (e.g. std::min<T>). We need to ensure that those cannot throw, which in this case it cannot for the unsigned integer data types we use it with (whether e.g. std::min can throw depends on whether the operator< implementation of T can throw).
  3. All constructs that involve hook functions that can be defined by client code, most prominently cxx::function, can in principle throw (depends on the wrapped function) and hence cannot be noexcept.
  4. All posix wrappers should never throw as they only use C library calls and non-throwing STL code.
  5. Iceryx code itself cannot contain any throw directive.
  6. All iceoryx containers and constructs (like the vector, optional, expected, list, stack - list not complete) must be at least weak exception safe. At the moment those containers are corrupted when a move assignment operator throws an exception. The same goes also for copy operations.

Based on this it should be possible to correctly identify all non-throwing functions and mark them with noexcept.

Interface implications

noexcept is only part of the signature for e.g. overloading from C++17 onward. Nevertheless functions that previously would terminate if a exception is thrown (by user code) will now propagate the exception and hence it must be considered API breaking (as it changes observable semantics).

The other way can currently not happen (well, almost since some functions are not noexcept currently), i.e. a function that was not noexcept becomes noexcept now. This would be similarly severe since a user might prepare to catch some exception outside of the function (thrown again by user code), but instead it would terminate now. This again changes the semantics and is hence a breaking change.

MatthiasKillat commented 2 years ago

This is incomplete and requires agreement about the overall strategy before we proceed. It is, however a separate issue from https://github.com/eclipse-iceoryx/iceoryx/issues/1032 which is concerned with the ErrorHandler.

If we agree on which kind of functions should be noexcept and which should not (as e.g. outlined in the issue), it should be systematically possible to tag them correctly. There may be special cases not listed above which likely apply for a few functions only. Unfortunately, I do not think this can be automated.

mossmaurice commented 2 years ago

@MatthiasKillat Thanks for opening this issue! Based on this custom clang-tidy rule I might be able to parse the implementation of the methods and check the method calls for a noexcept keyword. Not 100% if this will work, though.