Subject: Re: [boost] [Fit]Â formal review starts today
From: Paul Fultz II (pfultz2_at_[hidden])
Date: 2016-03-05 10:58:48
> > >>
> > >> - The library reinvents (poorly) its own versions
> > >> of several components:
> > >> compressed_pair <-> boost::compressed_pair,
> > > As I recall boost::compressed_pair is never empty. In addition,
> > it doesn't
> > > support constexpr. Plus, I structured it in a way to avoid bugs
> > with gcc when
> > > doing EBO with constexpr.
> > Could you provide a patch for boost::compressed_pair that is backward
> > compatible and provides whatever you need?
> > Nope. Compressed pair is designed such that &x.first() != &x.second()
> > is always the case. As some users may make assumptions about that.
> Is this the single difference?
That and it works with constexpr. Compressed pair is an implementation
detail and is not part of the interface.
> > >> placeholders <-> lambda, bind, phoenix
> > > Doesn't work with constexpr, and/or is not SFINAE-friendly nor
> > does it handle
> > > perfect forwarding for more than 2 arguments.
> > The same here.
> > Making bind SFINAE-friendly breaks backwards compatibility. However,
> > the lazy adapter passes the same tests for Boost.Bind, plus with
> > constexpr.
> This should be documented as motivation of the addition of new
I do mention that they support `constexpr`.
> > >
> > >> - Some library functions require function objects
> > >> to have a const operator(), without any rhyme
> > >> or reason. e.g. apply requires Callable, but
> > >> apply_eval requires ConstCallable.
> > > This is partly because of the fold done for older compiler. I
> > could easily
> > > lift this restriction. Almost all of the adaptors use
> > `ConstCallable`.
> > IIUC Steven, he is requesting to use Callable everywhere. Steven
> > could
> > you confirm?
> > The library could switch to using `Callable`, if everyone feels that
> > is better. However, I honestly prefer to make mutable explicit. I
> > would like to hear feedback from other reviewers as well.
> We need to see the motivation for explicit mutable function objects.
> How your library work with standard function objects and other that user
> have written that conform to the standard requirements?
Yes, I need to document this rationale.
> > >
> > >> 107: I have no idea why you made alias_value variadic.
> > >> The extra arguments are unused, their presence is
> > >> undocumented, and all calls from inside the library
> > >> (pack.hpp, combine.hpp, and detail/compressed_pair.hpp)
> > >> that use them, could just as well be written without.
> > > This is to make `alias_value` depend on a template parameter
> > because compiler
> > > eagerly instantiate template parameters.
> > Please could a clear explanation be added in the documentation?
> > I don't know if it should be added to the documentation as its a
> > implementation detail. Perhaps an extra comment could help.
> Anything that justify an interface change should be described in the
Its not an interface change. Its just a workaround for gcc.
> > >
> > >> arg.hpp:
> > >>
> > >> 29: /// template<class IntegralConstant>
> > >> /// constexpr auto arg(IntegralConstant);
> > >> ///
> > >> /// template<std::size_t N, class... Ts>
> > >> /// constexpr auto arg_c(Ts&&...);
> > >> arg and argc should do the same thing. It looks
> > >> like arg creates a function object, while arg_c
> > >> evaluates it directly.
> > > Yes, this because you write `arg(int_<5>)(...)` and
> > `arg_c<5>(...)`. In the
> > > future, I may make this a template variable(so `arg_c<5>` will
> > be a function
> > > object).
> > What prevents you to doing it now?
> > Time constraints mainly, but this applies to both arg_c and if_c.
> If this would change the interface it is better to do it before the
> library is accepted.
> If not could this be added to a future work section?
This is not really an interface change. The change would be backwards
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk