Louis Dionne via Boost
2017-04-17 04:36:00 UTC
Dear Boost,
I am pleased to announce that the CallableTraits library is CONDITIONALLY
ACCEPTED into Boost. I counted the following formal reviews:
- Bruno Dutra: Yes
- Emil Dotchevski: Yes
- John P. Fletcher: Yes
- Klemens Morgenstern: Yes
- Peter Dimov: Yes
- Zach Laine: Yes
The following people also participated to the review by providing informal
feedback:
- Edward Diener
- Gavin Lambert
- Niall Douglas
- Paul Fultz II
- Tim Song
I also based my decision on previous experience I've had using the library,
and some issues we faced in the Standard Committee where we could have used
CallableTraits. I'd like to thank everyone who participated to the review
for their feedback, and also Barrett Adair for his submission.
The following lists the conditions for acceptance, and also summarizes the
general feedback received during the review. GitHub issues are leveraged
to make the tracking of these comments easier. Below, whenever I refer to
#XYZ, the corresponding ticket can be found at
https://github.com/badair/callable_traits/issues/XYZ
To see all the issues that were discussed during the review, you can search
for tickets labeled with `review` on GitHub (please allow some time for
Barrett to update the new issues I just filed):
https://github.com/badair/callable_traits/issues?page=1&q=+label%3Areview
Conditions for acceptance
-------------------------
The following are points that must be addressed for the library to be
accepted. Once all of the associated GitHub issues are closed, I ask
that the author post to this list with links to the closed issues.
Unless major problems (having to do with the closed issues) are raised
at that time, the library will be automatically accepted into Boost,
no additional review required.
- The library needs to clearly document the advantages of CallableTraits
over FunctionTypes. Concrete examples must be given (at least one,
preferably two). See #117.
- The level of C++ conformance required for the whole library needs to be
clearly stated in the documentation. Individual components should only
document divergences from that base level of conformance. The current
state of things is quite confusing for users. See #118.
- The "algorithmic" parts of the library must be removed, since they are
out of scope. This has already been partly done in issue #114, but the
following algorithms must be discussed too (let's follow up in #114):
+ args_at
+ clear_args
+ pop_front_args
+ push_front_args
+ expand_args_right
+ expand_args_left
- The lazy metafunctions and predicate traits should be SFINAE-friendly,
just like the aliases are. I think this inconsistency is a big deal,
enough to be a condition for acceptance. See #134.
Summary of the feedback received
--------------------------------
The following is a compilation of other points that were made during the
review, but which are not requirements for acceptance. I trust, but do not
require, that all of those points will be considered by the author:
- There is a loss of information with the `args` metafunction. For example,
`void(T)` and `void(T, ...)` both give the same result. See #132.
- Consider adding a way of obtaining the parameter list of a function
without the type of `*this`. See #133.
- The documentation uses the terms of art "implementation defined" and
"undefined behavior" incorrectly. See #103 and #131.
- The non-reference documentation should present at least one real-world
use case. See #116.
- The library should handle top-level cv-qualifiers. See #106.
- Consider supporting different calling conventions (e.g. `__cdecl`).
See #102.
- Consider adding a pre-C++17 implementation of `is_invocable` and friends.
See #119.
- The documentation should clearly state that the library can be used
without the rest of Boost. See #99.
- Consider combining `expand_args` and `args` into a single metafunction
with a default "output" parameter of `std::tuple`. See #126.
- Consider adding `make_fn` and `make_mem_fn` metafunctions to construct
a signature from a return type and a tuple of the arguments. See #113.
- Consider renaming some traits, like `is_reference_member`. See #108.
- Reword some parts of the documentation that look odd due to the use of
passive tense. See #105.
- Consider adding a version of `apply_member_pointer` which carries
cv-qualifiers. See #138.
- Consider adding `remove_member_cv_ref` and `copy_member_cv_ref`.
See #139.
- Why does `qualified_parent_class_of` always returns a const& for PMDs?
See #112.
- Why does `qualified_parent_class_of` return a ref-qualified type even
for unqualified PMFs? See #112.
A few of my own comments:
- Consider using alphabetical order in the table of contents of the
reference. See #135.
- Consider renaming `apply_member_pointer` to something like
`make_member`. See #136.
- Consider dropping the `parent_` in `parent_class_of`, or adding a
note to justify the name. See #137.
Again, thanks to everyone who participated in the review, and
congratulations to Barrett for his work!
Regards,
Louis Dionne
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
I am pleased to announce that the CallableTraits library is CONDITIONALLY
ACCEPTED into Boost. I counted the following formal reviews:
- Bruno Dutra: Yes
- Emil Dotchevski: Yes
- John P. Fletcher: Yes
- Klemens Morgenstern: Yes
- Peter Dimov: Yes
- Zach Laine: Yes
The following people also participated to the review by providing informal
feedback:
- Edward Diener
- Gavin Lambert
- Niall Douglas
- Paul Fultz II
- Tim Song
I also based my decision on previous experience I've had using the library,
and some issues we faced in the Standard Committee where we could have used
CallableTraits. I'd like to thank everyone who participated to the review
for their feedback, and also Barrett Adair for his submission.
The following lists the conditions for acceptance, and also summarizes the
general feedback received during the review. GitHub issues are leveraged
to make the tracking of these comments easier. Below, whenever I refer to
#XYZ, the corresponding ticket can be found at
https://github.com/badair/callable_traits/issues/XYZ
To see all the issues that were discussed during the review, you can search
for tickets labeled with `review` on GitHub (please allow some time for
Barrett to update the new issues I just filed):
https://github.com/badair/callable_traits/issues?page=1&q=+label%3Areview
Conditions for acceptance
-------------------------
The following are points that must be addressed for the library to be
accepted. Once all of the associated GitHub issues are closed, I ask
that the author post to this list with links to the closed issues.
Unless major problems (having to do with the closed issues) are raised
at that time, the library will be automatically accepted into Boost,
no additional review required.
- The library needs to clearly document the advantages of CallableTraits
over FunctionTypes. Concrete examples must be given (at least one,
preferably two). See #117.
- The level of C++ conformance required for the whole library needs to be
clearly stated in the documentation. Individual components should only
document divergences from that base level of conformance. The current
state of things is quite confusing for users. See #118.
- The "algorithmic" parts of the library must be removed, since they are
out of scope. This has already been partly done in issue #114, but the
following algorithms must be discussed too (let's follow up in #114):
+ args_at
+ clear_args
+ pop_front_args
+ push_front_args
+ expand_args_right
+ expand_args_left
- The lazy metafunctions and predicate traits should be SFINAE-friendly,
just like the aliases are. I think this inconsistency is a big deal,
enough to be a condition for acceptance. See #134.
Summary of the feedback received
--------------------------------
The following is a compilation of other points that were made during the
review, but which are not requirements for acceptance. I trust, but do not
require, that all of those points will be considered by the author:
- There is a loss of information with the `args` metafunction. For example,
`void(T)` and `void(T, ...)` both give the same result. See #132.
- Consider adding a way of obtaining the parameter list of a function
without the type of `*this`. See #133.
- The documentation uses the terms of art "implementation defined" and
"undefined behavior" incorrectly. See #103 and #131.
- The non-reference documentation should present at least one real-world
use case. See #116.
- The library should handle top-level cv-qualifiers. See #106.
- Consider supporting different calling conventions (e.g. `__cdecl`).
See #102.
- Consider adding a pre-C++17 implementation of `is_invocable` and friends.
See #119.
- The documentation should clearly state that the library can be used
without the rest of Boost. See #99.
- Consider combining `expand_args` and `args` into a single metafunction
with a default "output" parameter of `std::tuple`. See #126.
- Consider adding `make_fn` and `make_mem_fn` metafunctions to construct
a signature from a return type and a tuple of the arguments. See #113.
- Consider renaming some traits, like `is_reference_member`. See #108.
- Reword some parts of the documentation that look odd due to the use of
passive tense. See #105.
- Consider adding a version of `apply_member_pointer` which carries
cv-qualifiers. See #138.
- Consider adding `remove_member_cv_ref` and `copy_member_cv_ref`.
See #139.
- Why does `qualified_parent_class_of` always returns a const& for PMDs?
See #112.
- Why does `qualified_parent_class_of` return a ref-qualified type even
for unqualified PMFs? See #112.
A few of my own comments:
- Consider using alphabetical order in the table of contents of the
reference. See #135.
- Consider renaming `apply_member_pointer` to something like
`make_member`. See #136.
- Consider dropping the `parent_` in `parent_class_of`, or adding a
note to justify the name. See #137.
Again, thanks to everyone who participated in the review, and
congratulations to Barrett for his work!
Regards,
Louis Dionne
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost