Skip to content

Commit

Permalink
iox-eclipse-iceoryx#2080 Prefix Expects and Ensures with IOX
Browse files Browse the repository at this point in the history
  • Loading branch information
elBoberido committed Nov 8, 2023
1 parent bf42822 commit 05c4f4b
Show file tree
Hide file tree
Showing 77 changed files with 478 additions and 484 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ codebase follows these rules, things are work in progress.
which are not compatible with the STL (e.g. `iox::cxx::vector::emplace_back()` does return a bool); see
[section](CONTRIBUTING.md#external-dependencies) below
7) **Always use `iox::log::Logger`**, instead of `printf()`
8) **Always use `iox::ErrorHandler` or `iox::cxx::Expects`/`iox::cxx::Ensures`**, when an error occurs that cannot or
8) **Always use `iox::ErrorHandler` or `IOX_EXPECTS`/`IOX_ENSURES`**, when an error occurs that cannot or
shall not be propagated via an `iox::expected`
9) **Not more than two-level nested namespaces**, three-level nested namespace can be used sparsely

Expand Down
36 changes: 18 additions & 18 deletions doc/design/error-handling.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,13 @@ Currently, error codes are used to identify the location of an error. These are

Each module has it's own range of errors. The ranges are defined `error_handler.hpp` and the error codes in `error_handling.hpp` for each module. More details in the API reference.

### `Expects` and `Ensures`
### `IOX_EXPECTS` and `IOX_ENSURES`

These assert-like constructs are used to document assumptions in the code which are checked (at least) in debug mode. Currently they are always active, i.e. also checked in release mode. If the condition is violated they print the condition, the location of occurrence in the code and terminate the program execution.

Since they are not necessarily active in release mode, they cannot be used to handle errors. Their purpose is to detect misuse or bugs of the API early in debug mode or to verify a result of an algorithm before returning. In this way, assumptions of the developer are made explicit without causing overhead when not needed. Therefore errors to be caught by Expects and Ensures are considered bugs and need to be fixed or the underlying assumptions and algorithms changed. This is in contrast to errors which are expected to occur during runtime which are handled by the error handler (i.e. a system resource cannot be obtained).
Since they are not necessarily active in release mode, they cannot be used to handle errors. Their purpose is to detect misuse or bugs of the API early in debug mode or to verify a result of an algorithm before returning. In this way, assumptions of the developer are made explicit without causing overhead when not needed. Therefore errors to be caught by IOX_EXPECTS and IOX_ENSURES are considered bugs and need to be fixed or the underlying assumptions and algorithms changed. This is in contrast to errors which are expected to occur during runtime which are handled by the error handler (i.e. a system resource cannot be obtained).

Although Expects end Ensures behave the same, the former is used to signify a precondition (e.g. arguments of a function) is checked, while the latter indicates a postcondition check (e.g. result of a function before returning)
Although IOX_EXPECTS end IOX_ENSURES behave the same, the former is used to signify a precondition (e.g. arguments of a function) is checked, while the latter indicates a postcondition check (e.g. result of a function before returning)

Examples include expecting pointers that are not null (as input, intermediate or final result) or range checks of variables.

Expand All @@ -139,7 +139,7 @@ Examples include wrapping third party API functions that return error codes or o

Error logging shall be done by the logger only, no calls to ``std::cerr`` or similar should be performed.

All the methods presented (``iox::expected``, ``Expects`` and ``Ensures`` and the error handler) can be used in posh. The appropriate way depends on the type of error scenario (cf. the respective sections for examples). The error handler should be considered the last option.
All the methods presented (``iox::expected``, ``IOX_EXPECTS`` and ``IOX_ENSURES`` and the error handler) can be used in posh. The appropriate way depends on the type of error scenario (cf. the respective sections for examples). The error handler should be considered the last option.

### Error handling in hoofs

Expand All @@ -150,9 +150,9 @@ The error handler cannot be used in hoofs.

Whether it is appropriate to use ``iox::expected`` even if STL compatibility is broken by doing so depends on the circumstances and needs to be decided on a case-by-case basis. If the function has no STL counterpart ``iox::expected`` can be used freely to communicate potential failure to the caller.

It should be noted that since currently `Expects` and `Ensures` are active at release mode, prolific usage of these will incur a runtime cost. Since this is likely to change in the future, it is still advised to use them to document the developer's intentions.
It should be noted that since currently `IOX_EXPECTS` and `IOX_ENSURES` are active at release mode, prolific usage of these will incur a runtime cost. Since this is likely to change in the future, it is still advised to use them to document the developer's intentions.

Do not use `std::terminate` directly but use assert-like constructs such as ``Expects`` or ``Ensures``.
Do not use `std::terminate` directly but use assert-like constructs such as ``IOX_EXPECTS`` or ``IOX_ENSURES``.

### Interface for 3rd party code

Expand Down Expand Up @@ -191,34 +191,34 @@ errorHandler(Error::SOME_ERROR_CODE)
More information on how to setup and use the `errorHandler` can be found in the API reference.
### `Expects` and `Ensures`
### `IOX_EXPECTS` and `IOX_ENSURES`
Assume myAlgorithm is part of an inner API and not supposed to be called with a ``nullptr``. We may have used a reference here, this is just for illustration.
In addition the value pointed to is assumed to be in the range (-1024, 1024). While we could check this every time, this can be avoided if we specify that the caller is responsible to ensure that these conditions hold.
```cpp
int32_t myAlgorithm(int32_t* ptr) {
Expects(ptr!=nullptr);
IOX_EXPECTS(ptr!=nullptr);
//observe the order, we only dereference after the nullptr check
Expects(*ptr > -1024 && *ptr < 1024);
IOX_EXPECTS(*ptr > -1024 && *ptr < 1024);
int32_t intermediate = timesTwo(*ptr);
//this may not be necessary here to ensure that the next function call is valid,
//but it states our expectations clearly
Ensures(intermediate % 2 == 0);
IOX_ENSURES(intermediate % 2 == 0);
int32_t result = abs(intermediate);
Ensures(result % 2 == 0);
Ensures(result >= 0);
Ensures(result < 2048);
IOX_ENSURES(result % 2 == 0);
IOX_ENSURES(result >= 0);
IOX_ENSURES(result < 2048);
return result;
}
```

Note that in the case of ``nullptr`` checks it is also an option to use references in arguments (or ``iox::not_null`` if it is supposed to be stored since references are not copyable). It should be considered that ``iox::not_null`` incurs a runtime cost, which may be undesirable.
When Expects and Ensures are implemented to leave no trace in release mode, we do not incur a runtime cost using them. For this reason, it is advised to use them to document and verify assumptions where appropriate.
When IOX_EXPECTS and IOX_ENSURES are implemented to leave no trace in release mode, we do not incur a runtime cost using them. For this reason, it is advised to use them to document and verify assumptions where appropriate.

### `expected`

Expand Down Expand Up @@ -334,13 +334,13 @@ What is needed to have a limited stack-trace even in release mode?
### Debug vs. release mode
We need to further clarify behavior in release and debug mode of the error handler and ``Expects`` and ``Ensures`` (and maybe the logger as well). Can we have a release build with additional information? (e.g. symbols for a stack-trace).
We need to further clarify behavior in release and debug mode of the error handler and ``IOX_EXPECTS`` and ``IOX_ENSURES`` (and maybe the logger as well). Can we have a release build with additional information? (e.g. symbols for a stack-trace).
### Assert
Do we want an Assert in addition to ``Expects`` and ``Ensures``? If so, shall it possibly be active in release mode or only debug mode?
Do we want an Assert in addition to ``IOX_EXPECTS`` and ``IOX_ENSURES``? If so, shall it possibly be active in release mode or only debug mode?
In principle with a sufficiently powerful Assert or ``Expects`` (resp. ``Ensures``), this should not be needed (they are equivalent in their functionality).
In principle with a sufficiently powerful Assert or ``IOX_EXPECTS`` (resp. ``IOX_ENSURES``), this should not be needed (they are equivalent in their functionality).
## Future improvements
Expand All @@ -357,7 +357,7 @@ In this section we briefly describe ways to potentially improve or extend functi
1. Add file, line and function information (using ``__FILE__``, ``__LINE__`` and ``__func__``). This would require using macros for the error handler call in a future implementation.
1. If deactivation or reduced operation (e.g. not handling ``MODERATE`` errors) is desired, this partial deactivation should cause no (or at least very little) runtime overhead in the deactivated cases.
### `Expects` and `Ensures`
### `IOX_EXPECTS` and `IOX_ENSURES`
Allow deactivation in release mode, but it should still be possible to leave them active in release mode as well if desired. Deactivation in debug mode can also be considered but is less critical. Deactivation should eliminate all runtime overhead (i.e. condition evaluation).
Expand Down
64 changes: 32 additions & 32 deletions doc/design/error_reporting.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ This is intended as a quick guide through the new error reporting API that will

## Terminology

### Module
### Module

Self-contained part of code, e.g. hoofs or posh (or smaller scope). Each module must have a unique
id and defines its errors.
Expand All @@ -29,7 +29,7 @@ The part of the custom implementation that defines the actions on reported error

### Error Code

Some error code that is unique for each error module. It is recommended that it corresponds only
Some error code that is unique for each error module. It is recommended that it corresponds only
to the type of error (e.g. out-of-memory error) and not the location.

### Error
Expand All @@ -39,9 +39,9 @@ module (by its id).

## How It Works

The whole mechanism is very flexible and generic and relies on overloading and perfect forwarding
to relay information, e.g. errors. That means much of it can be optimized at compile time,
as the compiler has full knowledge about all the template definitions.
The whole mechanism is very flexible and generic and relies on overloading and perfect forwarding
to relay information, e.g. errors. That means much of it can be optimized at compile time,
as the compiler has full knowledge about all the template definitions.

This holds up to the custom implementation, where it is possible to e.g. use runtime polymorphism or simply link
against some library.
Expand Down Expand Up @@ -92,7 +92,7 @@ Assume the module defines some error codes

```cpp
enum class Code
{
{
OutOfMemory = 73,
OutOfBounds = 21
};
Expand Down Expand Up @@ -137,7 +137,7 @@ IOX_REPORT_FATAL(Code::OutOfMemory);
reports a fatal error that aborts execution after the custom implementation specific handler is invoked.
Decoupling the error and its category is intentional, as e.g. an `OutOfMemory` error may not always be
fatal. It may become fatal after it is propagated further along the call stack.
fatal. It may become fatal after it is propagated further along the call stack.
Propagation is possible by various means, e.g. return codes, monadic types or even exceptions (that
must be caught before reporting the error elsewhere again).
Expand Down Expand Up @@ -172,7 +172,7 @@ The condition is required to hold and this requirement is always checked.
If the condition does not hold, panic is invoked and the execution stops.

This should be used for conditions that may not hold on the correct path, e.g. for error cases.
It should not be used for assumptions that have to be true in correct code
It should not be used for assumptions that have to be true in correct code
(use `IOX_ASSUME` or `IOX_PRECONDITION` for this).

Note that no condition can generally be enforced in the sense that it must be true and no checking is required.
Expand Down Expand Up @@ -202,8 +202,8 @@ IOX_PRECONDITION(x>=0, "precondition violation message");
is used to verify assumptions **BEFORE** any logic in the function body is executed. Technically copy
constructors may run before any condition can be checked, and there is also the possibility of
reordering if the following code does not depend on the condition at all.
This is not a problem since any reordering is not allowed to affect the observable result.
reordering if the following code does not depend on the condition at all.
This is not a problem since any reordering is not allowed to affect the observable result.
Specifically it cannot affect the value of the precondition itself as this would change the
observable behaviour.
Expand Down Expand Up @@ -273,12 +273,12 @@ int handleColor(Color color) {
}
```

If `IOX_UNREACHABLE` is reached during execution, `panic` will be invoked and the program aborts.
Stating that specific code cannot be reached is a specific assumption and any violation
is considered a bug. Defensive programming, i.e. checking for conditions that are not supposed
If `IOX_UNREACHABLE` is reached during execution, `panic` will be invoked and the program aborts.
Stating that specific code cannot be reached is a specific assumption and any violation
is considered a bug. Defensive programming, i.e. checking for conditions that are not supposed
to happen in a correct implementation, naturally creates unreachable code.

Marking unreachable code like this has advantages for test coverage as the compiler and other tools
Marking unreachable code like this has advantages for test coverage as the compiler and other tools
that rely on the compiler are aware of the `noreturn` guarantee of `IOX_UNREACHABLE`.
As a consequence, branches with `IOX_UNREACHABLE` do not necessarily lead to a return statement.

Expand Down Expand Up @@ -314,22 +314,22 @@ expected<int, Code> algorithm(int x)
}

expected<int, E> identity() {

auto result = algorithm(73);

if(result.has_error())
{
// transform the error to E and propagate it
return into<detail::err<E>>(result.error());
}

// no error, return result
return ok(*result);
};
```
This is similar to exception handling without the convenience of propagation.
While this shows the use with `expected`, it can be used with any error return type,
While this shows the use with `expected`, it can be used with any error return type,
for example the error code itself.
A generalization allows to report more complex error types directly. This requires a corresponding
Expand Down Expand Up @@ -399,7 +399,7 @@ int algorithm(int x)
}
```

A generalization to error types other than codes is possible with a corresponding
A generalization to error types other than codes is possible with a corresponding
backend implementation.

## Structure
Expand All @@ -425,31 +425,31 @@ These are
8. `errors.hpp` : supported error types and related free functions

All the files focus on singular aspects to allow fine-grained inclusion.
All definitions have to reside in `iox::er`, which is considered a private (detail) namespace
All definitions have to reside in `iox::er`, which is considered a private (detail) namespace
for everything related to error reporting. Since the API uses macros, it has no namespace itself.

### Custom Implementation

A specific custom implementation may depend on any of them and has to implement an error reporting
interface `error_reporting.hpp`.
interface `error_reporting.hpp`.

Apart from implementing the error reporting interface, a custom implementation does not have to follow
a specific structure. However, it cannot depend on anything that intends to use the error
reporting itself. This especially is important if e.g. another communication mechanism such as a
Apart from implementing the error reporting interface, a custom implementation does not have to follow
a specific structure. However, it cannot depend on anything that intends to use the error
reporting itself. This especially is important if e.g. another communication mechanism such as a
socket is used to report the errors. In this case, the socket implementation cannot use the error
reporting as this would create a circular dependency.

A custom implementation can override or extend some definitions and it is encouraged to use the same
file names as in the mandatory basics. For example `custom/error_kind.hpp` specifies additional
A custom implementation can override or extend some definitions and it is encouraged to use the same
file names as in the mandatory basics. For example `custom/error_kind.hpp` specifies additional
error kinds (apart from the mandatory fatal errors).

The main purpose of the custom implementation is to define the actions to take for each error.
Extension of existing definitions is possible by either changing the default implementation
Extension of existing definitions is possible by either changing the default implementation
or providing an additional custom implementation and ensure that it is used by all modules.

### Default Implementation

The default implementation in `custom/default` allows switching between a `DefaultHandler`
The default implementation in `custom/default` allows switching between a `DefaultHandler`
and a `TestHandler` at runtime.
The latter is used in testing to verify that an error occurred when it is expected.

Expand All @@ -460,8 +460,8 @@ The default implementation does not depend on any code that uses the error repor
### Testing

All testing related definitions are located in `iceoryx_hoofs/testing/error_reporting`.
These are the definition of `TestErrorHandler` in `testing_error_handler.hpp` and auxiliary
functions in `testing_support.hpp` to be used in tests to verify errors.
These are the definition of `TestErrorHandler` in `testing_error_handler.hpp` and auxiliary
functions in `testing_support.hpp` to be used in tests to verify errors.
The latter can be extended as required.

### Modules
Expand All @@ -470,12 +470,12 @@ There must be a single point where all modules are defined to ensure they use un
same custom implementation. Currently this happens in the `modules` folder but is work in progress
to be completed during integration of error reporting.

There is `modules/hoofs/error_reporting.hpp` that defines all the errors and custom implementation
used by `iceoryx_hoofs`. This header includes `error_reporting_macros.hpp` to make it easy to use
There is `modules/hoofs/error_reporting.hpp` that defines all the errors and custom implementation
used by `iceoryx_hoofs`. This header includes `error_reporting_macros.hpp` to make it easy to use
the custom error reporting in any iceoryx hoofs file by including `modules/hoofs/error_reporting.hpp`.

Replacing the previous error handling is supposed to happen by

1. Adapting the error definitions for `iceoryx_hoofs` in `modules/hoofs/errors.hpp`
2. Introducing a similar folder structure for `iceoryx_posh`
3. Replacing occurrences of the previous error handler call (including `cxx::Expects`)
3. Replacing occurrences of the previous error handler call (including `IOX_EXPECTS`)
2 changes: 1 addition & 1 deletion doc/design/logging.md
Original file line number Diff line number Diff line change
Expand Up @@ -425,4 +425,4 @@ int main()
`Logger::customize().logLevelFromEnvOr(LogLevel::WARN).init()`
- wrap `__FILE__`, `__LINE__` and `__FUNCTION__` into a `source_location` struct
- where should this struct be placed
- could also be used by `Expects`, `Ensures`
- could also be used by `IOX_EXPECTS`, `IOX_ENSURES`
Loading

0 comments on commit 05c4f4b

Please sign in to comment.