iox-1751 Add equality operators for expected and variant

[mossmaurice]

Pre-Review Checklist for the PR Author

1. Code follows the coding style of CONTRIBUTING.md
2. Tests follow the best practice for testing
3. Changelog updated in the unreleased section including API breaking changes
4. Branch follows the naming format (iox-123-this-is-a-branch)
5. Commits messages are according to this guideline
   • Commit messages have the issue ID (iox-#123 commit text)
   • Commit author matches Eclipse Contributor Agreement (and ECA is signed)
6. Update the PR title
   • Follow the same conventions as for commit messages
   • Link to the relevant issue
7. Relevant issues are linked
8. Add sensible notes for the reviewer
9. All checks have passed (except task-list-completed)
10. All touched (C/C++) source code files from iceoryx_hoofs are added to ./clang-tidy-diff-scans.txt
11. Assign PR to reviewer

Notes for Reviewer

• Add equality operators for expected and variant

Checklist for the PR Reviewer

• Commits are properly organized and messages are according to the guideline
• Code according to our coding style and naming conventions
• Unit tests have been written for new behavior
• Public API changes are documented via doxygen
• Copyright owner are updated in the changed files
• All touched (C/C++) source code files from iceoryx_hoofs have been added to ./clang-tidy-diff-scans.txt
• PR title describes the changes

Post-review Checklist for the PR Author

1. All open points are addressed and tracked via issues

References

• Closes Add equality operators to cxx::variant and cxx::expected #1751

[codecov]

Codecov Report

Merging #1757 (0956f1c) into master (3aecfb8) will decrease coverage by 0.04%.
The diff coverage is 60.97%.

@@            Coverage Diff             @@
##           master    #1757      +/-   ##
==========================================
- Coverage   77.39%   77.35%   -0.05%     
==========================================
  Files         366      366              
  Lines       14196    14230      +34     
  Branches     1983     1991       +8     
==========================================
+ Hits        10987    11007      +20     
- Misses       2583     2591       +8     
- Partials      626      632       +6     

Flag	Coverage Δ
unittests	77.02% <60.97%> (-0.03%)	⬇️
unittests_timing	15.65% <0.00%> (-0.06%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...eoryx_hoofs/include/iceoryx_hoofs/cxx/expected.hpp	100.00% <ø> (ø)
...ceoryx_hoofs/include/iceoryx_hoofs/cxx/variant.hpp	100.00% <ø> (ø)
...clude/iceoryx_hoofs/internal/cxx/variant_queue.inl	77.38% <0.00%> (-0.94%)	⬇️
...de/iceoryx_hoofs/internal/cxx/variant_internal.hpp	69.49% <20.00%> (-8.51%)	⬇️
...fs/include/iceoryx_hoofs/internal/cxx/expected.inl	97.08% <87.50%> (-1.27%)	⬇️
...ofs/include/iceoryx_hoofs/internal/cxx/variant.inl	96.11% <88.88%> (-0.73%)	⬇️
iceoryx_posh/source/popo/listener.cpp	87.17% <0.00%> (-5.13%)	⬇️
...oofs/source/log/building_blocks/console_logger.cpp	77.61% <0.00%> (-1.50%)	⬇️
... and 2 more

[elfenpiff]

Content-wise this PR makes sense but why do we add the comparison feature? Autosar states that when one implements comparison operators one should implement them as free functions. But there is nowhere stated that they have to be implemented at all.

So why add them now?

[mossmaurice]

Content-wise this PR makes sense but why do we add the comparison feature? Autosar states that when one implements comparison operators one should implement them as free functions. But there is nowhere stated that they have to be implemented at all.
So why add them now?

@elfenpiff You are right that we currently don't use these operators in iceoryx_posh. However, iceoryx_hoofs is a general purpose library and I would expect the equality operators from such a library. The STL also implements those for most of their types and with iceoryx_hoofs we try to be as close to the STL as possible. So I decided to add them after this point popped when performing a Software FMEA.

[elfenpiff]

I think you forgot noexcept here and in the other operator implementations.

[mossmaurice]

I think equality operators can't be noexcept in this case as we call the equality operator of an underlying type inside the expected. Also in case of the variant the if (N != index) it could lead to an Expects call which could throw in future if configured that way by the user.