Iox #27 add typed client and server api [stacked PR #5]

[elBoberido]

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 messages are signed (git commit -s)
   • 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. Assign PR to reviewer

Notes for Reviewer

This PR adds the typed API. The functionality is complete.

Test will be added in PR #1140

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
  • Each unit test case has a unique UUID
• Public API changes are documented via doxygen
• Copyright owner are updated in the changed files
• PR title describes the changes

Post-review Checklist for the PR Author

1. All open points are addressed and tracked via issues

References

• Relates to Request/Response communication with iceoryx #27

[elfenpiff]

You have here and in ClientImpl some code duplication. What about introducing a HeaderConcept like:

template <typename T>
struct HeaderConcept {
    static_assert(!std::is_void<T>::value, "Must not be void");
    static_assert(!std::is_const<T>::value, "Must not be const");
    static_assert(!std::is_reference<T>::value, "Must not be a reference");
    static_assert(!std::is_pointer<T>::value, "Must not be a pointer");
    using Evaluate = void; // required and has to be used with typename HeaderConcept<int>::Evaluate otherwise the compiler ignores the static_assert's
};

And use it like:

template <typename Req, typename Res, typename BaseServerT = int,
          typename RequiresForRequest = typename HeaderConcept<Req>::Evaluate,
          typename RequiresForResponse = typename HeaderConcept<Res>::Evaluate>
class ServerImpl {};

Then one would see that a header concept is required (looks a bit like C++20), if something changes we have only one piece of code we have to adjust and not 4 different pieces.

[MatthiasKillat]

@elfenpiff Looks interesting. Can be extended with checks for specific members etc. in the concept should this be needed (with SFINAE). I just do not like the need to introduce artificial template args for this (as with other meta-programming e.g. enable_if) which are not really supposed to be used apart from the default values.

I think it is possible/better to transfer the aggregated condition value to some internal bool, i.e. basically create a type trait for headers. Then we can static_assert on this as before, like
static_assert(HeaderConcept<MyHeader>::value, "concept not satisfied");
I leave out the details, but its basically a constexpr bool value = ... on your conditions above.

Then we do not have artificial template args.

[elBoberido]

@elfenpiff this might be tricky since the asserts have subtle variations

[elBoberido]

@elfenpiff after a closer look, forget what I wrote

[elBoberido]

@elfenpiff I agree with @MatthiasKillat to not add additional template args so I adjusted your proposal a bit.

[MatthiasKillat]

This works too to avoid template args (and we keep the fine grained errors of the static asserts).

[codecov]

Codecov Report

Merging #1089 (a0310be) into master (26c7310) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1089      +/-   ##
==========================================
+ Coverage   76.19%   76.21%   +0.01%     
==========================================
  Files         346      346              
  Lines       13595    13595              
  Branches     1938     1938              
==========================================
+ Hits        10359    10361       +2     
+ Misses       2604     2602       -2     
  Partials      632      632              

Flag	Coverage Δ
unittests	75.47% <ø> (+0.01%)	⬆️
unittests_timing	12.00% <ø> (-0.02%)	⬇️

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

Impacted Files	Coverage Δ
...eoryx_posh/include/iceoryx_posh/popo/publisher.hpp	100.00% <ø> (ø)
...nternal/relocatable_pointer/pointer_repository.inl	86.36% <0.00%> (-2.28%)	⬇️
iceoryx_hoofs/source/concurrent/loffli.cpp	88.57% <0.00%> (+8.57%)	⬆️