Christophe Henry writes:

Dear all,

Following is my review of Hana.

[...]

However, being value-based instead of type-based turns out to be a strength but also a weakness. To test the library, I replaced some simple MPL-based implementation parts of MSM with Hana. It works, but the code is not as simple and clean as I had hoped. For example: [...]

I had to create object where I actually just wanted types and also a static function so I can decltype. This is because this code was written within a struct (serving as metafunction). Honestly, my win in readability is limited

It seems to me like this is testing the MPL/Hana interoperation rather than Hana itself. Of course, that interoperation is part of Hana and it ought to work well, but mixing MPL and Hana styles together should not be expected to give incredible results. Here's how I would have implemented the above metafunction in pure Hana: // This wouldn't be part of the metafunction, because you would ideally // receive objects instead of types. constexpr auto make_row_tag_v = hana::metafunction; constexpr auto stt_simulated_v = hana::tohana::Tuple(stt_simulated{}); static auto fold() { return hana::fold_left(stt_simulated_v, hana::tuple_t<>, [](auto ts, auto t){ return hana::append(ts, make_row_tag_v(t, hana::type<BaseType>)); }); } typedef decltype(fold()) type; However, the following would also work (and be much more efficient): static auto fold() { return hana::unpack(stt_simulated_v, [](auto ...t) { return hana::make_tuple(make_row_tag_v(t, hana::type<BaseType>)...); }); } Now, this implementation gives us a hint about how we might want to proceed for a Hana/MPL implementation: static auto fold() { return hana::unpack(stt_simulated{}, [](auto ...t) { return mpl::vector< typename make_row_tag::type... >{}; }); } Regarding the need for a static function, this would be unnecessary if (1) the enclosing metafunction was implemented as a function too (2) lamdbas were allowed in unevaluated contexts

But my problems started earlier. The heart of MSM is the transition table: struct transition_table : mpl::vector< Row < Stopped , stop , Stopped , none , none >, ...

...

{}

All states and events being types, using Hana would lead me to write: Row < Stopped{} , stop{} , Stopped{} , none{} , none{} >

which would be a regression.

In a potential MSM rewrite with Hana, you could write something like: auto transition_table = table( row(...) ... row(...) ); I don't know much about MSM, but I think you must have callbacks somewhere in your transition table, right? One advantage of Hana there would be the ability to write something like auto transition_table = table( row(..., [](auto event) { ... }) );

I'd love to get rid of my mpl::vector but Hana won't help me there. Maybe MSM was the wrong test object.

The lack of support for mpl::set reduces to a very small part the algorithms I could port to Hana. Louis seems to want to change this, which I can only welcome.

In the end, I feel that using Hana with MSM would be a take all or nothing choice. With the current compiler support, it is not something I will do in the next few months.

IMHO, the morale is that porting existing code to use Hana is possible, but not necessarily easy depending on how pervasively you want to use Hana. The primary goal of the library was _not_ to make it easy to port old code, but rather to make new libraries much easier to write.

Still, the library has a nice design and potential and I will probably use it either later in other use cases (or with MSM with better support of MPL).

* Implementation I had a short glance and found it very clean. I found only one thing which bugged me, the use of non fully qualified names in some places. True, many other boost libraries do the same and it brings a lot of problems (anyone who tried to include Geometry and TypeErasure in one TU will love the duplicate definition of use_default). Still, a "return true_;" is begging for conflicts.

I use qualified names when calling functions to avoid ADL-related problems, but I wasn't aware of potential problems with using `return true_` (or similar). `true_` is defined in the `boost::hana` namespace, so shouldn't I be able to refer to it as `true_` unambiguously from within that namespace? What is the catch?

* Documentation Great! Way above Boost level. As others said, please use fully qualified names where possible. It makes it much easier to follow.

Will do.

* Tests Did not look.

* Usefulness Very useful if you have objects in the first place, maybe less to do TMP.

- Did you attempt to use the library? If so: * Which compiler(s)? Clang 3.5.

* What was the experience? Any problems? I got a few compiler errors. - A static_assert "hana::fold.left(xs, state, f) requires xs to be Foldable" when forgetting to #include I had included boost/hana.hpp but it did not include all, which is annoying.

The problem is that if I start including external adapters from , and if I support more than only MPL/Fusion/std, those dependencies are passed on to the user, which is a problem. For example, if Eric's Meta library is supported in the future, you shouldn't be required to add this dependency to your project all of a sudden.

- a name conflict with std::size_t, though I deserved punishment by adding a using namespace boost::hana before including other headers.

- How much effort did you put into your evaluation of the review? 8 hours reading the doc and rewriting some MSM algorithms with Hana.

Thank you Louis for writing this excellent library and thank you Glen for managing the review!

Christophe

Thanks a lot for your review, Christophe. Regards, Louis