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

Subject: Re: [boost] [Fit] formal review starts today
From: Lee Clagett (forum_at_[hidden])
Date: 2016-03-17 00:53:18

On Tue, 15 Mar 2016 08:20:27 +0000 (UTC)
paul Fultz <pfultz2_at_[hidden]> wrote:
> > On Monday, March 14, 2016 11:15 PM, Lee Clagett
> > <forum_at_[hidden]> 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 ...? 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.

> >
[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.

> 
[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<decltype(lazy_sum)>::value, "A
> bind expression");
> static_assert(!std::is_bind_expression<decltype(f)>::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::vector<std::uint8_t>;

      bytes operator()(bytes const&) { return bytes{}; };
    };

    constexpr const interface<compress_> compress{};
    constexpr const interface<encrypt_> encrypt{};
    constexpr const interface<base64_> 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<encrypt_> 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? 

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.

> > 
> >     // 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?

Lee