Re: [boost] [Fit] formal review starts today

17 Mar 2016

...
...
On Tue, 15 Mar 2016 08:20:27 +0000 (UTC)
On Wednesday, March 16, 2016 11:54 PM, Lee Clagett  wrote:
paul Fultz  wrote:
...
...
On Monday, March 14, 2016 11:15 PM, Lee Clagett
 wrote:  
[snip]
Most of the adaptors return a callable that is ready-for-use,
whereas the lazy and decorator adaptors have not finished
configuration. The partial and pipable adaptors have a third
characteristic in that they _may_ be ready-for-use, depending on
whether the function can be invoked.
In this particular case, I was suggesting `fit::lazy(foo, _1, 2)`
simply because it would return a callable that was ready-for-use,
...
which I thought was more consistent with the other adaptors.
But adaptors take functions as a their parameters. If there are other
 parameters than just functions, than a decorator is used.
The lazy approach is actually more flexible, I was being too difficult
earlier. I was trying to go for consistency on the adapted calls, but
it is not worth the loss in flexibility.
...
...
You can use pipable operators in fit::flow? Everything returns an
immediate value except for when a pipable-adapted function is
partially-invoked, which returns a `pipe_closure` function. If the
`operator|` is used at all a value is returned (which could be, but
is
...
unlikely to be another function). Can you give an example?
Yes, instead of writing:
auto r = x | f(y) | g(z);
The user can write:
auto r = flow(f(y), g(z))(x);
This was the case I referred to - a partially evaluated function that
returned a pipe_closure into the flow adaptor. Although I did think of
another interesting case:
struct foo {
    constexpr bool operator()(int, int, int) const { return true; }
  };
int main() {
      const auto bar = fit::pipable(foo{})(1); // bar is dead?
      return 0;
  }
`bar` doesn't appear to be invokable. fit::limit does not help here
...
either - since its an upward bound constraint ...?
Nope, limit doesn't help in this case. Limit is mainly for the case where you
give it the exact number of parameters because you want it to be fully
evaluated, that is if you write:

struct C {};
auto bar = pipable(foo{})(C{}, C{}, C{});

So `foo` is not callable with `C`, however, this compiles as `bar` is really
just a partially evaluate `foo`. So, if you want to present an error to the
user, limit can be used:

struct C {};
auto bar = pipable(limit_c<3>(foo{})(C{}, C{}, C{});

Of course, this can't handle the case of giving it less number of parameters.
...
A compiler error at> the creation of `bar` would have been preferred, but at least one is
...
given later in any attempt to call `operator()` on it.
Yes, I wish there was a way to present the error earlier, but I don't think
its possible. I have thought about writing a `final` function that would give
a compiler error if it was given a partially evaluted function, so then the
user could write:

auto bar = final(pipable(foo{})(1));

So there would be a compiler error immediately.
...
...
...
[snip]
...
I forgot about the return type compution of Fit - constexpr does not
appear to be the issue in this case. The problem is asking the
compiler to compute the return type of a recursive function. The
compiler cannot "see" the depth unless it runs constexpr
simultaneously while computing - and even then it would have to do
a dead-code optimization in the ternary operator or something.
Head-hurting for sure.
I added constexpr to repeat_integral_decorator<0>::operator(),
and
...
defined FIT_RECURSIVE_CONSTEXPR_DEPTH=0 then ran test/repeat.cpp
with Clang 3.4 and Gcc 4.8 both of which compiled just fine. Only a
two-phase approach is needed here (DEPTH=1), where the first phase
has the full expression for SFINAE purposes and normalizing the
value with the ternary operator, and the second phase is a fixed
return type based
...
on the forwarded value.
Hmm, that may work. I originally tried a two-phase approach at first,
 which didn't work. However, it may work, at least for fit::repeat and
 fit::repeat_while. I don't think it will work at all for fit::fix
 even with an explicit return type. I will need to investigate this
 more.
...
Is there a bug/issue in an older Gcc, Clang, or even MSVC that
needed this larger loop unrolling? It is still unfortunate that a
stackoverlow
...
is possible with this implementation
Yes, it is. I need to look into a way to prevent or minimize this.
Consider a loop-based approach if you decide to have the last stage
non-constexpr (it is currently _not_ constexpr). I do not see a point in
making it recursive in that situation. Although, if you mark it
constexpr, compilers that properly support C++11 constexpr recursion

...
will do more calls without having to manipulate the pre-defined limit.
True, for C++14 I could use a loop for constexpr as well.
...
...
[snip]
...
...
The adapted flip function _is a_ phoenix actor due to
inheritance.
And thats the problem right there. It shouldn't still be a phoenix
 actor after inheritance. Using fit::lazy, it becomes a bind
 expression, however, after using flip, this is no longer the case:
auto lazy_sum = lazy(_+_)(_1, _2);
 auto f = flip(lazy_sum);
static_assert(std::is_bind_expression::value,
"A
...
bind expression");
 static_assert(!std::is_bind_expression::value, "Not 
a
 bind expression");
Furthermore, your example doesn't compile when using Fit's
 placeholders.
This is because for a trait `T` is a better match than its base. But the
base is definitely there:
namespace test {
    struct compress_;
    struct encrypt_;
    struct base64_;
template<typename>
    struct interface {
      using bytes = std::vectorstd::uint8_t;
bytes operator()(bytes const&) { return bytes{}; };
    };
constexpr const interface compress{};
    constexpr const interface encrypt{};
    constexpr const interface base64{};
    constexpr const auto output = fit::flow(compress, encrypt, base64);
constexpr bool is_interface(...) { return false; }
template<typename T>
    constexpr bool is_interface(interface<T> const&) { return 
true; }
constexpr bool is_encrypt(...) { return false; }
    constexpr bool is_encrypt(interface const&) {
      return true; 
    } 
  }
int main() {
    static_assert(test::is_interface(test::compress), "passed");
    static_assert(test::is_interface(test::encrypt), "passed");
    static_assert(test::is_interface(test::base64), "passed");
    static_assert(test::is_interface(test::output), "passed");
static_assert(!test::is_encrypt(test::compress), "passed");
    static_assert(test::is_encrypt(test::encrypt), "passed");
    static_assert(!test::is_encrypt(test::base64), "passed");
    static_assert(test::is_encrypt(test::output), "passed");
return 0;
  }
In this context it might make sense for `output` to be considered an
encryption, compression, and base64 function, since it is composed of
those parts, but does it always make sense? Is a function adapted by
`fit::flip`, etc., always related to the original in academic OOP
concepts and should it be related in the C++ type system? My example
from Phoenix earlier was dismissed as being an issue with its design,
but it cannot always be a flaw to have a callable with associated

...
functions?
However, if it doesn't make sense to override the class's behavior because of
its associated functions then the class should be declared final.
...
Admittedly, I do need to think about this some more to know if there
are other issues in common usage other than what Steven and I have
pointed out.
...
[snip]
...
...
For instance, should there be some documentation explaining how the
library "tests" for `operator()` in certain adaptors, and
how
...
templates
(SFINAE/hard errrors) play a role:
int main() {
    namespace arg = boost::phoenix::placeholders;
    const auto negate = (!arg::_1);
    const auto sub = (arg::_1 - arg::_2);
// does not compile, hard-error with one argument call
...
// std::cout << fit::partial(sub)(0)(1) << std::endl;
...
And that hard error comes from phoenix, of course, as it does compile
 using the Fit placeholders:
const auto sub = (_1 - _2);
 std::cout << partial(sub)(0)(1) << std::endl;
...
// outputs -1
    std::cout << fit::partial(sub)(0, 1) << std::endl;
// outputs 1
    std::cout << fit::conditional(negate, sub)(0) <<
std::endl;
...
...
// outputs 1
    std::cout << fit::conditional(negate, sub)(0, 1) <<
std::endl;
...
// does not compile, hard-error with one argument call -
    // the ambiguous overload is irrelevant to the error
...
// std::cout << fit::match(negate, sub)(0) <<
std::endl;
And that works with Fit placeholders.
The example was poor, because I think Phoenix became the focus. Several
of the adaptors require template callables to be SFINAE friendly, or
they will not work. This is an advanced topic, especially when basic
template usage might be tried in a callable.
I cannot find any documentation mentioning the relationship to the
conditional calls of Fit, and templates. Describing how SFINAE works is
definitely out-of-scope. However, I think mentioning that any adaptor
based on conditional calls must be SFINAE friendly when the adapted
function is a templated callable would be a good start. Additionally,
all adaptors that have conditional logic should be explicitly listed
(somehow) to provide a reference back to the section describing
conditional calling. Hopefully this conditional section will also have
a discussion of variadics (both C and C++) and default arguments

...
because as I already mentioned that could surprise some programmers too.
This would be good to add the documentation.
...
...
...
// does not compile, ambiguous (both have variadic overloads)
...
// std::cout << fit::match(negate, sub)(0, 1) <<
std::endl;
This does not, and is a bug. I am going to look into that.
If two functions are variadic, shouldn't it fail to compile? Or are you
thinking of restricting the Fit placeholders to an upward bound on

...
arguments?
The issue is that the following function should not be callable(it is a
compile error but not a substitution failure):

auto f = (_1 - _2);
static_assert(!is_callable::value, "Callable");
That is if the user does not give of enough parameters for the function
placeholders, then the function should not callable. The reason why its not in
this case is because the function `arg` is not SFINAE-friendly when given out-
of-bounds argument index, that is, `arg_c<2>(1)`  should not be callable.
...
Lee