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

Subject: Re: [boost] [Fit] formal review starts today
From: paul Fultz (pfultz2_at_[hidden])
Date: 2016-03-17 02:57:47

> On Wednesday, March 16, 2016 11:54 PM, Lee Clagett <forum_at_[hidden]> wrote:
> > 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 ...? 

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

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<decltype(f), int>::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
>