This is a somewhat perfunctory review of Hana, because I wanted to get something in before the end of the review period. I have submitted a few issues on GitHub, for minor things that need not be repeated here. Below are the things I found that might warrant more discussion. - Whether you believe the library should be accepted into Boost * Conditions for acceptance Yes it should. No specific conditions. - Your name John Bytheway - Your knowledge of the problem domain. User of TMP for years. Once wrote a source-to-source translator to help me write MPL programs because it was prohibitively difficult to write them correctly by hand. - What is your evaluation of the library's: * Design The design seems reasonable, and I like the adherence to mathematical concepts. The most difficult thing is getting the names right, and I know there's been plenty of debate on that. The name that most concerns me is Range, given that that term is already commonly used in Boost as a concept with a completely different meaning (in the Boost.Range sense). * Implementation Did not look. * Documentation Mostly very good, but of course there is always room for improvement. One particular thing I wanted to raise here is that I struggled to find enough information about Hana-convertibility (i.e. using the 'to' function). For example, I wondered if I could create a Tuple from a Range, and it seems I can. Then I wondered whether I could create a std::Array from a homogeneous Tuple, but it seems I cannot. How should I know these things? (This latter case of converting to an array is something I feel ought to be supported for easing the boundary between runtime and compile-time sequences) * Tests Did not look. * Usefulness Very, although not useful for me personally until I decide to bite the ABI-breaking bullet and recompile all my code in gcc 5.x with the new libstdc++. The biggest risk I see is MSVC support. Microsoft seem to be much more standards-gung-ho than they once were, so I hope it won't take too much longer before we see support. But I do encourage Louis to be wary of taking advantage of any new language features (e.g. a potential change in C++17 lambdas-in-unevaluated-contexts rules, or the compiler extension for compile-time string literals) lest it delay MSVC support. - Did you attempt to use the library? If so: * Which compiler(s) clang 3.6.1 * What was the experience? Any problems? I played around a bit, mostly with a view to finding out how bad the compiler errors are when things go wrong. The results were promising. I went looking for some code of mine to try porting to Hana, and got sidetracked by one difficulty. The first project I came across using Boost.Fusion was passing a lot of fusion::vectors across function interfaces, but the vector types were specific and the functions not defined in headers. Hana's tuple cannot easily be used in this way since the type is unspecified. I don't think this is a problem, because (a) that code was just poorly written and ought instead to be using adapted structs, which Hana does support, and (b) I can use Hana algorithms with Fusion's tuples anyway. - How much effort did you put into your evaluation of the review? A few hours reading and playing. Thanks very much to Louis for his effort. I look forward to being able to use Hana in the not-too-distant future, and I'll make GitHub issues for any more things I discover in my testing. John
John Bytheway <jbytheway+boost <at> gmail.com> writes:
[...] - What is your evaluation of the library's: * Design
The design seems reasonable, and I like the adherence to mathematical concepts. The most difficult thing is getting the names right, and I know there's been plenty of debate on that. The name that most concerns me is Range, given that that term is already commonly used in Boost as a concept with a completely different meaning (in the Boost.Range sense).
Interval might be reasonable too?
* Implementation
Did not look.
* Documentation
Mostly very good, but of course there is always room for improvement. One particular thing I wanted to raise here is that I struggled to find enough information about Hana-convertibility (i.e. using the 'to' function). For example, I wondered if I could create a Tuple from a Range, and it seems I can. Then I wondered whether I could create a std::Array from a homogeneous Tuple, but it seems I cannot.
A Tuple is a Sequence, and a Sequence can be created from any Foldable. Since a Range is Foldable, it works. However, a std::array is _not_ a Sequence.
How should I know these things?
The Foldable -> Sequence conversion is documented in the reference of Foldable, here: http://ldionne.com/hana/structboost_1_1hana_1_1Foldable.html However, I agree that it could/should appear closer to the description of the `to` function. One problem is that a lot of types provide the `to` function, but I can't really describe all of them in the reference for `to`. Where should these conversions be documented to make it easier for users to find them?
(This latter case of converting to an array is something I feel ought to be supported for easing the boundary between runtime and compile-time sequences)
std::array is not "general" enough to allow construction from an arbitrary Hana Sequence. As you point out, it would only work for Sequences containing objects of a single type. However, this breaks the naturality of the `to` function. Of course, nothing prevents me from adding a conversion to std::arrays, but it wouldn't actually fit in Hana's mathematical framework. I know the above must seem quite fuzzy, so let me try to explain my point by considering the implementation of a `to<std::array>` conversion: namespace boost { namespace hana { template <typename S> struct to_impl<ext::std::Array, S, when<models<Sequence, S>()>> { template <typename Xs> static /*constexpr*/ auto apply(Xs&& xs) { using T = std::remove_reference_t<decltype(hana::head(xs))>; auto len = hana::length(xs); return hana::unpack(std::forward<Xs>(xs), [=](auto&& ...x) { return std::array<T, len>{{std::forward<decltype(x)>(x)...}}; }); } }; }} See how we make an arbitrary choice to use the type of the first element of the sequence for `T`? This, and the fact that it will only work when the Sequence happens to contain elements of a single type, are what breaks naturality.
[...]
I went looking for some code of mine to try porting to Hana, and got sidetracked by one difficulty. The first project I came across using Boost.Fusion was passing a lot of fusion::vectors across function interfaces, but the vector types were specific and the functions not defined in headers. Hana's tuple cannot easily be used in this way since the type is unspecified. I don't think this is a problem, because (a) that code was just poorly written and ought instead to be using adapted structs, which Hana does support, and (b) I can use Hana algorithms with Fusion's tuples anyway.
Technically, Hana's tuple has a specified type. That type is `hana::_tuple<...>`. The name will change to (probably) `hana::tuple<...>`. It is true that most other data structures have unspecified types, but this is similar to how Fusion's vectors have numbered forms. I'm curious to know how the original code managed to pass Fusion vectors across interfaces? Did it make sure to only use the non-numbered fusion::vector<...> form, or something else?
- How much effort did you put into your evaluation of the review?
A few hours reading and playing.
Thanks very much to Louis for his effort. I look forward to being able to use Hana in the not-too-distant future, and I'll make GitHub issues for any more things I discover in my testing.
John
Thanks a lot for your review, John. Regards, Louis
Le 24/06/15 18:39, Louis Dionne a écrit :
John Bytheway <jbytheway+boost <at> gmail.com> writes:
[...] - What is your evaluation of the library's: * Design
The design seems reasonable, and I like the adherence to mathematical concepts. The most difficult thing is getting the names right, and I know there's been plenty of debate on that. The name that most concerns me is Range, given that that term is already commonly used in Boost as a concept with a completely different meaning (in the Boost.Range sense). Interval might be reasonable too?
* Implementation
Did not look.
* Documentation
Mostly very good, but of course there is always room for improvement. One particular thing I wanted to raise here is that I struggled to find enough information about Hana-convertibility (i.e. using the 'to' function). For example, I wondered if I could create a Tuple from a Range, and it seems I can. Then I wondered whether I could create a std::Array from a homogeneous Tuple, but it seems I cannot. A Tuple is a Sequence, and a Sequence can be created from any Foldable. Since a Range is Foldable, it works. However, a std::array is _not_ a Sequence.
Couldn't std::array<T,0> be considered the empty Array and so Array could be considered a homogeneous sequence?
How should I know these things? The Foldable -> Sequence conversion is documented in the reference of Foldable, here: http://ldionne.com/hana/structboost_1_1hana_1_1Foldable.html However, I agree that it could/should appear closer to the description of the `to` function. One problem is that a lot of types provide the `to` function, but I can't really describe all of them in the reference for `to`.
Where should these conversions be documented to make it easier for users to find them?
I would put the To function on the Source class it it s a concrete type. Otherwise on the Target class. It could be added on a see also section for the other class or concept.
(This latter case of converting to an array is something I feel ought to be supported for easing the boundary between runtime and compile-time sequences) std::array is not "general" enough to allow construction from an arbitrary Hana Sequence. As you point out, it would only work for Sequences containing objects of a single type. However, this breaks the naturality of the `to` function. Of course, nothing prevents me from adding a conversion to std::arrays, but it wouldn't actually fit in Hana's mathematical framework.
I know the above must seem quite fuzzy, so let me try to explain my point by considering the implementation of a `to<std::array>` conversion:
namespace boost { namespace hana { template <typename S> struct to_impl<ext::std::Array, S, when<models<Sequence, S>()>> { template <typename Xs> static /*constexpr*/ auto apply(Xs&& xs) { using T = std::remove_reference_t<decltype(hana::head(xs))>; auto len = hana::length(xs); return hana::unpack(std::forward<Xs>(xs), [=](auto&& ...x) { return std::array<T, len>{{std::forward<decltype(x)>(x)...}}; }); } }; }}
See how we make an arbitrary choice to use the type of the first element of the sequence for `T`? This, and the fact that it will only work when the Sequence happens to contain elements of a single type, are what breaks naturality. Why not request that the sequence is homogeneous?
Vicente
Vicente J. Botet Escriba <vicente.botet <at> wanadoo.fr> writes:
Le 24/06/15 18:39, Louis Dionne a écrit :
John Bytheway <jbytheway+boost <at> gmail.com> writes:
[...]
Mostly very good, but of course there is always room for improvement. One particular thing I wanted to raise here is that I struggled to find enough information about Hana-convertibility (i.e. using the 'to' function). For example, I wondered if I could create a Tuple from a Range, and it seems I can. Then I wondered whether I could create a std::Array from a homogeneous Tuple, but it seems I cannot. A Tuple is a Sequence, and a Sequence can be created from any Foldable. Since a Range is Foldable, it works. However, a std::array is _not_ a Sequence.
Couldn't std::array<T,0> be considered the empty Array and so Array could be considered a homogeneous sequence?
The problem is that a homogeneous sequence (in Hana's mathematical universe) is a sequence whose tags are homogeneous. Hence, a tuple containing IntegralConstant<int> objects only is a homogeneous sequence for Hana's concepts. Hence, a std::array is not powerful enough to hold Hana-homogeneous data. Indeed, the following is Hana-homogeneous but not homogeneous in the usual C++ sense: auto xs = make_tuple(int_<1>, int_<2>, int_<3>); It is Hana-homogeneous because all the int_s have the IntegralConstant<int> tag, but it is not C++-homogeneous because they all have different C++ types. Again, the concern here is being correct mathematically, but there's no other technical reason why this can't be done. Ideally, I'd like to be mathematically correct while also providing this functionality, or similar.
[...]
See how we make an arbitrary choice to use the type of the first element of the sequence for `T`? This, and the fact that it will only work when the Sequence happens to contain elements of a single type, are what breaks naturality. Why not request that the sequence is homogeneous?
My answer to your other question above should also answer this one. If not, let me know and I'll gladly clarify. Regards, Louis
On 2015-06-24 12:39, Louis Dionne wrote:
John Bytheway <jbytheway+boost <at> gmail.com> writes:
[...] - What is your evaluation of the library's: * Design
The design seems reasonable, and I like the adherence to mathematical concepts. The most difficult thing is getting the names right, and I know there's been plenty of debate on that. The name that most concerns me is Range, given that that term is already commonly used in Boost as a concept with a completely different meaning (in the Boost.Range sense).
Interval might be reasonable too?
Indeed, seems fine to me. I might be inclined to be more specific and call it IntegerInterval, but that's not really necessary.
Mostly very good, but of course there is always room for improvement. One particular thing I wanted to raise here is that I struggled to find enough information about Hana-convertibility (i.e. using the 'to' function). For example, I wondered if I could create a Tuple from a Range, and it seems I can. Then I wondered whether I could create a std::Array from a homogeneous Tuple, but it seems I cannot.
A Tuple is a Sequence, and a Sequence can be created from any Foldable. Since a Range is Foldable, it works. However, a std::array is _not_ a Sequence.
How should I know these things?
The Foldable -> Sequence conversion is documented in the reference of Foldable, here: http://ldionne.com/hana/structboost_1_1hana_1_1Foldable.html However, I agree that it could/should appear closer to the description of the `to` function. One problem is that a lot of types provide the `to` function, but I can't really describe all of them in the reference for `to`.
Where should these conversions be documented to make it easier for users to find them?
Well, if they are documented in the source concept and the documentation for 'to' states that that is where such conversions are documented, then I think that's fine.
(This latter case of converting to an array is something I feel ought to be supported for easing the boundary between runtime and compile-time sequences)
std::array is not "general" enough to allow construction from an arbitrary Hana Sequence. As you point out, it would only work for Sequences containing objects of a single type. However, this breaks the naturality of the `to` function. Of course, nothing prevents me from adding a conversion to std::arrays, but it wouldn't actually fit in Hana's mathematical framework.
I know the above must seem quite fuzzy, so let me try to explain my point by considering the implementation of a `to<std::array>` conversion:
namespace boost { namespace hana { template <typename S> struct to_impl<ext::std::Array, S, when<models<Sequence, S>()>> { template <typename Xs> static /*constexpr*/ auto apply(Xs&& xs) { using T = std::remove_reference_t<decltype(hana::head(xs))>; auto len = hana::length(xs); return hana::unpack(std::forward<Xs>(xs), [=](auto&& ...x) { return std::array<T, len>{{std::forward<decltype(x)>(x)...}}; }); } }; }}
See how we make an arbitrary choice to use the type of the first element of the sequence for `T`? This, and the fact that it will only work when the Sequence happens to contain elements of a single type, are what breaks naturality.
I understand the concern. FWIW, the specific use case I was thinking of was being able to do something like this for an arbitrary tuple t: boost::algorithm::join(hana::to<Array>(hana::transform(t, to_string)), " ") because boost::algorithm::join is expecting a runtime Range rather than a compile-time one. Array seemed like the most likely candidate for glueing these together. Do you have any alternative suggestions for such situations?
[...]
I went looking for some code of mine to try porting to Hana, and got sidetracked by one difficulty. The first project I came across using Boost.Fusion was passing a lot of fusion::vectors across function interfaces, but the vector types were specific and the functions not defined in headers. Hana's tuple cannot easily be used in this way since the type is unspecified. I don't think this is a problem, because (a) that code was just poorly written and ought instead to be using adapted structs, which Hana does support, and (b) I can use Hana algorithms with Fusion's tuples anyway.
Technically, Hana's tuple has a specified type. That type is `hana::_tuple<...>`. The name will change to (probably) `hana::tuple<...>`.
I thought that tuple_t<...> might produce a different type of Tuple?
It is true that most other data structures have unspecified types, but this is similar to how Fusion's vectors have numbered forms. I'm curious to know how the original code managed to pass Fusion vectors across interfaces? Did it make sure to only use the non-numbered fusion::vector<...> form, or something else?
Indeed, it only used the non-numbered versions. There was one part of the code implementing some generic optimization which returned a fusion::vector of parameter choices, and then elsewhere this was being used in a specific case where the number and type of parameters was known, and the resulting parameter vector was just passed around as-is rather than converting it to a more meaningful type. John
On 2015-06-24 12:39, Louis Dionne wrote:
[...] The Foldable -> Sequence conversion is documented in the reference of Foldable, here:http://ldionne.com/hana/structboost_1_1hana_1_1Foldable.html However, I agree that it could/should appear closer to the description of the `to` function. One problem is that a lot of types provide the `to` function, but I can't really describe all of them in the reference for `to`.
Where should these conversions be documented to make it easier for users to find them?
Well, if they are documented in the source concept and the documentation for 'to' states that that is where such conversions are documented, then I think that's fine.
Good idea. I added this to the documentation of `to`.
[...]
I understand the concern.
FWIW, the specific use case I was thinking of was being able to do something like this for an arbitrary tuple t:
boost::algorithm::join(hana::to<Array>(hana::transform(t, to_string)), " ")
because boost::algorithm::join is expecting a runtime Range rather than a compile-time one. Array seemed like the most likely candidate for glueing these together. Do you have any alternative suggestions for such situations?
I don't know how generally this applies, but I would suggest the following for your precise use case: boost::algorithm::join(hana::unpack(t, [](auto&& ...x) { return std::array<std::string, sizeof...(x)>{{ to_string(std::forward<decltype(x)>(x))... }}; }), " "); In addition, this will be lightning fast because unpacking a tuple is very cheap and you do not require any layer between your call to to_string and the creation of your array, so you minimize the number of functions that are instantiated.
[...]
I went looking for some code of mine to try porting to Hana, and got sidetracked by one difficulty. The first project I came across using Boost.Fusion was passing a lot of fusion::vectors across function interfaces, but the vector types were specific and the functions not defined in headers. Hana's tuple cannot easily be used in this way since the type is unspecified. I don't think this is a problem, because (a) that code was just poorly written and ought instead to be using adapted structs, which Hana does support, and (b) I can use Hana algorithms with Fusion's tuples anyway.
Technically, Hana's tuple has a specified type. That type is `hana::_tuple<...>`. The name will change to (probably) `hana::tuple<...>`.
I thought that tuple_t<...> might produce a different type of Tuple?
That is correct, and my comment was unclear. `make_tuple(...)` has a specified type, but `tuple_t<...>` does not.
It is true that most other data structures have unspecified types, but this is similar to how Fusion's vectors have numbered forms. I'm curious to know how the original code managed to pass Fusion vectors across interfaces? Did it make sure to only use the non-numbered fusion::vector<...> form, or something else?
Indeed, it only used the non-numbered versions. There was one part of the code implementing some generic optimization which returned a fusion::vector of parameter choices, and then elsewhere this was being used in a specific case where the number and type of parameters was known, and the resulting parameter vector was just passed around as-is rather than converting it to a more meaningful type.
Ok. Might this qualify as a borderline usage of Fusion's implementation details? Regards, Louis -- View this message in context: http://boost.2283326.n4.nabble.com/hana-Review-tp4677475p4677597.html Sent from the Boost - Dev mailing list archive at Nabble.com.
On 2015-06-27 10:25, Louis Dionne wrote:
FWIW, the specific use case I was thinking of was being able to do something like this for an arbitrary tuple t:
boost::algorithm::join(hana::to<Array>(hana::transform(t, to_string)), " ")
because boost::algorithm::join is expecting a runtime Range rather than a compile-time one. Array seemed like the most likely candidate for glueing these together. Do you have any alternative suggestions for such situations?
I don't know how generally this applies, but I would suggest the following for your precise use case:
boost::algorithm::join(hana::unpack(t, [](auto&& ...x) { return std::array<std::string, sizeof...(x)>{{ to_string(std::forward<decltype(x)>(x))... }}; }), " ");
In addition, this will be lightning fast because unpacking a tuple is very cheap and you do not require any layer between your call to to_string and the creation of your array, so you minimize the number of functions that are instantiated.
Makes sense. And I guess I can write a function converting to arrays fairly easily if I want one.
It is true that most other data structures have unspecified types, but this is similar to how Fusion's vectors have numbered forms. I'm curious to know how the original code managed to pass Fusion vectors across interfaces? Did it make sure to only use the non-numbered fusion::vector<...> form, or something else?
Indeed, it only used the non-numbered versions. There was one part of the code implementing some generic optimization which returned a fusion::vector of parameter choices, and then elsewhere this was being used in a specific case where the number and type of parameters was known, and the resulting parameter vector was just passed around as-is rather than converting it to a more meaningful type.
Ok. Might this qualify as a borderline usage of Fusion's implementation details?
I don't think so. I believe all Fusion sequences are assignable and constructible from each other (when they have the same number of and assignable elements), so even if a pass a vector2<double, double> to a function expecting a vector<double, double> it will work fine. This mutual assignability is very useful in certain cases; for example it makes it easy to use adapted structs in Boost.Spirit to store the result of parsing. John
participants (3)
-
John Bytheway -
Louis Dionne -
Vicente J. Botet Escriba