Constrain the Property object at compile-time

[antonysigma]

Description

The property struct/class requires a public member function signature apply(hid_t) const. Describe the constrain in the C++20 concepts syntax. The primarily goal is to make compiler errors more readable for average users. Average users are those who do not understand SFINAE.

Resolves feature #810 .

How to test this?

It is a compile-time type checking. If the production code or test code compiles, the code is valid.

Test System

• OS: Ubuntu 20.04
• Compiler: clang 12.0 with C++20 and/or C++11
• Dependency versions: Irrelevant.

[codecov]

Codecov Report

Merging #811 (7f14fdb) into master (3f005dc) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #811   +/-   ##
=======================================
  Coverage   84.58%   84.58%           
=======================================
  Files          68       68           
  Lines        4781     4781           
=======================================
  Hits         4044     4044           
  Misses        737      737           

Files Changed	Coverage Δ
include/highfive/H5PropertyList.hpp	93.75% <ø> (ø)
include/highfive/bits/H5PropertyList_misc.hpp	76.43% <ø> (ø)

[antonysigma]

Hi HighFive project owners @alkino . I noticed that the PR passes all CI validations except clang-format. Shall I revise and resubmit the PR? I know how to run clang-format locally. Just wanna be certain about who does what, before I proceed.

[1uc]

Thank you for the nice PR. Yes, please feel free to fix the formatting; I think the version of clang-format you need is 14.0.0. Otherwise the required diff to appease CI is:

 diff --git a/include/highfive/H5PropertyList.hpp b/include/highfive/H5PropertyList.hpp
index af149a9..241c988 100644
--- a/include/highfive/H5PropertyList.hpp
+++ b/include/highfive/H5PropertyList.hpp
@@ -143,10 +143,9 @@ class PropertyListBase: public Object {
 ///
 /// \cond
 #if __cplusplus >= 202002L
-template<typename P>
-concept PropertyInterface = requires(P p, const hid_t hid)
-{
-    { p.apply(hid) };
+template <typename P>
+concept PropertyInterface = requires(P p, const hid_t hid) {
+    {p.apply(hid)};
 };

I've not had time today to compile it and see how it works myself. Though I'm looking forward to having a reason to look at concepts.

[1uc]

@antonysigma I would like there to be a way of forcing the concepts to be off. I'm worried a user will find a compiler that claims to be 202002L but doesn't support concepts; or has a compiler bug. If this happens they would be unable to use HighFive, because a nice, but non-essential feature is broken.

If you like please add a CMake variable HIGHFIVE_HAS_CONCEPTS which defaults to On. Then add code to instruct the compiler to add -DHIGHFIVE_HAS_CONCEPTS={0,1} has appropriate. Then, the if-condition above would be

#if HIGHFIVE_HAS_CONCEPTS && __cplusplus >= 202002L

[antonysigma]

Done. I am not familiar to CMake >12.0 though. Feel free to revise the changes where it make sense.

[1uc]

Thanks! I slightly changed it and tested if it works. One might need -fconcepts-diagnostics-depth=2 to get meaningful compiler output.

[antonysigma]

Great! Let me know if you need anything else. I will standby.

[1uc]

I'd like to prepare a PR which modifies CI to also compile HighFive as C++20 code. Otherwise, the concepts aren't tested during CI. Then we can rebase this PR onto the other one; and everything is tested nicely again.

Thank you for this nice PR.

[1uc]

Thank you, nicely done!