On Mon, Apr 3, 2017 at 4:46 AM, Niall Douglas via Boost
On 03/04/2017 07:45, Louis Dionne via Boost wrote:
Dear Boost community,
The formal review of Barrett Adair's CallableTraits library begins today, April 3rd, and ends on April 12th.
First impressions of the docs: Looks pretty good, but:
* The fact this library can be used without the rest of Boost needs much more obvious declaration
Hmm, good point.
* Why is a std::tuple type the way of returning the function signature?
It's only used for `args` (and as special case input for `apply_return`). I thought this was useful.
Why not some arbitrary (user supplied) variadic template?
`expand_args` sounds like what you're describing.
* I would really prefer a dedicated Compatibility page with a matrix of tested compiler versions against each of the facilities. Right now I need to find the correct function to use and dig into its reference docs. This is unhelpful when I am evaluating the viability of whether to use CallableTraits or not.
Noted. I originally had one of these pages, but I decided to remove it, because I thought that most users of this library would only need one or two features at a time. Generally speaking, the library is usable with GCC 4.7.3+, Clang 3.5.2+, and Visual Studio 2015+. Intel C++ 2017 also kind of works, but this would be fixed/tested/documented fully if accepted.
* I can't help but think repeatedly that Concepts would be real useful in this library. I was surprised they were not mentioned. In particular the docs say things like "If cannot be legally instantiated according to the behaviour above, the behaviour is undefined". I would much prefer that failure to instantiate is a concept failure and/or a static assert or other useful compiler diagnostic, except where SFINAE is desired. In other words, can I not choose which behaviour I want?
I can't prevent users from static_assert-ing (or worse) inside their own templates when the library tries to instantiate them. The "behavior is undefined" notes are limited to the features with template template parameters. Everything else can only fail during substitution. However, I took some pains to use error messages as type names, such that a substitution failure outside of a SFINAE context still provides the same degree of verbosity that the library would have had it used static_assert instead. I made the decision to use SFINAE over static_assert while using CallableTraits to implement Eraserface [1]. I think this was the right decision. I see the benefits of static_assert, but I think they are significantly outweighed by the "SFINAE errors" approach used here, especially since these features are likely to be used in a context where SFINAE applies. Concepts might be nice, but this was not an option, because I chose to support a wide range of compilers.
* Some ABIs let you add proprietary qualifiers to function types e.g. __stdcall. I see only a tiny mention of this at the bottom of the FAQ, yet dealing with proprietary function signature semantics is unavoidable in those systems e.g. introspecting a user supplied pointer to a Win32 syscall.
Indeed. I originally had support for __stdcall, __fastcall, __cdecl, and pascal. There is still vestigial code for this, because I thought it would come up during review. I decided that the test surface for these features across compiler versions was larger than I wanted to bite off for the first iteration of the library. It's very time consuming to do that correctly, so I wanted to wait and see if it would be a desired feature. The library does correctly handle the __cdecl qualifier on MSVC varargs member functions, though. Support for the rest wouldn't be added quickly, but it could be done. Thank you for your time and suggestions. Barrett [1] https://github.com/badair/eraserface