- Whether you believe the library should be accepted into Boost Yes - Your name
David Stone
- Your knowledge of the problem domain.
Advanced
- What is your evaluation of the library's: * Design
Good. It seems like most of the library follows the naming conventions of boost and the standard library (identifiers_like_this), but there are a few exceptions. The most notable is boost::hana::IntegralConstant. Why not integral_constant? Aside from minor issues like this, I am a huge fan of the overall idea of making compile-time and run-time code the same. Ideally we would have just a single language in C++ (as opposed to plain C++, template C++, constexpr C++, macro C++, and concept C++) and a single style of programming. The design of this library follows and extends this general trend in C++ that started with constexpr.
* Implementation
Looking through the source, it seems to flow naturally. Most of the code looks essentially the same as I would have written it. It has small, self-contained functions and appropriate use of macros, comments, and static_asserts. It would be nice if your IntegralConstant provided a specialization of std::numeric_limits. This would allow it to work properly with a few libraries (two examples are my bounded::integer library and this fixed point library: https://github.com/mizvekov/fp). You could then detect type-level compile-time constants as being any integer type for which std::numeric_limits<T>::min() == std::numeric_limits<T>::max(), which would allow the interoperability to flow the other way, but I do not know if the library ever requires this information or if it just relies on the compiler to complain about things that are not constexpr.
* Documentation
From the perspective of someone who has never used MPL or Fusion (I started
Very good from some perspectives. I started using the library by trying out checking expression validity, and it followed a logical progression, covering all types of members. This naturally led to me wondering if it is possible to use the library to test for the existence of a member named `blah`, but without knowing what category of member it is (variable, function, type, or template). I am assuming that this is as simple as saying `is_valid(variable) or is_valid(function) or ...` However, I feel that the documentation is trying to serve two incompatible purposes. On the one hand, it is trying to explain the theory of type computation using terms from functional programming. On the other hand, it also trying to be a practical guide to using the library. I feel that the second goal is compromised by the first. It seems that most of the content needed for the "cut to the chase" user is present, but by having this presentation in the form of a single document dilutes it. In other words, I don't know that the documentation passes the coworker test. If I were to show it to my coworkers, I can already hear them saying "All this stuff about computational quadrants and heterogeneous algorithms is all well and good, but how do I actually use this thing?". metaprogramming in C++11 and C++14 and never found a need for them), I do not have a problem with the comparisons to the other libraries in the documentation. I know another reviewer had mentioned that as a concern, but it just reads as evaluating prior art rather than reading as assuming prior knowledge. * Tests
I have not looked at the library's tests, I just tried to use it.
* Usefulness
The lack of constexpr lambdas really hurts the library's usefulness. It is because of this hole in the language that it can be difficult to slowly move to hana. I would like to just replace the implementation of a few functions with hana implementations at first, but then my functions are no longer constexpr if I do the obvious implementation. I have to make sure to change all of my types to IntegralConstant, for instance, to get a non-constexpr value from which I can extract a constexpr value from its type. This has a larger ripple throughout than I would like for just testing out a library before committing to it. That being said, once updating your code to fit into this paradigm, it does seem to simplify things a bit. I expect that as I update my library to be more hana friendly throughout, this simplification will increase. It was a bit disheartening at first when my code size grew slightly as I wrote various shims to translate to the correct types (I had more constexpr functions returning bool, and not so much classes that derive from integral_constant), but that ended up being a temporary state until I worked through more code. I wonder if I passed a constexpr function object rather than a non-constexpr lambda if these limitations would be removed? In practice this would probably lead to more code in the intermediate step even if it did work due to the boilerplate of defining a non-lambda function object.
- Did you attempt to use the library? If so: * Which compiler(s)?
* What was the experience? Any problems?
- How much effort did you put into your evaluation of the review?
In addition to talking with Louis at C++Now, I spent a few days trying out
clang 3.6.0 The first thing I tried was to use is_valid to determine the existence of a member type. This immediately crashed clang in the parser (I haven't had time to extract the bug report yet, but I will). It didn't seem to be the library directly, especially since others have used this feature, but the combination of my library (bounded::integer) with this feature of hana seemed to interact to cause a crash. clang seemed to perform a bit better with my second test, which was to replace some enable_if with if_. the library.