Re: [boost] Boost Hana comments

Sorry for the slow answer; I completely missed your message. Also note that I've been working on a large changeset for the past week, so some comments are actually already fixed in my local version. Jeremy Maitin-Shepard writes:

First off, while I haven't thoroughly reviewed the library, I am certainly impressed by a lot of what I see. It is obvious that this library represents a lot of excellent work.

Here are my miscellaneous comments:

Constant type class: --------------------

Why isn't std::integral_constant an instance?

Good observations; that's a matter of time. std::integral_constant is an instance in my local version.

There is no documentation of what the value method actually returns, just that it returns something that is a constexpr. Does value(integral)) return int, or does it return integral ?

It returns the underlying value of `integral`, which is an `int`. The documentation will be improved to explain it.

Perhaps the Constant type class should be "parameterized" by a type specifying the return type of value.

Might be a good idea; I'll think about how this would interact with the rest of the library.

Integral data type: -------------------

It would be better not to duplicate std::integral_constant. However, I recognize that the overloaded operators are useful and you wouldn't be able to do this with std::integral_constant (well, I suppose you still could, but as they wouldn't be found by ADL, they wouldn't be terribly useful). This rationale (assuming it is correct) should be documented though.

This is part of the rationale. There's also the fact that integral_constant requires including , which is about the size of the whole library; I find this annoying. I'll document that.

Logical type class: -------------------

What is really gained by making this a type class at all?

Yes; how would you branch at compile-time otherwise? The if_ method for e.g. Integrals is not equivalent to a normal C++ if-statement; it allows both branches to have different and incompatible types. Also, since we want to be able to branch on `integral`s but also on `mpl::integral_c`s, we need a type class.

It seems that and_ and or_ require that all arguments be of the same "hana datatype", though actually this isn't documented anywhere and this might not even be the precise requirement (but obviously it needs to be documented very explicitly, and this probably applies to almost every method in Hana).

Really we really should be able to mix multiple data types in a call to or_ or and_, though. Otherwise the usability is unreasonably limited. Of course there is then the question of return type. However, I would assume most users would be happy getting back either a Hana Integral or a bool, depending on whether it is a compile-time or runtime condition. Certainly this would be better than getting back a compiler error.

The arguments of `and_` and `or_` only need to be Logicals. I'll document it.

Another question: how does the performance of and_ and or_ compare to the optimized verions you showed in your MPL11 talk?

I haven't benchmarked that yet, but I expect it to be significantly less efficient. I'll have to think about a way of specifying the requirements of `Logical` in a way that makes optimizations possible, which is not the case currently.

Pair datatype: --------------

Why duplicate std::pair?

For the same reason as `std::integral_constant`; operators.

Alternatively, why duplicate list?

Perhaps list should just define first and second as well?

Product type class: ----------------

Why isn't a 2-element List, a 2-element std::tuple, etc. an instance?

`hana::list`, `std::tuple` and friends can't be made an instance of the `Product` type class because they would not satisfy its laws.

List type class: ----------------

Just as for Logical, it would be useful to be able to mix datatypes in calls to concat, zip, zip_with.

I agree, but it would then be harder to implement those operations efficiently, since you would not control the representation of all the arguments. I'll think about a way of supporting different data types while still allowing optimizations.

The list function itself shouldn't be documented as part of the type class, since it isn't logically part of it.

That's because `List` was both a data type and a type class. In my local version, `List` is only a type class and the data type is named `Tuple` instead, which makes this a non-issue.

What about get<N>, get accessors? I understand these can be defined in terms of the other methods, but clearly they are useful on their own.

For `get<N>`, I'll provide an helper method `at_c<N>(iterable)`. I don't understand what you mean by `get` though. What does `get` do? Regards, Louis -- View this message in context: http://boost.2283326.n4.nabble.com/Boost-Hana-comments-tp4666314p4666536.htm... Sent from the Boost - Dev mailing list archive at Nabble.com.