[review][Fit] Review of Fit starts today : September 8 - September 17

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
40 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
A formal review of the Fit library developed by Paul Fultz II starts
today, September 8, and runs through September 17.

A branch has been made for the review. You can find Fit for review at
these links:

Source: https://github.com/pfultz2/Fit/tree/boost
Docs: http://pfultz2.github.io/Fit/doc/html/doc/

Anyone who is able to provide an assessment of the design,
implementation, and/or docs can participate in this review. Usage
experience is not absolutely necessary to give feedback, but is highly
recommended if you are writing a review. Discussions about aspects of
the library are welcome even if a complete review is not presented,
though participants are asked to please state whether they would like
to accept or reject the library before the deadline of September 17,
paying close attention to the questions raised at the end of this
email. The success of Boost is partially a result of the quality
review process which is conducted by the community. You are part of
the Boost community. We will be grateful to receive a review based on
whatever level of effort or time you can devote.

This is the second time that Fit will go through the review process,
with the previous time being in March of 2016. The result of the
initial review can be found here:
https://lists.boost.org/Archives/boost/2016/04/228770.php

In this second review, those who choose to participate should be as
rigorous as they'd be in any other review, but are also encouraged to
voice whether or not the concerns from March of 2016 were addressed.

The following is a brief description of the library from its author:

====================

Fit is a header-only C++11/C++14 library that provides utilities for
functions and function objects, which can solve many problems with
much simpler constructs than whats traditionally been done with
metaprogramming.

Fit is:

Modern: Fit takes advantages of modern C++11/C++14 features. It
support both constexpr initialization and constexpr evaluation of
functions. It takes advantage of type deduction, varidiac templates,
and perfect forwarding to provide a simple and modern interface.

Relevant: Fit provides utilities for functions and does not try to
implement a functional language in C++. As such, Fit solves many
problems relevant to C++ programmers, including initialization of
function objects and lambdas, overloading with ordering, improved
return type deduction, and much more.

Lightweight: Fit builds simple lightweight abstraction on top of
function objects. It does not require subscribing to an entire
framework. Just use the parts you need.
Fit is divided into three components:

Function Adaptors and Decorators: These enhance functions with
additional capability.
Functions: These return functions that achieve a specific purpose.
Utilities: These are general utilities that are useful when defining
or using functions.

Requirements

This requires a C++11 compiler. There are no third-party dependencies.
This has been tested on clang 3.5-3.8, gcc 4.6-6.2, and Visual Studio
2015. Gcc 5.1 is not supported at all, however, gcc 5.4 is supported.


Contexpr support:

Both MSVC and gcc 4.6 have limited constexpr support due to many bugs
in the implementation of constexpr. However, constexpr initialization
of functions is supported when using the FIT_STATIC_FUNCTION and
FIT_STATIC_LAMBDA_FUNCTION constructs.


Noexcept support:

On older compilers such as gcc 4.6 and gcc 4.7, noexcept is not used
due to many bugs in the implementation. Also, most compilers don’t
support deducing noexcept with member function pointers. Only newer
versions of gcc(4.9 and later) support this.

====================

Please provide in your review whatever information you think is
valuable to understand your final choice of ACCEPT or REJECT including
Fit as a Boost library. Please be explicit about your decision.

Some other questions you might want to consider answering:

  - What is your evaluation of the design?
  - What is your evaluation of the implementation?
  - What is your evaluation of the documentation?
  - What is your evaluation of the potential usefulness of the library?
  - Did you try to use the library? With which compiler(s)? Did you
    have any problems?
  - How much effort did you put into your evaluation? A glance? A quick
    reading? In-depth study?
  - Are you knowledgeable about the problem domain?
  - Were the concerns from the March 2016 review of Fit addressed?

More information about the Boost Formal Review Process can be found here:
http://www.boost.org/community/reviews.html

Thank you for participating!

--
-Matt Calabrese

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list

________________________________________
From: Boost [[hidden email]] on behalf of Matt Calabrese via Boost [[hidden email]]
Sent: 08 September 2017 12:02
To: Boost Developers List
Cc: Matt Calabrese
Subject: [boost] [review][Fit] Review of Fit starts today : September 8 - September 17

> A formal review of the Fit library developed by Paul Fultz II starts
> today, September 8, and runs through September 17.

.....


>This is the second time that Fit will go through the review process,
>with the previous time being in March of 2016. The result of the
>initial review can be found here:

> In this second review, those who choose to participate should be as
> rigorous as they'd be in any other review, but are also encouraged to
> voice whether or not the concerns from March of 2016 were addressed.

I looked at the previous review and found the following:

-----------------------------------------------
Detailed Report
===============

You will need to wait yet for a month before I will be able to do it.
On the mean time there are a lot of issues that must be addressed and
that are part of the github issues.

------------------------------------------------

Is that detailed report available somewhere please?

Thanks

John Fletcher

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list

> On Sep 8, 2017, at 7:04 AM, Fletcher, John P via Boost <[hidden email]> wrote:
>
> I looked at the previous review and found the following:
>
> -----------------------------------------------
> Detailed Report
> ===============
>
> You will need to wait yet for a month before I will be able to do it.
> On the mean time there are a lot of issues that must be addressed and
> that are part of the github issues.
>
> ------------------------------------------------
>
> Is that detailed report available somewhere please?

No, a detailed report was never published.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list

________________________________________
From: P F [[hidden email]]
Sent: 08 September 2017 14:06
To: [hidden email]
Cc: Fletcher, John P
Subject: Re: [boost] [review][Fit] Review of Fit starts today : September 8 - September 17


> > Is that detailed report available somewhere please?

> No, a detailed report was never published.

Oh well.  I have just started to look at this in comparison with some examples I made with a version downloaded in February 2017.  I have not found any list of the changes.  I have noted the following.

1.  Change namespace to boost::fit
2.  Relocation of header files
3.  Add BOOST_ to macro names.

All of those I expected.

Also two of the examples I had done no longer work as "compress" and "reverse_compress" have disappeared.

Have they been renamed to something else or removed completely?

Are there any other changes please?

Best wishes

John


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
On Fri, 2017-09-08 at 15:14 +0000, Fletcher, John P wrote:

> ________________________________________
> From: P F [[hidden email]]
> Sent: 08 September 2017 14:06
> To: [hidden email]
> Cc: Fletcher, John P
> Subject: Re: [boost] [review][Fit] Review of Fit starts today : September 8
> - September 17
>
>
> >
> > >
> > > Is that detailed report available somewhere please?
> >
> > No, a detailed report was never published.
> Oh well.  I have just started to look at this in comparison with some
> examples I made with a version downloaded in February 2017.  I have not
> found any list of the changes.  I have noted the following.
>
> 1.  Change namespace to boost::fit
> 2.  Relocation of header files
> 3.  Add BOOST_ to macro names.
>
> All of those I expected.
>
> Also two of the examples I had done no longer work as "compress" and
> "reverse_compress" have disappeared.
>
> Have they been renamed to something else or removed completely?

These have been renamed to `fold` and `reverse_fold`.

>
> Are there any other changes please?

There are no other breaking changes. Other changes have been improvements to
take advantage of C++17 features where possible, support for msvc 2017, more
testing was added, some documentation improvements, and some minor fixes.


.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
Le 08/09/2017 à 14:04, Fletcher, John P via Boost a écrit :

> ________________________________________
> From: Boost [[hidden email]] on behalf of Matt Calabrese via Boost [[hidden email]]
> Sent: 08 September 2017 12:02
> To: Boost Developers List
> Cc: Matt Calabrese
> Subject: [boost] [review][Fit] Review of Fit starts today : September 8 - September 17
>
>> A formal review of the Fit library developed by Paul Fultz II starts
>> today, September 8, and runs through September 17.
> .....
>
>
>> This is the second time that Fit will go through the review process,
>> with the previous time being in March of 2016. The result of the
>> initial review can be found here:
>> In this second review, those who choose to participate should be as
>> rigorous as they'd be in any other review, but are also encouraged to
>> voice whether or not the concerns from March of 2016 were addressed.
> I looked at the previous review and found the following:
>
> -----------------------------------------------
> Detailed Report
> ===============
>
> You will need to wait yet for a month before I will be able to do it.
> On the mean time there are a lot of issues that must be addressed and
> that are part of the github issues.
>
> ------------------------------------------------
>
> Is that detailed report available somewhere please?
>
Hi,

I'm really sorry. I didn't take the time to write such a report :(
My apologies.

Vicente

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Fri, Sep 8, 2017 at 6:02 AM, Matt Calabrese via Boost <
[hidden email]> wrote:

> A formal review of the Fit library developed by Paul Fultz II starts
> today, September 8, and runs through September 17.
>
> A branch has been made for the review. You can find Fit for review at
> these links:
>
> Source: https://github.com/pfultz2/Fit/tree/boost
> Docs: http://pfultz2.github.io/Fit/doc/html/doc/
>

[snip]


> Please provide in your review whatever information you think is
> valuable to understand your final choice of ACCEPT or REJECT including
> Fit as a Boost library. Please be explicit about your decision.
>

I vote to accept Fit.  I would like to see some naming changes, but I'm not
making any of them conditions for acceptance, because I do not consider any
of them to be deal-breakers.

Some other questions you might want to consider answering:
>
>   - What is your evaluation of the design?
>

(Still) good.  Here's what I wrote in the last review:

I particularly like the fact that it attempts to keep everything constexpr-
and SFINAE-friendly wherever possible.

I *really* like reveal() and failure(). Too few TMP-heavy libraries
pay attention to providing good error messages.

I have no problem with the constness requirements on Callables used
throughout Fit, as was discussed previously. I even consider this a
feature, allowing me to detect non-const Callables, which are almost always
an error on my part. This is especially true since std::ref() and
mutable_() let me explicitly choose how to handle mutable Callables, in a
way that is also explicit at the call site.

There are a number of adaptors that I find have obscure names:

by() is a projection; why not call it project()?

flow() is a super obscure name! Why is this not reverse_compose() or
similar?

indirect() could more easily be understood if it were called deref()
or dereference().

I don't feel as strongly about partial(), but I think it might be clearer
if
it were called partial_apply() or curry().

conditional() should be called invoke_first(), call_first_of(), or
similar.  I find it too easy to confuse with if_().

For that matter, it would be nice if "if_()" were called "if_constexpr()",
since that seems to match its semantics, and since there's a language
feature that already does this; a reader of uses of "if_constexpr()" will
get the intent faster than they would if they read "if_()".

>   - What is your evaluation of the implementation?
>

I did not look at it much, but what I did look at had no readily apparent
issues.


>   - What is your evaluation of the documentation?
>

It has gotten dramatically better since the last review.  The non-reference
sections now provide a thorough tour of the library and its capabilities.

I took issue with some of the reference docs in the last review; those have
all been addressed.


>   - What is your evaluation of the potential usefulness of the library?
>

I think the library is very useful.  For example, I had to do some somewhat
hairy overload-resolution metaprogramming in an expression template library
I've been working on, and I really wished conditional() were already in
Boost so I could have used it instead.


>   - Did you try to use the library? With which compiler(s)? Did you
>     have any problems?
>

I did this quite a bit the first time around, and it was very easy to use
the part I care about the most (conditional()), using Clang 3.6.  The same
code works fine with Clang 4, but I did not do anything with the new
version of the library more than what I did with the old one.

  - How much effort did you put into your evaluation? A glance? A quick
>     reading? In-depth study?
>

I spent 5-6 hours on the first review, and about 2 hours this time.


>   - Are you knowledgeable about the problem domain?
>

Yes.  I do a lot of generic programming.


>   - Were the concerns from the March 2016 review of Fit addressed?
>

 Many of them were.  The only ones that were not were related to naming, as
repeated above.  As I said, I don't consider any of those naming choices to
be blockers, just suboptimal.

Zach

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
On Mon, 2017-09-11 at 11:55 -0500, Zach Laine via Boost wrote:

> On Fri, Sep 8, 2017 at 6:02 AM, Matt Calabrese via Boost <
> [hidden email]> wrote:
>
> >
> > A formal review of the Fit library developed by Paul Fultz II starts
> > today, September 8, and runs through September 17.
> >
> > A branch has been made for the review. You can find Fit for review at
> > these links:
> >
> > Source: https://github.com/pfultz2/Fit/tree/boost
> > Docs: http://pfultz2.github.io/Fit/doc/html/doc/
> >
> [snip]
>
>
> >
> > Please provide in your review whatever information you think is
> > valuable to understand your final choice of ACCEPT or REJECT including
> > Fit as a Boost library. Please be explicit about your decision.
> >
> I vote to accept Fit.  I would like to see some naming changes, but I'm not
> making any of them conditions for acceptance, because I do not consider any
> of them to be deal-breakers.

Thanks, Zach, for the review.

>
> Some other questions you might want to consider answering:
> >
> >
> >   - What is your evaluation of the design?
> >
> (Still) good.  Here's what I wrote in the last review:
>
> I particularly like the fact that it attempts to keep everything constexpr-
> and SFINAE-friendly wherever possible.
>
> I *really* like reveal() and failure(). Too few TMP-heavy libraries
> pay attention to providing good error messages.
>
> I have no problem with the constness requirements on Callables used
> throughout Fit, as was discussed previously. I even consider this a
> feature, allowing me to detect non-const Callables, which are almost always
> an error on my part. This is especially true since std::ref() and
> mutable_() let me explicitly choose how to handle mutable Callables, in a
> way that is also explicit at the call site.
>
> There are a number of adaptors that I find have obscure names:
>
> by() is a projection; why not call it project()?

by() is not a projection, rather it takes a projection.

I mainly named it for how it was used, so it would read more english-like:

std::sort(std::begin(people), std::end(people),
        by(&Person::year_of_birth, _ < _));

Like "sort by year_of_birth". In haskell, its called 'on', but it seems 'by'
is more natural for English.

>
> flow() is a super obscure name! Why is this not reverse_compose() or
> similar?

Its because its used with pipable. So it can take a set of pipable functions
so they "flow" together. reverse_compose() could work, but I think the name
looks clunky as an alternative to pipable functions. 

Also, in haskell there is no reverse compose(just `.` for composition),
instead the `>>>` arrow operator can be used to indicate the direction of
composition, but there is no way to implement such an operator in C++.

>
> indirect() could more easily be understood if it were called deref()
> or dereference().

This named after the indirect in the Ranges library. Also, I might add a
custom operator in the future, so calling it deref might not make sense.

>
> I don't feel as strongly about partial(), but I think it might be clearer
> if
> it were called partial_apply() or curry().

Which there could be an uncurry function, but I've always understood curry to
be for one arg at a time.

Either waty, I could provide curry for repeated partial applications(ie what
partial is currently). Then provide partial and reverse_partial like hana for
a single partial application. I just wonder if this bloats the interface, as
we will have a lot of different functions for partial application.

>
> conditional() should be called invoke_first(), call_first_of(), or
> similar.  I find it too easy to confuse with if_().

I could call it first_of(). I do see the confusion with `std::conditional`.

>
> For that matter, it would be nice if "if_()" were called "if_constexpr()",
> since that seems to match its semantics, and since there's a language
> feature that already does this; a reader of uses of "if_constexpr()" will
> get the intent faster than they would if they read "if_()".

To work like if_constexpr(), eval() would needed to be used. There are of
course other ways to use it(especially in point-free programming) that is not
an emulation of `if constexpr`, like how I implement `yield_if` here:

https://github.com/pfultz2/Hero/blob/master/include/hero/for_each.h#L43

>
> >
> >   - What is your evaluation of the implementation?
> >
> I did not look at it much, but what I did look at had no readily apparent
> issues.
>
>
> >
> >   - What is your evaluation of the documentation?
> >
> It has gotten dramatically better since the last review.  The non-reference
> sections now provide a thorough tour of the library and its capabilities.
>
> I took issue with some of the reference docs in the last review; those have
> all been addressed.
>
>
> >
> >   - What is your evaluation of the potential usefulness of the library?
> >
> I think the library is very useful.  For example, I had to do some somewhat
> hairy overload-resolution metaprogramming in an expression template library
> I've been working on, and I really wished conditional() were already in
> Boost so I could have used it instead.

Well, hopefully it will be there soon.

Thanks,
Paul

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 9/8/2017 7:02 AM, Matt Calabrese via Boost wrote:
> A formal review of the Fit library developed by Paul Fultz II starts
> today, September 8, and runs through September 17.
>
> A branch has been made for the review.

And that branch is ... ?

> You can find Fit for review at
> these links:
>
> Source: https://github.com/pfultz2/Fit/tree/boost
> Docs: http://pfultz2.github.io/Fit/doc/html/doc/
>
> Anyone who is able to provide an assessment of the design,
> implementation, and/or docs can participate in this review. Usage
> experience is not absolutely necessary to give feedback, but is highly
> recommended if you are writing a review. Discussions about aspects of
> the library are welcome even if a complete review is not presented,
> though participants are asked to please state whether they would like
> to accept or reject the library before the deadline of September 17,
> paying close attention to the questions raised at the end of this
> email. The success of Boost is partially a result of the quality
> review process which is conducted by the community. You are part of
> the Boost community. We will be grateful to receive a review based on
> whatever level of effort or time you can devote.
>
> This is the second time that Fit will go through the review process,
> with the previous time being in March of 2016. The result of the
> initial review can be found here:
> https://lists.boost.org/Archives/boost/2016/04/228770.php
>
> In this second review, those who choose to participate should be as
> rigorous as they'd be in any other review, but are also encouraged to
> voice whether or not the concerns from March of 2016 were addressed.
>
> The following is a brief description of the library from its author:
>
> ====================
>
> Fit is a header-only C++11/C++14 library that provides utilities for
> functions and function objects, which can solve many problems with
> much simpler constructs than whats traditionally been done with
> metaprogramming.
>
> Fit is:
>
> Modern: Fit takes advantages of modern C++11/C++14 features. It
> support both constexpr initialization and constexpr evaluation of
> functions. It takes advantage of type deduction, varidiac templates,
> and perfect forwarding to provide a simple and modern interface.
>
> Relevant: Fit provides utilities for functions and does not try to
> implement a functional language in C++. As such, Fit solves many
> problems relevant to C++ programmers, including initialization of
> function objects and lambdas, overloading with ordering, improved
> return type deduction, and much more.
>
> Lightweight: Fit builds simple lightweight abstraction on top of
> function objects. It does not require subscribing to an entire
> framework. Just use the parts you need.
> Fit is divided into three components:
>
> Function Adaptors and Decorators: These enhance functions with
> additional capability.
> Functions: These return functions that achieve a specific purpose.
> Utilities: These are general utilities that are useful when defining
> or using functions.
>
> Requirements
>
> This requires a C++11 compiler. There are no third-party dependencies.
> This has been tested on clang 3.5-3.8, gcc 4.6-6.2, and Visual Studio
> 2015. Gcc 5.1 is not supported at all, however, gcc 5.4 is supported.
>
>
> Contexpr support:
>
> Both MSVC and gcc 4.6 have limited constexpr support due to many bugs
> in the implementation of constexpr. However, constexpr initialization
> of functions is supported when using the FIT_STATIC_FUNCTION and
> FIT_STATIC_LAMBDA_FUNCTION constructs.
>
>
> Noexcept support:
>
> On older compilers such as gcc 4.6 and gcc 4.7, noexcept is not used
> due to many bugs in the implementation. Also, most compilers don’t
> support deducing noexcept with member function pointers. Only newer
> versions of gcc(4.9 and later) support this.
>
> ====================
>
> Please provide in your review whatever information you think is
> valuable to understand your final choice of ACCEPT or REJECT including
> Fit as a Boost library. Please be explicit about your decision.
>
> Some other questions you might want to consider answering:
>
>    - What is your evaluation of the design?
>    - What is your evaluation of the implementation?
>    - What is your evaluation of the documentation?
>    - What is your evaluation of the potential usefulness of the library?
>    - Did you try to use the library? With which compiler(s)? Did you
>      have any problems?
>    - How much effort did you put into your evaluation? A glance? A quick
>      reading? In-depth study?
>    - Are you knowledgeable about the problem domain?
>    - Were the concerns from the March 2016 review of Fit addressed?
>
> More information about the Boost Formal Review Process can be found here:
> http://www.boost.org/community/reviews.html
>
> Thank you for participating!
>



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list

> On Sep 13, 2017, at 7:47 PM, Edward Diener via Boost <[hidden email]> wrote:
>
> On 9/8/2017 7:02 AM, Matt Calabrese via Boost wrote:
>> A formal review of the Fit library developed by Paul Fultz II starts
>> today, September 8, and runs through September 17.
>> A branch has been made for the review.
>
> And that branch is … ?

Its the branch boost. Thats what the source links to.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
Here is my review of the FIT library

>Please provide in your review whatever information you think is
>valuable to understand your final choice of ACCEPT or REJECT including
>Fit as a Boost library. Please be explicit about your decision.


I vote to ACCEPT  FIT as a Boost library.

> Some other questions you might want to consider answering:
>  - What is your evaluation of the design?
>  - What is your evaluation of the implementation?

I have found both the design and the implementation to be consistent and well carried through.
There are a few things which are in the detail which are in fact of use to users and it would be helpful it they were available in the fit namespace. See notes below.

>  - What is your evaluation of the documentation?

The general standard of the reference documentation is good.
It would help to have some more examples.
There are some things which are not documented such as alias and the configuration.  Also move and forward which are in detail although useful to users.

  - What is your evaluation of the potential usefulness of the library?

I think this is a very useful library.  I have explored this by building an example use - see notes below.

  - Did you try to use the library? With which compiler(s)? Did you
    have any problems?

Yes, I have built examples to run everything I can find in the library.  I did this with the version available in February this year and have moved the examples over to the Boost version with the necessary changes to namespace and boostification of the macros.  All worked after I found the name changes to compress/reverse compress to fold/reverse_fold.  I see that this is a response to previous discussion and I agree with the changes.

  - How much effort did you put into your evaluation? A glance? A quick
    reading? In-depth study?

I have been working with FIT for some time and building the examples.  I have mainly used various versions of clang and libc++, also using various versions of gcc.
I have discovered that not all of the compiler configuration is carried out in fit/configure.hpp.  The following headers also detect the compiler being used:  alias, always, implicit, pack, reveal, unpack.

I have built examples of all of the code I can find, including for alias where there is no documentation and only test examples for some of the things in the header.  It is used in pack and combine and may have been intended to be internal although it can be used externally.

In some of the examples I have combined FIT with boost function and boost phoenix

  - Are you knowledgeable about the problem domain?

I have been able to compare the operations of FIT with other implementations of similar code and found that FIT can go far beyond what was possible using C++.  I have been active in this area for longer that I care to remember (> 10 years).

  - Were the concerns from the March 2016 review of Fit addressed?

This is hard to answer as I could not find the full report on the first review.  I think that it is the case.

> Thank you for participating!

Thank you Paul for the library.  I look forward to making use of it.

Best wishes

John Fletcher


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
On Fri, 2017-09-15 at 10:48 +0000, Fletcher, John P via Boost wrote:
> Here is my review of the FIT library
>
> >
> > Please provide in your review whatever information you think is
> > valuable to understand your final choice of ACCEPT or REJECT including
> > Fit as a Boost library. Please be explicit about your decision.
>
> I vote to ACCEPT  FIT as a Boost library.

Thanks, for the review.

>
> >
> > Some other questions you might want to consider answering:
> >  - What is your evaluation of the design?
> >  - What is your evaluation of the implementation?
> I have found both the design and the implementation to be consistent and
> well carried through.
> There are a few things which are in the detail which are in fact of use to
> users and it would be helpful it they were available in the fit namespace.
> See notes below.
>
> >
> >  - What is your evaluation of the documentation?
> The general standard of the reference documentation is good.
> It would help to have some more examples.

What kind of more examples would you like to see?

> There are some things which are not documented such as alias and the
> configuration.  Also move and forward which are in detail although useful to
> users.
>
>   - What is your evaluation of the potential usefulness of the library?
>
> I think this is a very useful library.  I have explored this by building an
> example use - see notes below.
>
>   - Did you try to use the library? With which compiler(s)? Did you
>     have any problems?
>
> Yes, I have built examples to run everything I can find in the library.  I
> did this with the version available in February this year and have moved the
> examples over to the Boost version with the necessary changes to namespace
> and boostification of the macros.  All worked after I found the name changes
> to compress/reverse compress to fold/reverse_fold.  I see that this is a
> response to previous discussion and I agree with the changes.
>
>   - How much effort did you put into your evaluation? A glance? A quick
>     reading? In-depth study?
>
> I have been working with FIT for some time and building the examples.  I
> have mainly used various versions of clang and libc++, also using various
> versions of gcc.
> I have discovered that not all of the compiler configuration is carried out
> in fit/configure.hpp.  The following headers also detect the compiler being
> used:  alias, always, implicit, pack, reveal, unpack.

Some of those is because there is no specific compiler feature it is working
around. Although in a few places I could use another config.

>
> I have built examples of all of the code I can find, including for alias
> where there is no documentation and only test examples for some of the
> things in the header.  It is used in pack and combine and may have been
> intended to be internal although it can be used externally.

It is intended to be used externally, which is why there is an initial
documentation, and it is not in the detail namespace. Many didn't see how it
is related to this library, plus it was missing a useful example. So for now,
I just took it out of the TOC, but would like to add it back, but need a good
motivating example.

Thanks,
Paul


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 9/8/17 4:02 AM, Matt Calabrese via Boost wrote:
> A formal review of the Fit library developed by Paul Fultz II starts
> today, September 8, and runs through September 17.
>
> A branch has been made for the review. You can find Fit for review at
> these links:
>
> Source: https://github.com/pfultz2/Fit/tree/boost
> Docs: http://pfultz2.github.io/Fit/doc/html/doc/
>

I'm beginning to take a look at the documentation and immediatly I have
question:

How does this contrast with recently accepted Boost.CallableTraits and
Boost.FunctionTypes?

Reading the introduction About and Motivation I would like to see a
simple example of a problem solved by the library.  I should be enough
to make me want to investigate the library further or help me decide
that it's not a good fit for what I want to do.

I don't like names like Fit which give no helpful information.  But I've
lost that battle before so I won't belabor this any more.

Robert Ramey

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
On 9/15/17 11:45 AM, Robert Ramey via Boost wrote:

I much like the "Getting Started" page- it's really helpful and clearly
presented.  So maybe my critcism came off harsher than it should be.

Robert Ramey

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 9/8/2017 7:02 AM, Matt Calabrese via Boost wrote:
> A formal review of the Fit library developed by Paul Fultz II starts
> today, September 8, and runs through September 17.
>
> A branch has been made for the review. You can find Fit for review at
> these links:
>
> Source: https://github.com/pfultz2/Fit/tree/boost
> Docs: http://pfultz2.github.io/Fit/doc/html/doc/

I have some questions about Fit which I can not understand by reading
the docs:

1) Does Fit functionality work only with function objects as opposed to
callables in general ?

2) Is the purpose of BOOST_FIT_STATIC_FUNCTION only to create a function
object at global or namespace level ?

3) Does BOOST_FIT_STATIC_FUNCTION only accept a function object or does
it accept any callable ?

4) The documentation says that "BOOST_FIT_STATIC_LAMBDA_FUNCTION can be
used to the declare the lambda as a function". I am not sure what this
actually means since a lambda function is a function.

5) I do not understand the purpose of the pipable adaptor. What is the
point of using it ? It is mentioned as an alternative to Unified Call
Syntax but, while I understand the latter, I do not understandable how
pipable allows "extension methods", which is a large part of the purpose
of UFCS.

In general I am confused by what the requirements of Fit adaptors are.
This may be because the terms "function objects" and "functions" are
used in a way which does not follow standard C++ terminology as far as I
know it. I think I complained about this confusion in my comments
regarding Fit in the earlier review, but while the docs seem more
extensive than earlier, my confusion still remains.

>
> Anyone who is able to provide an assessment of the design,
> implementation, and/or docs can participate in this review. Usage
> experience is not absolutely necessary to give feedback, but is highly
> recommended if you are writing a review. Discussions about aspects of
> the library are welcome even if a complete review is not presented,
> though participants are asked to please state whether they would like
> to accept or reject the library before the deadline of September 17,
> paying close attention to the questions raised at the end of this
> email. The success of Boost is partially a result of the quality
> review process which is conducted by the community. You are part of
> the Boost community. We will be grateful to receive a review based on
> whatever level of effort or time you can devote.
>
> This is the second time that Fit will go through the review process,
> with the previous time being in March of 2016. The result of the
> initial review can be found here:
> https://lists.boost.org/Archives/boost/2016/04/228770.php
>
> In this second review, those who choose to participate should be as
> rigorous as they'd be in any other review, but are also encouraged to
> voice whether or not the concerns from March of 2016 were addressed.
>
> The following is a brief description of the library from its author:
>
> ====================
>
> Fit is a header-only C++11/C++14 library that provides utilities for
> functions and function objects, which can solve many problems with
> much simpler constructs than whats traditionally been done with
> metaprogramming.
>
> Fit is:
>
> Modern: Fit takes advantages of modern C++11/C++14 features. It
> support both constexpr initialization and constexpr evaluation of
> functions. It takes advantage of type deduction, varidiac templates,
> and perfect forwarding to provide a simple and modern interface.
>
> Relevant: Fit provides utilities for functions and does not try to
> implement a functional language in C++. As such, Fit solves many
> problems relevant to C++ programmers, including initialization of
> function objects and lambdas, overloading with ordering, improved
> return type deduction, and much more.
>
> Lightweight: Fit builds simple lightweight abstraction on top of
> function objects. It does not require subscribing to an entire
> framework. Just use the parts you need.
> Fit is divided into three components:
>
> Function Adaptors and Decorators: These enhance functions with
> additional capability.
> Functions: These return functions that achieve a specific purpose.
> Utilities: These are general utilities that are useful when defining
> or using functions.
>
> Requirements
>
> This requires a C++11 compiler. There are no third-party dependencies.
> This has been tested on clang 3.5-3.8, gcc 4.6-6.2, and Visual Studio
> 2015. Gcc 5.1 is not supported at all, however, gcc 5.4 is supported.
>
>
> Contexpr support:
>
> Both MSVC and gcc 4.6 have limited constexpr support due to many bugs
> in the implementation of constexpr. However, constexpr initialization
> of functions is supported when using the FIT_STATIC_FUNCTION and
> FIT_STATIC_LAMBDA_FUNCTION constructs.
>
>
> Noexcept support:
>
> On older compilers such as gcc 4.6 and gcc 4.7, noexcept is not used
> due to many bugs in the implementation. Also, most compilers don’t
> support deducing noexcept with member function pointers. Only newer
> versions of gcc(4.9 and later) support this.
>
> ====================
>
> Please provide in your review whatever information you think is
> valuable to understand your final choice of ACCEPT or REJECT including
> Fit as a Boost library. Please be explicit about your decision.
>
> Some other questions you might want to consider answering:
>
>    - What is your evaluation of the design?
>    - What is your evaluation of the implementation?
>    - What is your evaluation of the documentation?
>    - What is your evaluation of the potential usefulness of the library?
>    - Did you try to use the library? With which compiler(s)? Did you
>      have any problems?
>    - How much effort did you put into your evaluation? A glance? A quick
>      reading? In-depth study?
>    - Are you knowledgeable about the problem domain?
>    - Were the concerns from the March 2016 review of Fit addressed?
>
> More information about the Boost Formal Review Process can be found here:
> http://www.boost.org/community/reviews.html
>
> Thank you for participating!
>



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list

> On Sep 15, 2017, at 1:45 PM, Robert Ramey via Boost <[hidden email]> wrote:
>
> On 9/8/17 4:02 AM, Matt Calabrese via Boost wrote:
>> A formal review of the Fit library developed by Paul Fultz II starts
>> today, September 8, and runs through September 17.
>> A branch has been made for the review. You can find Fit for review at
>> these links:
>> Source: https://github.com/pfultz2/Fit/tree/boost
>> Docs: http://pfultz2.github.io/Fit/doc/html/doc/
>
> I'm beginning to take a look at the documentation and immediatly I have question:
>
> How does this contrast with recently accepted Boost.CallableTraits and Boost.FunctionTypes?

Those seems more about querying properties about functions, which is different than the Fit library. Although, the Fit library does provide a `is_callable` trait which is similar to `is_invocable` in CallableTraits, but Fit implements this without using substitution failure on MSVC because it is problematic.

>
> Reading the introduction About and Motivation I would like to see a simple example of a problem solved by the library.  I should be enough to make me want to investigate the library further or help me decide that it's not a good fit for what I want to do.
>
> I don't like names like Fit which give no helpful information.  But I've lost that battle before so I won't belabor this any more.

True, I really like Vicente’s suggestion of HigherOrderFunctions.



.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list

> On Sep 15, 2017, at 8:46 PM, Edward Diener via Boost <[hidden email]> wrote:
>
> On 9/8/2017 7:02 AM, Matt Calabrese via Boost wrote:
>> A formal review of the Fit library developed by Paul Fultz II starts
>> today, September 8, and runs through September 17.
>> A branch has been made for the review. You can find Fit for review at
>> these links:
>> Source: https://github.com/pfultz2/Fit/tree/boost <https://github.com/pfultz2/Fit/tree/boost>
>> Docs: http://pfultz2.github.io/Fit/doc/html/doc/ <http://pfultz2.github.io/Fit/doc/html/doc/>
>
> I have some questions about Fit which I can not understand by reading the docs:
>
> 1) Does Fit functionality work only with function objects as opposed to callables in general ?

In general yes, but its not the case for every adaptor.

>
> 2) Is the purpose of BOOST_FIT_STATIC_FUNCTION only to create a function object at global or namespace level ?

Yes.

>
> 3) Does BOOST_FIT_STATIC_FUNCTION only accept a function object or does it accept any callable ?

It only works with function objects, which I should document its type requirements.

>
> 4) The documentation says that "BOOST_FIT_STATIC_LAMBDA_FUNCTION can be used to the declare the lambda as a function". I am not sure what this actually means since a lambda function is a function.

Maybe that could be worded better, but it lets you declare the lambda at global or namespace scope.

>
> 5) I do not understand the purpose of the pipable adaptor. What is the point of using it ? It is mentioned as an alternative to Unified Call Syntax but, while I understand the latter, I do not understandable how pipable allows "extension methods", which is a large part of the purpose of UFCS.

That is, you want to add an extension method to a class so it can be called as `x.f(y)`. With UFCS, you can just declare a free function called as `f(x, y)` and then call it as `x.f(y)`, with the Fit library you would declare the function as pipable and then call it as `x | f(y)`.

>
> In general I am confused by what the requirements of Fit adaptors are.

Which ones?

> This may be because the terms "function objects" and "functions" are used in a way which does not follow standard C++ terminology as far as I know it.

The type requirements for the adaptors are documented as either `ConstCallable` or `ConstFunctionObject`, which follows standard C++ definitions with an additional `const` requirement.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
On 9/16/2017 2:14 PM, P F via Boost wrote:

>
>> On Sep 15, 2017, at 8:46 PM, Edward Diener via Boost <[hidden email]> wrote:
>>
>> On 9/8/2017 7:02 AM, Matt Calabrese via Boost wrote:
>>> A formal review of the Fit library developed by Paul Fultz II starts
>>> today, September 8, and runs through September 17.
>>> A branch has been made for the review. You can find Fit for review at
>>> these links:
>>> Source: https://github.com/pfultz2/Fit/tree/boost <https://github.com/pfultz2/Fit/tree/boost>
>>> Docs: http://pfultz2.github.io/Fit/doc/html/doc/ <http://pfultz2.github.io/Fit/doc/html/doc/>
>>
>> I have some questions about Fit which I can not understand by reading the docs:
>>
>> 1) Does Fit functionality work only with function objects as opposed to callables in general ?
>
> In general yes, but its not the case for every adaptor.

OK.

>
>>
>> 2) Is the purpose of BOOST_FIT_STATIC_FUNCTION only to create a function object at global or namespace level ?
>
> Yes.

OK.

>
>>
>> 3) Does BOOST_FIT_STATIC_FUNCTION only accept a function object or does it accept any callable ?
>
> It only works with function objects, which I should document its type requirements.

Please do.

>
>>
>> 4) The documentation says that "BOOST_FIT_STATIC_LAMBDA_FUNCTION can be used to the declare the lambda as a function". I am not sure what this actually means since a lambda function is a function.
>
> Maybe that could be worded better, but it lets you declare the lambda at global or namespace scope.

OK.

>
>>
>> 5) I do not understand the purpose of the pipable adaptor. What is the point of using it ? It is mentioned as an alternative to Unified Call Syntax but, while I understand the latter, I do not understandable how pipable allows "extension methods", which is a large part of the purpose of UFCS.
>
> That is, you want to add an extension method to a class so it can be called as `x.f(y)`. With UFCS, you can just declare a free function called as `f(x, y)` and then call it as `x.f(y)`, with the Fit library you would declare the function as pipable and then call it as `x | f(y)`.
>

Let us suppose I have a class:

struct X { some member functions etc. };

Then I want to add a member function to class X with a prototype called
void f(int) so I can call it as an extension method without modifying X.
With UFCS in effect I create the free function:

void f(X px,int py) { some implementation }

and now if I have:

X x;

I can call:

x.f(3); // Works with UFCS support

successfully. Could you please show me the equivalent code that uses
pipable in this same scenario ?

>>
>> In general I am confused by what the requirements of Fit adaptors are.
>
> Which ones?
>
>> This may be because the terms "function objects" and "functions" are used in a way which does not follow standard C++ terminology as far as I know it.
>
> The type requirements for the adaptors are documented as either `ConstCallable` or `ConstFunctionObject`, which follows standard C++ definitions with an additional `const` requirement.

OK.

Thanks !

>
>
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
>



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list

> On Sep 16, 2017, at 2:31 PM, Edward Diener via Boost <[hidden email]> wrote:
>
> On 9/16/2017 2:14 PM, P F via Boost wrote:
>>
>>> 5) I do not understand the purpose of the pipable adaptor. What is the point of using it ? It is mentioned as an alternative to Unified Call Syntax but, while I understand the latter, I do not understandable how pipable allows "extension methods", which is a large part of the purpose of UFCS.
>> That is, you want to add an extension method to a class so it can be called as `x.f(y)`. With UFCS, you can just declare a free function called as `f(x, y)` and then call it as `x.f(y)`, with the Fit library you would declare the function as pipable and then call it as `x | f(y)`.
>
> Let us suppose I have a class:
>
> struct X { some member functions etc. };
>
> Then I want to add a member function to class X with a prototype called void f(int) so I can call it as an extension method without modifying X. With UFCS in effect I create the free function:
>
> void f(X px,int py) { some implementation }
>
> and now if I have:
>
> X x;
>
> I can call:
>
> x.f(3); // Works with UFCS support
>
> successfully. Could you please show me the equivalent code that uses pipable in this same scenario ?

You could define `f` like this(using a lambda for simplification):

BOOST_FIT_STATIC_LAMBDA_FUNCTION(f) = boost::fit::pipable([](X px,int py) { some implementation });

And then you can call it like this:

X x;
x | f(3); // Calls f as f(x, 3)



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review][Fit] Review of Fit starts today : September 8 - September 17

Boost - Dev mailing list
On 9/16/2017 7:53 PM, P F via Boost wrote:

>
>> On Sep 16, 2017, at 2:31 PM, Edward Diener via Boost <[hidden email]> wrote:
>>
>> On 9/16/2017 2:14 PM, P F via Boost wrote:
>>>
>>>> 5) I do not understand the purpose of the pipable adaptor. What is the point of using it ? It is mentioned as an alternative to Unified Call Syntax but, while I understand the latter, I do not understandable how pipable allows "extension methods", which is a large part of the purpose of UFCS.
>>> That is, you want to add an extension method to a class so it can be called as `x.f(y)`. With UFCS, you can just declare a free function called as `f(x, y)` and then call it as `x.f(y)`, with the Fit library you would declare the function as pipable and then call it as `x | f(y)`.
>>
>> Let us suppose I have a class:
>>
>> struct X { some member functions etc. };
>>
>> Then I want to add a member function to class X with a prototype called void f(int) so I can call it as an extension method without modifying X. With UFCS in effect I create the free function:
>>
>> void f(X px,int py) { some implementation }
>>
>> and now if I have:
>>
>> X x;
>>
>> I can call:
>>
>> x.f(3); // Works with UFCS support
>>
>> successfully. Could you please show me the equivalent code that uses pipable in this same scenario ?
>
> You could define `f` like this(using a lambda for simplification):
>
> BOOST_FIT_STATIC_LAMBDA_FUNCTION(f) = boost::fit::pipable([](X px,int py) { some implementation });
>
> And then you can call it like this:
>
> X x;
> x | f(3); // Calls f as f(x, 3)

OK. Thanks !

BTW I do not recall seeing anywhere in the doc where you mention that
Fit constructs are in the boost::fit namespace.

> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
>



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
12