Re: [boost] [Hana] Formal review for Hana

Subject: Re: [boost] [Hana] Formal review for Hana
From: Louis Dionne (ldionne.2_at_[hidden])
Date: 2015-06-17 09:01:22

Kohei Takahashi <flast <at> flast.jp> writes:

>
> [...]
>
> >   * Implementation
> Some identifiers, whitch defined under boost::hana, begin with single
> underscore (e.g. `_is_a' in core/is_a.hpp). I think (and be afraid) it
> will violate [lex.name] p.3 (I referenced n4296) when user use
> using-directive (i.e. `using namespace boost::hana;'). Rename or move to
> detail namespace might be better, since thouse are documented as
> /unspecified-type/.

I think you're right. But then it means that the placeholders from Boost.Bind
(or even Boost.Lambda if you import them to the global namespace) will also
violate [lex.name], right? Still, I'll find a different way to name those.
The main interest was that it results in very clear error messages. For
example,

    fold_left(1, 2, 3); // obviously an error

    error: static_assert failed
        "hana::fold_left(xs, state, f) requires xs to be Foldable"
                static_assert(_models<Foldable, S>{},
                ^             ~~~~~~~~~~~~~~~~~~~~~~
    note: in instantiation of function template specialization
        'boost::hana::_fold_left::operator()<int, int, int>' requested here
        fold_left(1, 2, 3);
                 ^

Notice how the name of the function template we're instantiating looks almost
the same as the algorithm we called? I'd like to keep something as close to
this as possible. You can refer to this issue [1] in the future.

Note: It just occurred to me that user-defined literals are required to start
with an underscore. Does that mean that we can't possibly import custom UDLs
into the global namespace without violating [lex.name]?

> (I found return type of `make<Tuple>' is not a unspecified-type in the
> document. Is this intended?)

Yes, this is intended. I will change the name of _tuple to avoid it looking
like an implementation detail.

>
> >   * Documentation
> Well described, but...
>
>   * Should be more clearly about which C++14(11) features are required.
>     Most of users assume working with C++98 since all of currently
>     released Boost libraries work under C++98. Also, some (proprietary
>     or not) vendor might ship customized compiler based on OSS one.
>     In such case, users have difficulty in determining usable or not
>     since some features are unsupported or dropped. Boost.Config will
>     help and depending it is better than self organizing, IMO.

The compiler requirements are currently documented in the README, which
is the first thing that pops up when you go to the project page on GitHub.
However, I think I wrongly assumed that everybody would start by the GitHub
page and reading the README. A lot of people seem to start with the
documentation, which is legitimate. I'll add building instructions and
compiler requirements to the tutorial documentation. That's a good
suggestion, thanks. See this issue [2] for future reference.

>   * It is important, in contrast to Fusion and std::tuple, Data types
>     can't modify even its element. Users might use similar to Fusion
>     (assgin, tie, ...) because most of the C++ programmers have no
>     backend about purely functional programming and immutables and god
>     compile error. Also, C++14 constexpr allow to assign, thus
>     /constant-expression/ doesn't make sense to describe purely.
>     It is the best, describing about it in Introduction section (also
>     describing about can't make /tie/ is better).

Technically, Hana requires the functions that are used inside higher-order
algorithms to be pure, but that's about it. In other words, there's nothing
in Hana's philosophy that prevents you from modifying the elements of a tuple

    auto xs = make_tuple(...);
    xs[3_c] = "foobar";

However, there is quite a bit of imprecision in the documentation regarding
which functions should return a reference and which ones should return copies,
so right now it is unclear when the above is actually valid. That will be
addressed by this issue [3].

>   * Where is source of documentations? How to generate it?
>     It seems to be generated using doxygen, but any of headers don't
>     contain its documents.

The documentation is in the headers. I assume you looked at the online
version, though. Comments were stripped from the online version because
the library was otherwise too large to upload to Wandbox. If you go look
at the source, for example [4], you'll see the Doxygen comments.

> >   * Tests
> I didn't run, but it seems to be enough though.
>
> >   * Usefulness
> Since names are similar to haskell, some users might confuse about usage
> (e.g. `maybe' v.s. `Optional'), but it's trivial: no worry about that.
>
> > - Did you attempt to use the library? If so:
> >   * Which compiler(s)
> I tried some tiny code with GCC 5.1.0.
>
> >   * What was the experience? Any problems?
> Compiling is much faster than Fusion, Wow!
>
> > - How much effort did you put into your evaluation of the review?
> It was bit hard to understand some semantics because I'm not a Haskell
> programmer and have no mathmatic backends.

I feel like only the most advanced/abstract parts of Hana like Applicative
and Monad require some functional programming background, but the rest of
it should ideally be understandable by any C++ programmer. I'm thinking
about the Iterable, Foldable, Searchable and Sequence concepts, which
contain 90% of the useful stuff anyway.

Thanks a lot for your review, Kohei!

Regards,
Louis

[1]: https://github.com/ldionne/hana/issues/130
[2]: https://github.com/ldionne/hana/issues/128
[3]: https://github.com/ldionne/hana/issues/90
[4]: https://goo.gl/3u122a