Re: [boost] [Hana] David Stone's formal review

Subject: Re: [boost] [Hana] David Stone's formal review
From: Louis Dionne (ldionne.2_at_[hidden])
Date: 2015-06-27 10:22:28

> [...]
>
> 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?

integral_constant<T, v> is already a variable template creating a Hana
IntegralConstant. IntegralConstant refers to the tag of an
integral_constant<...> object. There has been some talk revolving around
renaming those tags to integral_constant_tag or something similar; I'll
consider it.

> [...]
>
> 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).

So for example `std::numeric_limits<IntegralConstant&lt;int>>::max()` would
return `integral_constant<int, std::numeric_limits&lt;int>::max()>`? I like
the idea, but it would violate the expected signature of
`std::numeric_limits<int>::max`, which is (according to cppreference)

    static constexpr T max();

In this case, `T` would be `IntegralConstant<int>`, but `max()` would return
something of a different type. That might or might not be an issue.

> 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.

I don't understand why `numeric_limits<T>::min() ==
numeric_limits<T>::max()`
implies that `T` is a compile-time constant. What am I missing?

> >   * Documentation
> >
>
> 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 ...`

I think it would be as simple as that, yes. However, when testing for a
member function, you have to be checking for a specific signature. I can't
think of a way to check for a member function with an arbitrary signature
off the top of my head.

> 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?".

If the documentation does not pass the coworker test, I think it means
the quick start is not doing its job properly. Did you feel like the
Quick Start section of the tutorial was missing its goal of being a
lightweight introduction without any deep explanation?

> [...]
>
> >   * 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's true; in general, if you want to do something complex with Hana,
you'll want to use lambdas. But then you lose the ability to make the
result constexpr. This is a pain point, but I think the restriction on
lambdas might be lifted in C++17. Let's hope so.

> 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?

Yes; Hana's algorithms are constexpr. If you pass constexpr arguments to
an algorithm, the result will be constexpr.

> > - Did you attempt to use the library? If so:
> >   * Which compiler(s)?
> >
>   * What was the experience? Any problems?
> >
>
> 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_.

Could you please send me the code that caused Clang to crash? You could open
a GitHub issue so I can have a look, and I'll just close it if it's a Clang
bug. Also, it might be a bug I've already ran into, so that would save you
the job of reducing your test case to file a bug against Clang.

> > - 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
> the library.

Thanks a lot for the review, David. I also appreciate the discussions we've
had at C++Now (this year and in 2014), which were always very instructive
and
sometimes even directly applicable to Hana.

Regards,
Louis

--
View this message in context: http://boost.2283326.n4.nabble.com/Boost-Hana-David-Stone-s-formal-review-tp4677533p4677596.html
Sent from the Boost - Dev mailing list archive at Nabble.com.